-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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(v2): remove HTML from heading slug #2426
Conversation
Deploy preview for docusaurus-2 ready! Built with commit f0fdcc6 |
@@ -62,7 +62,7 @@ exports[`non text phrasing content 1`] = ` | |||
}, | |||
{ | |||
value: '<i>HTML</i>', | |||
id: 'ihtmli', | |||
id: 'html', |
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.
Fixed extra bug! 😃
|
||
const processFixture = async (name, options) => { | ||
const path = join(__dirname, 'fixtures', `${name}.md`); | ||
const file = await vfile.read(path); | ||
const result = await remark() | ||
.use(slug) |
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.
Here I use our slugger plugin, because these plugins are interdependent:
remarkPlugins: [emoji, slug, rightToc], |
Therefore the tests should take this fact into account.
FYI, rightToc also generated slug, which is actually somewhat redundant, because we already did this in the slugger (ex-remark-slug) plugin.
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.
How about sending a PR to remark-slug
and get it merged upstream?
Motivation
Resolve #2419
Currently, the use of HTML in headings is reflected in slug, see the example below:
Using this heading:
## <span className="faq-header">General</span>
will lead to slug of "span-classname-faq-header-general" instead of "general".
I made a kind of fork remark-slug, because to solve this bug we need to change the way the slug is generated. To ensure that nothing broke, I adopted the tests from the underlying lib (rewritten to Jest) and added a new test that confirms the fix for this bug.
Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
Try add heading which contains HTML (see Motivation section for sample).
Related PRs
(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)