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-transformer-remark): fix plugin conflict causing escaped HTML in headings #20024

Merged
merged 2 commits into from Dec 16, 2019

Conversation

Ameobea
Copy link
Contributor

@Ameobea Ameobea commented Dec 9, 2019

Description

  • The gatsby-remark-prismjs plugin was causing HTML code to be escaped and rendered as text in TOC entries generated by gatsby-transformer-remark. It generated HTML nodes which the mdast-util-toc module flattened into text. This was fixed upstream in a major version bump.
  • Update the version of mdast-util-toc used in gatsby-transformer-remark to the latest version in order to fix the issue
    • Set { allowDangerousHTML: true } on calls to hastToHTML() and toHAST() when building the TOC. This is required in order to properly render the embedded HTML nodes.
  • Bump mdast-util-to-hast to its latest version as well
    • Update one snapshot test that was very slightly broken as a result of this
  • Slight refactor of code in on-node-create.js to reduce nesting and clean up control flow
  • Add two snapshot tests to verify that the fix was successful

Related Issues

Fixes #13608

Additional Info

This does not address updating the generated anchor link as mentioned here: #13608 (comment)

Ideally, another change would be applied to remove the HTML artifacts from the anchor link.

@Ameobea Ameobea requested a review from a team as a code owner December 9, 2019 23:18
@Ameobea Ameobea changed the title Fix plugin conflict causing escaped HTML in headings fix(gatsby-transformer-remark) Fix plugin conflict causing escaped HTML in headings Dec 9, 2019
@pieh
Copy link
Contributor

pieh commented Dec 10, 2019

I just merged few other remark PRs that cause merge conflicts here - happy to resolve them if you want me to (don't want to push to your branch without asking first :) )

@Ameobea
Copy link
Contributor Author

Ameobea commented Dec 10, 2019

I've taken feedback into account and rebased to the latest master. Feel free to squash these two commits when merging to produce a cleaner history.

@Ameobea
Copy link
Contributor Author

Ameobea commented Dec 13, 2019

Hey - just wanted to bump this since it's close to getting off the first page of PRs and I know this is an incredibly active repo.

I've made all requested changes as per code review and reduced the size of the diff. It's been freshly rebased to the latest master.

 * The `gatsby-remark-prismjs` plugin was causing HTML code to be escaped and rendered as text in TOC entries generated by `gatsby-transformer-remark`.  It generated HTML nodes which the `mdast-util-toc` module flattened into text.  This was fixed upstream in a major version bump.
 * Update the version of `mdast-util-toc` used in `gatsby-transformer-remark` to the latest version in order to fix the issue
  * Set `{ allowDangerousHTML: true }` on calls to `hastToHTML()` and `toHAST()` when building the TOC.  This is required in order to properly render the embedded HTML nodes.
 * Bump `mdast-util-to-hast` to its latest version as well
  * Update one snapshot test that was very slightly broken as a result of this
 * Slight refactor of code in `on-node-create.js` to reduce nesting and clean up control flow
 * Add two snapshot tests to verify that the fix was successful
 * This bump caused a small regression in one of the tests which was deemed unacceptable.  This reverts it to its previous version and restores the original test behavior.
  * Revert the updated snapshot test as well
 * Undo refactoring changes in `extend-node-type.js` in order to produce a cleaner diff for the PR
Copy link
Contributor

@pieh pieh left a comment

Choose a reason for hiding this comment

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

Let's get this in. Thanks for research, fixes and tests!

@pieh pieh changed the title fix(gatsby-transformer-remark) Fix plugin conflict causing escaped HTML in headings fix(gatsby-transformer-remark): fix plugin conflict causing escaped HTML in headings Dec 16, 2019
@pieh pieh added the bot: merge on green Gatsbot will merge these PRs automatically when all tests passes label Dec 16, 2019
@gatsbybot gatsbybot merged commit 1abcbb2 into gatsbyjs:master Dec 16, 2019
@gatsbot
Copy link

gatsbot bot commented Dec 16, 2019

Holy buckets, @Ameobea — we just merged your PR to Gatsby! 💪💜

Gatsby is built by awesome people like you. Let us say “thanks” in two ways:

  1. We’d like to send you some Gatsby swag. As a token of our appreciation, you can go to the Gatsby Swag Store and log in with your GitHub account to get a coupon code good for one free piece of swag. We’ve got Gatsby t-shirts, stickers, hats, scrunchies, and much more. (You can also unlock even more free swag with 5 contributions — wink wink nudge nudge.) See gatsby.dev/swag for details.
  2. We just invited you to join the Gatsby organization on GitHub. This will add you to our team of maintainers. Accept the invite by visiting https://github.com/orgs/gatsbyjs/invitation. By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests.

If there’s anything we can do to help, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’.

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: merge on green Gatsbot will merge these PRs automatically when all tests passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[gatsby-transformer-remark] Section titles with code in ``s creates escaped HTML <code> tags
4 participants