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

fix(gatsby-plugin-mdx): Do not leak frontmatter into page #35859

Merged
merged 4 commits into from
Jun 9, 2022
Merged

Conversation

tyhopp
Copy link
Contributor

@tyhopp tyhopp commented Jun 7, 2022

Description

Fix for a recent change that introduced unexpected behavior where mdx frontmatter is rendered into pages.

Documentation

N/A

Related Issues

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jun 7, 2022
@tyhopp tyhopp changed the title fix(gatsby-plugin-mdx): Do not leak front matter into page fix(gatsby-plugin-mdx): Do not leak frontmatter into page Jun 7, 2022
@tyhopp tyhopp removed the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jun 7, 2022
@tyhopp tyhopp marked this pull request as draft June 7, 2022 10:09
@tyhopp tyhopp added the topic: remark/mdx Related to Markdown, remark & MDX ecosystem label Jun 7, 2022

code = `${matter ? matter : ``}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When grayMatter was not passed options prior to this change, matter would always be undefined, resulting in no frontmatter rendered to the page. This changes removes matter entirely from the output node raw body.

Unsure if this is the correct fix since it's unclear to me why matter was included here in the first place (maybe it is stripped in later code paths?).

Copy link
Contributor

Choose a reason for hiding this comment

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

I tracked first introduction of it to ChristopherBiscardi/gatsby-mdx#329 - there is 0 context on it - to me it seem more like debugging code than anything else - the code is in the MDX format, so it would just output any content put there as content which doesn;t make much sense (to me anyway).

Especially with fact that matter in returned object means:

file.matter {String}: the raw, un-parsed front-matter string

@tyhopp tyhopp added the topic: plugins Related to plugin system, themes & catch-all for plugins that don't have a label label Jun 8, 2022
@tyhopp tyhopp marked this pull request as ready for review June 8, 2022 08:02
@pieh
Copy link
Contributor

pieh commented Jun 9, 2022

I don't think we actually want matter there. As long as user continue to have access to frontmatter in the MDX file, we are not removing functionality.

For the behaviour change - gray-matter has some conditional paths based on wether any options were passed or not - so I would expect the difference in behaviour coming from some bugs in conditionals or maybe intentional, but confusing decisions based on that.

In any case, this looks good!

@pieh pieh merged commit 5c7e2a6 into master Jun 9, 2022
@pieh pieh deleted the fix-gray-matter branch June 9, 2022 14:50
pieh pushed a commit that referenced this pull request Jun 10, 2022
pieh pushed a commit that referenced this pull request Jun 10, 2022
pieh pushed a commit that referenced this pull request Jun 10, 2022
…35913)

Co-authored-by: Ty Hopp <tyhopp@users.noreply.github.com>
pieh pushed a commit that referenced this pull request Jun 10, 2022
…35912)

Co-authored-by: Ty Hopp <tyhopp@users.noreply.github.com>
@pieh
Copy link
Contributor

pieh commented Jun 10, 2022

Published in:

  • gatsby-plugin-mdx@3.16.1
  • gatsby-plugin-mdx@2.14.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: plugins Related to plugin system, themes & catch-all for plugins that don't have a label topic: remark/mdx Related to Markdown, remark & MDX ecosystem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants