MDX parsing appears broken

[godmar]

I'm trying to interleave MDX inside JSX, as outlined here.

The result is a completely unpredictable parse result which rarely works correctly.

I've placed a minimal repo that reproduces the issue here: https://github.com/godmar/gatsby-docz-bug
To see it, clone and do

yarn
yarn gatsby develop

The following MDX:

---
title: "Test for Bug"
date: 2019-09-05T14:53:20.798Z"
menu: MenuEntry1
---
import Notice from '../../src/components/Notice'

<Notice type={"tip"} title={"Side note"}>

This text uses *markdown*, but the markdown does not arrive at the component.

</Notice> 
Markdown in the *middle* does not work, either. A [link](www.google.com)
<Notice type={"tip"} title={"Tip"}>

The same is _true_ here.

</Notice>

is rendered as:

, but the markdown does not arrive at the component. This text uses
Markdown in the middle does not work, either. A link
here. The same is

when embedded in the Notice component (which pretty much echoes its children in a box, without manipulating them in any way.

Could this be a parser bug? If so, is it possible to update the mdx parser and override the one gatsby-docz-theme uses?

[rakannimer]

Hello @godmar

As I told you in #1039 you would need to parse the MDX as a string from your side.

Example

In your jsx

const Notice = (children) =>  {
	// maybe use react-markdown to parse https://github.com/rexxars/react-markdown
	return <code>{children}</code>
}

In your mdx

Notice Example : 

<Notice>
{`
Markdown here 
`}
</Notice>

[godmar]

@rakannimer please take a look at my source. Here it is again. This should work. BTW, sometimes it does work, it's just extremely brittle.

[rakannimer]

Docz uses gatsby-plugin-mdx to parse MDX files.

Maybe they can help with your issue ?

Happy to review a PR if it's fixable from docz's side !

[godmar]

That was my next step - reproduce it without docz.

I'm curious about that in general though. If say there were a bug in the gatsby-plugin-mdx and that bug is already fixed, would I be able to pick up the fix without an update to gatsby-theme-docz? Or am I tied to the version of gatsby-plugin-mdx you're bundling? It seems docz is relying on several gatsby plugins.

[godmar]

I wasn't able to reproduce this bug with the gatsby mdx starter.
You can see that it works just fine without docz: https://github.com/godmar/gatsby-mdx-bug and specifically here.

On the other hand, with gatsby-theme-docz: https://github.com/godmar/gatsby-docz-bug and here it does not. The .mdx files are identical except for the path to my component.

The MDX starter used gatsby-plugin-mdx ^1.0.0 whereas docz uses ^1.0.13, but that doesn't make a difference. With the MDX starter, it works with gatsby-plugin-mdx 1.0.0 and 1.0.13.

These are both minimal repositories. The docz is created with your starter instructions and just a few files added. The MDX is from the gatsby mdx starter, and gatsby-plugin-mdx is upgraded.

So there must be some setting or option or some divergent version that steers docz away. Could it be that you have other plugins that operate on the AST?

ps: reversing to @mdx-js/mdx 1.1.0 and @mdx-js/react^1.0.27 also doesn't reproduce the bug.

[godmar]

FWIW, removing remark-docz from node_modules/gatsby-theme-docz/gatsby-config.js avoids this bug, so this is where I'll look at now.

[godmar]

It's caused by whatever you're doing here. It seems to be assuming that a JSX node has to have both an opening and closing tag, but that's not the case when markdown is interleaved in JSX as can be seen from this AST. Thoughts?

[godmar]

Specifically, this is the tree coming in:

     { type: 'jsx',
       value: '<Notice type={"tip"} title={"Side note"}>',
       position: [Position] },
     { type: 'paragraph', children: [Array], position: [Position] },
     { type: 'jsx', value: '</Notice> ', position: [Position] },
     { type: 'paragraph', children: [Array], position: [Position] },
     { type: 'jsx',
       value: '<Notice type={"tip"} title={"Tip"}>',
       position: [Position] },
     { type: 'paragraph', children: [Array], position: [Position] },
     { type: 'jsx', value: '</Notice>', position: [Position] },

and this is what you do with it:

     { type: 'jsx',
       value:
        '<Notice type={"tip"} title={"Side note"}>\n, but the markdown does not arrive at the component.\n\nThis text uses \n</Notice> \n\n does not work, either. A \n\nMarkdown in the \n<Notice type={"tip"} title={"Tip"}>\n here.\n\nThe same is \n</Notice>',
       position: [Position] },

This is taken before and after the mergeNodeWithoutCloseTag:

  console.log("Tree coming in");
  console.dir(tree);
  visit(tree, 'jsx', (node, idx) => {
    // check if a node has just open tag
    mergeNodeWithoutCloseTag(tree, node, idx);
  });
  console.log("Tree going out");
  console.dir(tree);
}

[godmar]

For now, I just commented out the call to visit (hot-patched in my node_modules folder) but it would be great if you could reason about what you meant to do there. It looks like it was intended to help with error recovery if a user omits the closing tag, but the assumptions don't hold in the face of MDX in JSX.

On the other hand, for me personally, having MDX inside JSX is pretty much the only reason to use MDX (and docz by implication, since I haven't been to find anything else comparable that creates a sidebar automatically).

[rakannimer]

Nice work pinning down the exact issue !

It looks like it was intended to help with error recovery if a user omits the closing tag

Yep, we might get away with removing it, it shouldn't break anything in the examples or docs.

Would you be interested in submitting a PR ?

[godmar]

Would you be interested in submitting a PR ?

Only if you show me how to test your package.

PS: by that, I mean I'd like to know the steps of cloning, building, and then testing the way a user of your package would (unless it's easy to figure out from the README)

[johno]

From what I recall this is for handling some edge cases in JSX parsing that became common in Docz with the render prop pattern (which might now be removed in the latest version?). That code was meant to better handle MDX files with:

# Hello, world!

<Foo>({ bar }) => {
  const someVar = doStuff()

  return (
    <h1>{bar} {someVar}</h1>
  )
}</Foo>

The blank line in the Foo block isn't currently properly parsed in MDX so the code you're having an issue with was originally meant to "stitch it back together" in the AST.

[godmar]

Would a heuristic then need to be applied to tell the cases apart? For instance, would the first child be a paragraph (or some other markdown element) in the case where MDX is embedded in JSX?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[godmar]

PS: It seems that this issue still hasn't been fixed, see here for a reproduction with gatsby-theme-docz@2.3.1.

For my purposes, this makes docz unusable - I cannot create content when it doesn't render correctly.