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

Replace rehype with rehype-dom in the browser #242

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

eatyourgreens
Copy link
Contributor

@eatyourgreens eatyourgreens commented Jan 6, 2024

Use rehype-dom 5.0 (36k) as the Rehype processor in browsers, where Rehype can use DOM APIs to parse HTML.

In Node, require in the full rehype processor, which is ~240k.

This should give smaller, faster bundles for Markdown parsing in browsers.

Use `rehype-dom` 5.0 (36k) as the Rehype parser in browsers, where Rehype can use DOM APIs to parse HTML.

In Node, require in the full `rehype` processor, which is ~240k.

This should give smaller, faster bundles for Markdown parsing in browsers.
Copy link
Contributor

@mcbouslog mcbouslog left a comment

Choose a reason for hiding this comment

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

Nice refactor! 🚀 🏃

@mcbouslog mcbouslog merged commit cb53b31 into zooniverse:main Jan 17, 2024
2 of 3 checks passed
@eatyourgreens eatyourgreens deleted the rehype-dom branch January 17, 2024 15:48
@mcbouslog
Copy link
Contributor

I'm still investigating, but I think these changes are causing the following issue when running PFE locally linked to this updated markdownz:

ERROR in ../markdownz/node_modules/hast-util-from-dom/dist/hast-util-from-dom.mjs 2:0-32
Module not found: Error: Can't resolve 'hastscript/html' in '/Users/markbouslog/zooniverse/markdownz/node_modules/hast-util-from-dom/dist'
Did you mean 'html.js'?
BREAKING CHANGE: The request 'hastscript/html' failed to resolve only because it was resolved as fully specified
(probably because the origin is strict EcmaScript Module, e. g. a module with javascript mimetype, a '*.mjs' file, or a '*.js' file where the package.json contains '"type": "module"').
The extension in the request is mandatory for it to be fully specified.
Add the extension to the request.
Did you miss the leading dot in 'resolve.extensions'? Did you mean '[".*",".js",".jsx",".json",".cjsx",".coffee",".styl",".css"]' instead of '["*",".js",".jsx",".json",".cjsx",".coffee",".styl",".css"]'?

@mcbouslog
Copy link
Contributor

mcbouslog commented Jan 17, 2024

I think rehype-dom should be v6.0.0 and I'll update the import in utils.js to import { rehypeDom } from 'rehype-dom';

@eatyourgreens
Copy link
Contributor Author

😳 I tested it by installing it in @zooniverse/react-components then building and running the Next.js apps. I assumed it would work with the PFE webpack build if it also worked in the monorepo.

v6 is ESM only so can't be used here. Or at least, removing the CJS build from markdownz would then break PFE and the monorepo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants