Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

p tags get merged and className removed #237

Closed
timsuchanek opened this issue Aug 24, 2018 · 7 comments
Closed

p tags get merged and className removed #237

timsuchanek opened this issue Aug 24, 2018 · 7 comments

Comments

@timsuchanek
Copy link

timsuchanek commented Aug 24, 2018

We're using mdx together with next.js' static export.
Everything works perfect, besides one problem.

Certain p tags get merged together when the javascript has been run on the static html.
The problem is that their className gets lost. As we use styled-components (would be the same problem with styled-jsx), the styling is broken at that point.
An example is this page: https://docs-beta.prisma.io/1.13/understand-prisma/prisma-basics-server-services-data-model-avrp/
At the very bottom, you'll see a paragraph with bad line-height.
image

If you disable javascript in the browser (in chrome dev tools settings -> preferences -> Debugger -> Disable JavaScript), you can see, that it's rendered correctly without javascript enabled, with an empty <p></p> tag before the p tag that will be broken.

image

I saw that mdx is using remark-squeeze-paragraphs which as far as I understand should fix exactly this problem. The actual remark-squeeze-paragraphs just removes not used p tags - so this should work.

Now the question is, where in the stack does this p tag merging happen and how can we fix it?

Thanks!

@emplums
Copy link

emplums commented Sep 14, 2018

Having a similar issue, also using next.js. Except all my

tags are getting replaced with <path> elements 😭

@silvenon
Copy link
Contributor

Hi! Is there any code reproducing this issue that I could take a look at? The link is now broken, btw.

@shawnbot
Copy link

@silvenon It's tricky because there are a lot of tools in the chain here: namely, Next.js, mdx, webpack, babel, and, terser. I think we've narrowed it down to the fact that one of the other libraries ends up with one of its functions mangled to have the name p (e.g. function p() { return React.createElement('path', ...) }. That function just happens to be a React component that renders a <path>. This is only happening for us in production, and we're trying to figure out how to disable the terser webpack plugin that might be producing (incorrectly, I think) the offending bundle:

https://github.com/zeit/next.js/blob/10a9178e3206270fd6e317df2e0a4dbffb460a6e/build/webpack.js#L87-L91

It may take a while for us to create a test case for this, but in the meantime can anyone think of any way that the remark transformation would produce React.createElement(p, ...) rather than React.createElement('p')? Is it possible that the component mapping for p elements is being clobbered by a function that's hoisted in the webpack bundle such that it would take precedence over it?

@silvenon
Copy link
Contributor

silvenon commented Sep 17, 2018

Nothing like that comes to mind at the moment, but if you use JSX for tags natively supported by Markdown (<p>, <a> etc.), MDX has trouble with this (#246) and maybe in some combinations it might silently eat an element.

I'm looking forward to that test case once you manage to provide it.

@shawnbot
Copy link

shawnbot commented Sep 17, 2018

No test case yet, but I did confirm that disabling mangling in next.config.js fixes the problem for us:

module.exports = {
  webpack(config, {dev}) {
    // only disable mangling in production
    if (dev) {
      return config
    }
    for (const plugin of config.plugins) {
      // duck type: is this an UglifyJS plugin?
      if (plugin.options && plugin.options.uglifyOptions) {
        console.warn('*** disabling mangling in UglifyJS plugin ***')
        plugin.options.uglifyOptions.compress = {keep_fnames: true}
        plugin.options.uglifyOptions.mangle.keep_fnames = true
      }
    }
    return config
  }
}

I've attempted to trace the source of whatever is calling our mangled function name in our production bundles, but I haven't figured it out yet. I'm 80% sure that the mangled icon function p() declaration is bleeding into MDX's scope somehow, and whatever is rendering paragraphs is calling that function instead.

@silvenon
Copy link
Contributor

silvenon commented Sep 18, 2018

That's a pretty wild. It seems so unlikely that the string "p" turned into reference p at some point, even though it looks like a small thing, but I guess something like that is going on here. Meanwhile I'll make my own tests with mangling.

@johno
Copy link
Member

johno commented Feb 15, 2019

I suspect that this is happening due to JS minification since we haven't been able to reproduce. If you are able to create a repro without code minifying please open an issue and we can keep digging!

@johno johno closed this as completed Feb 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants