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

Allow TLA for legacy build and remove static Intl polyfills #19207

Merged
merged 3 commits into from
Jan 8, 2024

Conversation

steverep
Copy link
Member

Proposed change

Webpack's issue with TLA having ES5 support, webpack/webpack#11874, is likely never to get fixed, so I wrote a fairly simple plugin to do the job using Babel: https://github.com/steverep/transform-async-modules-webpack-plugin.

So this change gets rid of the static imports for the Intl polyfills, and now technically allows any TLA use. However, that comes with a big hiccup for now. Note that even though the change removes ~2MB of locale and time zone data from the bundle, the bundle size actually goes significantly up.

this is because the plugin does not make use of @babel/runtime and instead includes the helpers and regenerator inline. Because of duplicates from code splitting and the fact that every module that imports the polyfill is affected, that results in a lot of bytes. (Recall the drastic reductions from #16466.)

I think I can make the plugin use @babel/runtime, but it makes it much more complex. I could probably also mess with Webpack settings to reduce that duplication in a follow up. I think it's probably still worth merging now for browsers at the top of the legacy spectrum though since there would still be deferral benefits and some polyfill chunks would never get loaded.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue or discussion:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

@github-actions github-actions bot added Dependencies Pull requests that update a dependency file Build Related to building the code labels Dec 31, 2023
@steverep steverep force-pushed the fix-webpack-tla-for-intl branch from 9de6ac0 to 32662ac Compare December 31, 2023 18:58
bramkragten
bramkragten previously approved these changes Jan 2, 2024
Copy link
Member

@bramkragten bramkragten left a comment

Choose a reason for hiding this comment

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

Nice! 🚀

@steverep steverep force-pushed the fix-webpack-tla-for-intl branch from 8666ce4 to 6bd1439 Compare January 2, 2024 18:00
@steverep
Copy link
Member Author

steverep commented Jan 2, 2024

Took me like an hour just to fix the conflict because all my GPG keys expired 😮‍💨

@steverep
Copy link
Member Author

steverep commented Jan 2, 2024

re: your confusion... couldn't sign my commits

@steverep steverep removed the Dependencies Pull requests that update a dependency file label Jan 7, 2024
@bramkragten bramkragten merged commit ae79df8 into dev Jan 8, 2024
13 checks passed
@bramkragten bramkragten deleted the fix-webpack-tla-for-intl branch January 8, 2024 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Related to building the code cla-signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants