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

mdx-v2: feat: add support for remark, rehype and gatsby-remark plugins #35751

Merged
merged 7 commits into from
May 31, 2022

Conversation

axe312ger
Copy link
Collaborator

@axe312ger axe312ger commented May 25, 2022

This adds plugin support to the new iteration of the MDX plugin.

To achieve this, I'd had to replace @mdx-js/loader with a custom loader, which inject the GraphQL MdxNode and the path to the original file into the context of the remark plugins.

To keep the new MDX plugin compatible with old remark and gatsby-remark plugins, we wrap their output with https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml. We have to see the impact on real life projects of doing so, but this methods is the least processing and rewriting to achieve plugin support.

  • support remark plugins
  • support rehype plugins
  • support gatsby-remark plugins that return html nodes
  • support gatsby-remark plugins that return raw nodes (only generated by gatsby-remark-autolink-headers which we might adjust to be compatible)
  • supply actual real path to file, its mocked right now
  • move html/raw transformer into its own plugin, so its not run multiple times

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label May 25, 2022
@axe312ger axe312ger changed the title feat: add support for remark, rehype and gatsby-remark plugins mdx-v2: feat: add support for remark, rehype and gatsby-remark plugins May 25, 2022
@axe312ger axe312ger marked this pull request as ready for review May 26, 2022 12:29
processor.data(`mdxNode`, mdxNode)

const result = await processor.process(
// Inject path to original file for remark plugins. See: https://github.com/gatsbyjs/gatsby/issues/26914
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really still need that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I look at https://unifiedjs.com/learn/guide/create-a-plugin/ the file as a second argument is still there. So there will be remark plugins that rely on that.

Isn't the official loader also doing this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@axe312ger then we'll need to update our test later

@Khaledgarbaya Khaledgarbaya added this to the MDX v2 milestone May 30, 2022
@LekoArts LekoArts added topic: remark/mdx Related to Markdown, remark & MDX ecosystem and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels May 30, 2022
@LekoArts
Copy link
Contributor

LekoArts commented May 30, 2022

To achieve this, I'd had to replace @mdx-js/loader with a custom loader, which inject the GraphQL MdxNode and the path to the original file into the context of the remark plugins.

Do you have an issue open about that in the MDX repo that we could link as a papertrail?

To keep the new MDX plugin compatible with old remark and gatsby-remark plugins, we wrap their output with reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml.

Is this also necessary for remark plugins? I thought only for gatsby-remark-* plugins. Because rehype and remark are just supported by default from the MDX loader.

const code = await compile(contents, mdxOptions)
const code = await compileMDX(
contents,
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are these two empty objects doing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are mocked MDX and File nodes, as these are required by some gatsby-remark-* plugins to properly parse and render the MDX.

Here they are mocked, as they are not existing yet in thepreprocessSource step (correct me if I am wrong)

As this is mostly done to locate static GraphQL queries, I wonder if we should just skip all remark/rehype/gatsby-remark-* plugins in this step. That way we'd not need to pass any data down the pipeline here, probably also save some processing time.

@axe312ger
Copy link
Collaborator Author

To achieve this, I'd had to replace @mdx-js/loader with a custom loader, which inject the GraphQL MdxNode and the path to the original file into the context of the remark plugins.

Do you have an issue open about that in the MDX repo that we could link as a papertrail?

No, but I will create today.

To keep the new MDX plugin compatible with old remark and gatsby-remark plugins, we wrap their output with reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml.

Is this also necessary for remark plugins? I thought only for gatsby-remark-* plugins. Because rehype and remark are just supported by default from the MDX loader.

Maybe thats is because remark plugins shouldn't create HTML, or they should, but not in MDX context. I am not sure about this, but for sure I know, MDX does not like raw HTML nodes :D

Maybe also something to ask in the MDX repo

@axe312ger axe312ger force-pushed the feat/mdx-v2-plugin-support branch from 6b5c3d5 to 0fcde60 Compare May 31, 2022 09:45
@axe312ger axe312ger force-pushed the feat/mdx-v2-plugin-support branch from c41bb82 to db25e2e Compare May 31, 2022 10:57
@axe312ger axe312ger requested a review from LekoArts May 31, 2022 12:30
Copy link
Contributor

@LekoArts LekoArts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to revisit the GraphQL queries aspect of it but for now let's merge this as the rest looks good

@LekoArts LekoArts merged commit d0b4b9d into feat/mdx-v2 May 31, 2022
@LekoArts LekoArts deleted the feat/mdx-v2-plugin-support branch May 31, 2022 13:52
axe312ger added a commit that referenced this pull request Jun 27, 2022
#35751)

* feat: add support for remark, rehype and gatsby-remark plugins

* clean up

* refactor: extract our two custom remark into their own files

* fix: pass path to MDX file for remark plugins

* refactor: remove type assertions

* feat: extend plugin option validation

* remove old comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: remark/mdx Related to Markdown, remark & MDX ecosystem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants