-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
Conversation
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least not in the way our test is checking it:
There was a problem hiding this comment.
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
Do you have an issue open about that in the MDX repo that we could link as a papertrail?
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, | ||
{ |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
No, but I will create today.
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 |
6b5c3d5
to
0fcde60
Compare
c41bb82
to
db25e2e
Compare
There was a problem hiding this 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
#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
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.
html
nodessupport gatsby-remark plugins that return(only generated by gatsby-remark-autolink-headers which we might adjust to be compatible)raw
nodes