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

Refactor Intl polyfills and remove from ES5 entrypoints #16349

Merged
merged 2 commits into from
May 4, 2023

Conversation

steverep
Copy link
Member

Proposed change

This change drastically reduces the entrypoint sizes for the ES5 build by removing the FormatJS Intl polyfills and loading them with the same timing logic as the latest build. It also greatly simplifies use of Intl where desired by only requiring a single import. Steps are roughly:

  1. Split the dynamic import and top level await version for the latest build into its own module. Also split out the locale data polyfill function so there's no circular dependency.
  2. Move the TLA to the new intl-polyfill module, and just write code to import it before using Intl.
  3. Split the static import version for the ES5 build as well. Code is written for latest build, but Webpack makes the swap for the ES5 build.

And here's the result on the ES5 build:

ES5 Entrypoint Old New
app 582K 596K
core 2.1M 345K
custom-panel 2.0M 317K
authorize 2.5M 859K
onboarding 2.7M 1.1M
Total Bundle 36M 32M

And as a happy accident, the latest build entries go down a little too. This is because the connection mixin is no longer importing the localize module early:

Latest Entrypoint Old New
app 315K 293K
core 19K 19K
custom-panel 35K 35K
authorize 388K 367K
onboarding 417K 395K
Total Bundle 26M 26M

I mean... I knew that was going to happen. 🦊

This may actually fix the Webpack upgrade issue as well because that's reportedly related to conditional TLAs, which are eliminated here. I'll have to check that out.

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:

@steverep steverep added the Build Related to building the code label Apr 29, 2023
@steverep
Copy link
Member Author

I'm adding 2 corrections here:

  1. Dynamic imports should use the polyfill-force options so as not to repeat the shouldPolyfill checks.
  2. English locale data must be imported after the corresponding polyfill.

There are other corrections/improvements that I noticed could be made, but I'll save those for a follow up and list in an issue before I forget.

Also, these polyfills are still plaguing the supervisor entrypoints (see #15956). Some extra savvy is needed to fix that.

@steverep
Copy link
Member Author

steverep commented May 1, 2023

Did you want to wait on 2023.6 for this one?

@bramkragten
Copy link
Member

bramkragten commented May 1, 2023

Did you want to wait on 2023.6 for this one?

yeah, I'll merge it on thursday if that doesn't stop your from continuing something else? Otherwise we can also merge now, then I'll just cherry pick

@steverep
Copy link
Member Author

steverep commented May 1, 2023

There's more that requires this, but also plenty to do that doesn't. No big deal to wait or just work with a stash. 👍🏻

@steverep
Copy link
Member Author

steverep commented May 1, 2023

FYI, this does indeed fix the ability to upgrade Webpack (ref: webpack/webpack#16097).

@steverep steverep mentioned this pull request May 2, 2023
9 tasks
@bramkragten bramkragten merged commit d2321b5 into dev May 4, 2023
6 checks passed
@bramkragten bramkragten deleted the remove-intl-from-entry branch May 4, 2023 13:49
@steverep steverep linked an issue May 5, 2023 that may be closed by this pull request
3 tasks
@github-actions github-actions bot locked and limited conversation to collaborators May 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Build Related to building the code cla-signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Supervisor entrypoints are too big ?
2 participants