Migrate markdown-gfm to remark and rehype#18645
Conversation
…gfm-no-autolink`.
|
EDIT: PR is ready for review. |
markdown-gfm to remark / rehype packages.markdown-gfm to remark and rehype
markdown-gfm to remark and rehypemarkdown-gfm to remark and rehype
|
|
||
| // Check if view has correct data. | ||
| expect( html ).to.equal( viewString ); | ||
| expect( JSON.stringify( html ) ).to.equal( JSON.stringify( viewString ) ); |
There was a problem hiding this comment.
This improves the readability when the output is different from the expected value.
|
|
||
| // When converting back it will be normalized and spaces | ||
| // at the beginning of inline code will be removed. | ||
| 'regular text and `inline code`' |
There was a problem hiding this comment.
I don't see a reason why this should be “normalized” like this. It seems invalid.
pszczesniak
left a comment
There was a problem hiding this comment.
I assume that LICENSE.md file must be updated also.
Done ✔️ |
Good catch. This didn't work in the old implementation too, so I don't think it's a blocker. We will discuss whether to fix this now or as a follow-up. |
…to-unifiedjs-ecosystem
|
PR is ready for technical re-review. |
|
@filipsobol We've finished testing the changes with @juliaflejterska and apart from reported issues the changes look good to us 👌 |
…to-unifiedjs-ecosystem
d3de5e2
Suggested merge commit message (convention)
Other (markdown-gfm): Migrate to remark / rehype packages. Closes #18684.
MINOR BREAKING CHANGE (markdown-gfm): Migrate from
markedandturndowntoremarkandrehype.MINOR BREAKING CHANGE (markdown-gfm): Enable autolinking in Markdown (works only when loading Markdown content into the editor).
Things to prioritize when testing this PR
The following areas should be prioritized when testing this PR, as they previously had (or have now) custom handling:
www.example.comthat are converted into links when loaded into the editor.keepHtmlfunction (which appears to lack documentation aside from the API reference)A few thoughts on the migration
The
remarkandrehypecombo works more predictably – likely because they come from the same ecosystem – compared tomarkedandturndown, which are developed independently. This is also reflected in the tests, which needed to be updated to pass.One benefit of the older setup was its smaller size. Migrating from
markedandturndowntoremarkandrehypeincreases the build size by 40 KiB gzipped (140.51 KiB uncompressed).The
unifiedjsecosystem (which includesremarkandrehype) is also much more modular, whileturndownandmarkeddon't have any external dependencies. This increases the total number of dependencies from 10 to 97. The number of direct dependencies in markdown'spackage.jsonincreased from 5 to 14. However, the following plugins are foundational and are already dependencies of the larger ones:hasthast-util-from-domhast-util-to-htmlhast-util-to-mdastunist-util-visitChallenges
There were two major challenges when implementing this change.
Autolinking
The first was autolinking. The original implementation had autolinking (automatically turning text links like
http://example.cominto<http://example.com>) disabled. While this wasn't compliant with the GFM spec, there may have been reasons for it. However, we didn’t treat text links like regular text either, because we explicitly disabled escaping of them. This meant they were neither a link nor plain text.As a result, Markdown returned by the editor could include unescaped text links, which would later be turned into links by other Markdown parsers. We decided to enable autolinking so text links are turned into regular links when loaded in the editor. Text links typed directly into the editor will not be turned into links, but instead escaped like regular text –providing a better WYSIWYG experience.
Bundle size
Another issue we anticipated was bundle size. Typically, Markdown and HTML parsing happens on the server, so most packages are built with that environment in mind. However, unlike the browser, server environments don't have built-in DOM parsers and instead use packages like
parse5. Whileparse5is excellent, it's completely unnecessary in a browser environment, which already has a built-in DOM parser.The
rehype-dom-parseandrehype-dom-stringifypackages are DOM-based alternatives torehype-parseandrehype-stringify, and they resolved most of the related issues. One issue they didn’t solve was parsing HTML inside Markdown. Unfortunately, there doesn’t appear to be a first-party solution for this, which necessitates writing a small custom plugin.You can find more information here: https://github.com/orgs/rehypejs/discussions/202. While this discussion hadn’t received an answer at the time of writing, it provides useful context for the issue, so I’m leaving the link for “future us”.