-
Notifications
You must be signed in to change notification settings - Fork 158
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
Bundle size and build time improvements #715
Conversation
This avoids having two copies of rollup in two separate node_modules directories, makes it harder to forget to change something in all three rollup config files, and allows making various improvements, such as: - outputting playground.js directly to the website directory, rather than outputting it elsewhere and then copying it; - running the replace() plugin on script.js, which decreases the size slightly by eliminating the debug code instead of skipping it, shaving about 10 seconds off of 'npm run test262'.
Instead of importing the whole of es-abstract, import only the abstract operations that we actually use in the polyfill. This reduces the minified bundle size by about a quarter. It also speeds up 'npm run test262' by about thirty seconds on my machine. See: #710
- mkdirp out/docs/assets already creates out/ - the npm scripts are run in subshells, so cd .. at the end does nothing
In 93e3403 I changed this due to a warning from npm, but I had misunderstood how it works and also failed to notice that it actually runs rollup twice whenever you build. Change it back to what it was, for the time being.
These are duplicates of the polyfill's dependencies. They aren't used at the top level.
We don't have these files anymore.
Loading mermaid from a CDN is going to be faster for people visiting the page, as well as easier for us to maintain.
This cuts about a second off the build time, and I verified that it has no effect on the built bundles whatsoever. I think node-builtins was previously used to provide a custom shim for util.inspect in the browser polyfill, but that does not seem to be needed anywhere any longer.
Codecov Report
@@ Coverage Diff @@
## main #715 +/- ##
==========================================
- Coverage 92.45% 92.33% -0.13%
==========================================
Files 17 17
Lines 4720 4733 +13
Branches 746 743 -3
==========================================
+ Hits 4364 4370 +6
- Misses 351 360 +9
+ Partials 5 3 -2
Continue to review full report at Codecov.
|
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.
I'm not the biggest rollup/babel expert, but all commits LGTM.
tc39#715 broke Test262 tests (specifically the ability to `import * from 'es-abstract'), and I couldn't figure out how to import individual es-abstract js functions from the PoC TypeScript code, so I gave up and hacked up the three es-abstract methods (ToString, ToObject, and ToInteger) that the PoC uses. A real JS implementation will simply depend on ecmascript.js, so it wasn't worth doing a real implementation. I apologize for how bad this placeholder code is. ;-(
- Convert all conversion methods to use new toXxx naming convention - Fix Test262 tests broken by build changes in tc39#715
- Convert all conversion methods to use new toXxx naming convention - Fix Test262 tests broken by build changes in tc39#715
- Convert all conversion methods to use new toXxx naming convention - Fix Test262 tests broken by build changes in tc39#715
- Convert all conversion methods to use new toXxx naming convention - Fix Test262 tests broken by build changes in tc39#715
- Convert all conversion methods to use new toXxx naming convention - Fix Test262 tests broken by build changes in tc39#715
- Convert all conversion methods to use new toXxx naming convention - Fix Test262 tests broken by build changes in tc39#715
- Convert all conversion methods to use new toXxx naming convention - Fix Test262 tests broken by build changes in tc39#715
- Convert all conversion methods to use new toXxx naming convention - Fix Test262 tests broken by build changes in tc39#715
- Convert all conversion methods to use new toXxx naming convention - Fix Test262 tests broken by build changes in tc39#715
- Convert all conversion methods to use new toXxx naming convention - Fix Test262 tests broken by build changes in tc39#715
- Convert all conversion methods to use new toXxx naming convention - Fix Test262 tests broken by build changes in tc39#715
- Convert all conversion methods to use new toXxx naming convention - Fix Test262 tests broken by build changes in tc39#715
- Convert all conversion methods to use new toXxx naming convention - Fix Test262 tests broken by build changes in tc39#715
- Convert all conversion methods to use new toXxx naming convention - Fix Test262 tests broken by build changes in tc39#715
- Convert all conversion methods to use new toXxx naming convention - Fix Test262 tests broken by build changes in tc39#715
- Convert all conversion methods to use new toXxx naming convention - Fix Test262 tests broken by build changes in tc39#715
- Convert all conversion methods to use new toXxx naming convention - Fix Test262 tests broken by build changes in tc39#715
This was the lowest-hanging fruit I could find for #710. Another benefit of smaller bundle size is reducing the time it takes to run the test262 tests! So while I was looking at that I made some further improvements to the build scripts.
The minified bundle size went from 177 kb to 135 kb, and the time it takes for the CI to run went from 9 to 7½ minutes.