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

slim down jsdom dependency to parse5 #355

Merged
merged 1 commit into from
Oct 7, 2024
Merged

Conversation

titoBouzout
Copy link
Contributor

@titoBouzout titoBouzout commented Sep 25, 2024

Issue: jsdom imports a ton of stuff. This is usually not a problem as it's been treeshaked from the different build tools. But it causes an issue when you are trying to use the babel-solid-preset entirely in the browser. For example jsdom imports child_process, that just doesn't exist in the browser, and places like esm.sh cannot treeshake it because it doesn't know if it is going to be used or not. This is causing problems to projects like https://github.com/bigmistqke/repl and possibly some others.

Upon investigation, it seems that jsdom it's using behind the scenes parse5. So trying to slim down the dependencies I replaced it with that. This also gives me more confidence into the approach taken to validate the HTML because the functions from parse5 actually claim they are mirroring innerHTML behaviour.

Links:
jsdom importing parse5
https://github.com/jsdom/jsdom/blob/04541b377d9949d6ab56866760b7883a23db0577/lib/jsdom/browser/parser/html.js#L3

claim that should do the same as innerHTML
https://github.com/inikulin/parse5/blob/master/packages/parse5/lib/index.ts#L64

I have updated my transform to use parse5 too, so its been tested on my stuff besides npm run test on the babel plugin
https://github.com/potahtml/pota/blob/master/babel-preset/transform/validate.js

We will appreciate please a patch release(whenever there's time and that's possible) so projects like https://github.com/bigmistqke/repl can continue development.

Lastly, we are not entirely sure if this will actually fix the "use in browser issue", but it will narrow it greatly to investigate further in case that's necessary. I can confirm it works entirely in the browser

Thanks!

@ryansolid ryansolid merged commit fc5fdb7 into ryansolid:main Oct 7, 2024
2 checks passed
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