-
Notifications
You must be signed in to change notification settings - Fork 112
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
Exclude core-js polyfills from ESM-capable browser bundles #6068
Conversation
**Why**: So that the majority of users don't have to pay the performance penalty for polyfills supporting legacy browsers, avoid loading those polyfills except in the legacy browsers.
…r modern browsers
@@ -1,8 +1,7 @@ | |||
import 'promise-polyfill/src/polyfill'; |
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.
core-js
includes a Promise
and URL
polyfill, so we don't need the other one. Which is nice because it lets us use fewer dependencies.
Based on failing build, looks like I need to fix something with how the regenerator runtime is included for modern browser builds. |
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.
LGTM
…#6075) * Revert "Revert "Exclude core-js polyfills from ESM-capable browser bundles (#6068)" (#6074)" This reverts commit 376bc2c. * Drop jsbundling_rails in favor of custom rake task **Why**: So that we can have CI install dependencies in the same way as is expected in production, since jsbundling_rails will always install devDependencies with no option to opt-out. * Revert "Drop jsbundling_rails in favor of custom rake task" This reverts commit c4b97b5. * Use NODE_ENV production to exclude devDependencies in CI * Move babel-plugin-polyfill-corejs3 to dependencies **Why**: So that it's available in production build environments. changelog: Improvements, Optimization, Skip polyfill load behavior for modern browsers
**Why**: - Because polyfills are intended to be confined to the polyfill package (#6060) - So that it's not included in modern browsers (#6068, -0.9kb gzipped) - To reduce surface area of dependencies shared with USWDS (related to 18F/identity-design-system#312 (comment)) changelog: Internal, Optimization, Reduce size of JavaScript for modern browsers
**Why**: - Because polyfills are intended to be confined to the polyfill package (#6060) - So that it's not included in modern browsers (#6068, -0.9kb gzipped) - To reduce surface area of dependencies shared with USWDS (related to 18F/identity-design-system#312 (comment)) changelog: Internal, Optimization, Reduce size of JavaScript for modern browsers
Why: So that the majority of users don't have to pay the performance penalty for polyfills supporting legacy browsers, avoid loading those polyfills except in the legacy browsers.
This builds on #6060 (#6060 (comment)). While #6060 excluded polyfills implemented through our
@18f/identity-polyfill
package, it did not cover polyfills injected through Babel's@babel/preset-env
preset. With these changes,@babel/preset-env
is configured to only include polyfills for browsers which are capable of ESM, and includes a full set of polyfills for non-ESM-capable browsers in@18f/identity-polyfill
, to be loaded only for those browsers. JavaScript syntax is still downgraded to all supported browsers including IE11, which is also tested for usingyarn es5-safe
.Local measurements (via
NODE_ENV=production make run
) show a reduction of JavaScript on the home page of ~34% (80.3kb to 53.2kb).