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

Investigate tree shaking of common js utilities #4964

Closed
2 tasks
Tracked by #3731
owenatgov opened this issue May 2, 2024 · 2 comments · Fixed by #4980
Closed
2 tasks
Tracked by #3731

Investigate tree shaking of common js utilities #4964

owenatgov opened this issue May 2, 2024 · 2 comments · Fixed by #4980
Assignees
Milestone

Comments

@owenatgov
Copy link
Contributor

What

Investigate ways to ensure that our common js utilities, primarily the multiple modules in common/index/mjs, are correctly tree-shaken when bundled. This includes but is not limited to splitting the modules in index.mjs into their own files.

Why

We found during #4957 that whilst we were able to effectively get tree shaking to work as expected for components, bundlers were importing all of common/index.mjs when not all of common was being used. It would be good to further optimise our js and understand why our "sideEffects": true solution in #4961 didn't work for common.

Who needs to work on this

Developers

Who needs to review this

Developers

Done when

  • Test if splitting out common/index.mjs fixes tree shaking
  • Optional: Investigate alternative methods to fix the tree shaking
@romaricpascal
Copy link
Member

romaricpascal commented May 13, 2024

The issue appears to be Webpack specific. For Rollup and Vite1, functions fromcommon/index.mjs that are not used by the Button component, like getFragmentFromURL, are properly absent from the built single-component.js.

Footnotes

  1. Vite was tested by building a single entry point to avoid the issue listed in https://github.com/alphagov/govuk-frontend/issues/4978 regarding how code is split by Vite automatically

@romaricpascal
Copy link
Member

Looks like the reason is that our Webpack configuration does not include the Terser plugin, used for removing dead code. Looking at Webpack's documentation about tree-shaking, it seems that tree-shaking is a two step thing for Webpack:

I'll raise a PR that amends our configuration to use the production mode, but not mangle class, function and variable names for testing.

@romaricpascal romaricpascal moved this from In progress 📝 to Needs review 🔍 in GOV.UK Design System cycle board May 13, 2024
@owenatgov owenatgov added this to the 5.4.0 milestone May 16, 2024
@36degrees 36degrees moved this from Needs review 🔍 to Done 🏁 in GOV.UK Design System cycle board May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

2 participants