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

Add bundling to the build step #1535

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

BenjaminAster
Copy link
Contributor

Happy-DOM currently does not bundle its JavaScript files. This is fine for the most part, but I've found that especially when using it in a worker, this slows things down tremendously:

I use Happy-DOM for "server-side" HTML prerendering in my winzig framework where a worker imports Happy-DOM and pollutes the global scope with Happy-DOM's stuff to somewhat emulate an execution in the browser. This works fine, but the worker needs about >400ms to execute the prerendering, and I've found that most of this time is spent by Node.js parsing the files in the lib directory, searching for import statements and loading dependency files, and this hundreds of times for the many files of Happy-DOM.

By bundling all of Happy-DOM to a single, 40,000 line long file, I was able to drastically reduce the time the worker needs to <100ms, so a >300% speed increase.

Now, replacing the files in lib/ with a single bundle might break existing code that relies on specific files from there (?), and it might also worsen debuggability a bit. So my though was to make bundle importing opt-in by simply providing an additional /bundled export, i.e.

import { ... } from "happy-dom";

would still load lib/index.js like before, but

import { ... } from "happy-dom/bundled";

would load the bundled JavaScript file in dist/bundle.js.

This is all a bit hacky; maybe there's a better way to do it? And should the import be called /bundled or /bundle? And should a CommonJS bundle also be added? Personally, I'm of course fine with just completely defaulting to the bundle in ESM, but as I mentioned I don't know if it's feasible.

Let me know what you think of this idea and feel free to modify this PR if you have better ideas!


Unrelated side note: Happy-DOM currently lists the webidl-conversions package as a dependency, but it doesn't seem to be used anymore anywhere in the code. It was probably forgotten to be removed, so I removed it as part of this PR (in case this PR will be approved, otherwise you can simply remove it yourself 🙂).

@BenjaminAster
Copy link
Contributor Author

Oops, I made the linter tests get stuck because I forget to exclude the bundle from the linter, sorry for that 😬

Should be fixed now.

@capricorn86
Copy link
Owner

Hi @BenjaminAster! 🙂

Since adding the bundle to the "happy-dom" package would increase the package size quite a bit, I'm a bit hesitant to add it. I'm considering to move the CJS version to its own package, just to decrease the size. I think I might need to do some tests as well.

You might want to consider reusing your workers. In that case Happy DOM and other libraries will be cached within the workers. You could then spawn workers depending on how many available CPU cores you have.

@BenjaminAster
Copy link
Contributor Author

@capricorn86 Oh, ok, I see.

The "server-side" HTML prerendering in winzig, which I have deliberately put in quotes, currently just means the developer typing winzig into their terminal and winzig generating static HTML once and saving it to index.html, effectively acting as a preprocessor on steroids. Real SSR might be added some time in the future, and yes, then the workers will probably be reused. So in my case just reusing workers instead of having a bundled Happy-DOM is not an option.

That said, if you don't want to pollute your npm package with an additional ~1MB bundle file, which I totally understand, what if the bundled version was put in a different npm package, like @happy-dom/bundled? I could also just publish my own @benjaminaster/bundled-happy-dom package or something just for use by myself if you're ok with that (I guess the license would allow that?), since I do have an admittedly rather niche requirement.

In any case, thanks a lot for the immense amount of work you put into this package. And the code is just so much more readable (and uses TypeScript & ESM) than JSDOM 🤩!

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