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

Review Vite integration tests #4978

Closed
1 task
Tracked by #3731
romaricpascal opened this issue May 10, 2024 · 1 comment · Fixed by #4981
Closed
1 task
Tracked by #3731

Review Vite integration tests #4978

romaricpascal opened this issue May 10, 2024 · 1 comment · Fixed by #4981

Comments

@romaricpascal
Copy link
Member

What

Review Vite integration test to ensure each entry is built independently. If this means having two separate scripts that each build an entry, that's fine.

Why

Vite seems to treat the two entries we configured for building as part of a single thing, and extracts shared code in its own module. This would make sense if we were building a single project with multiple entry points. However, what we want to do is check that each of our entry points, when build on its own, includes what we expect. Because Vite splits the shared code in its own file, we can't verify that tree-shaking works by only looking in the build initAll.mjs or single-component.mjs, as some code in the shared module may not have been tree-shaken, or may be there because initAll imports it.

Who needs to work on this

Developers

Who needs to review this

Developers

Done when

  • Vite builds the two entries independently
@romaricpascal
Copy link
Member Author

Digging further, Rollup (which is what Vite uses during the build), automatically creates chunks (separate files imported by the "main" bundle) when it notices shared code across entries. This leads to a button-[hash].js file to be created with the code for the Button and its depedencies, that is then imported by the built initAll.js and single-component.js. To properly validate that tree-shaking works, we'd want that code bundled with initAll.js and single-component.js so that:

  • we have only one place to look into and don't miss that a component gets mistakenly included in the shared chunk without us noticing
  • results are not skewed by what Rollup deems shared by the two modules, which is ultimately the responsibility of the consumer code.

Library mode

Vite offers a Library mode, which defines the entries outside of the build.rollupOptions. Unfortunately, it doesn't change the behaviour regarding code splitting, and a shared button-[hash].js file ends up being created as well.

Rollup's manualChunks option

Rollup offers a manualChunks option to create custom chunks. Unfortunately, it only creates new chunks and doesn't disable the automatic creationg of chunks for shared code amongst entries. Indeed, neither undefined or {} prevented the creation of the chunk.

Further investigation led to discovering that disallowing automatic creation of chunks is not something that Rollup supports. And because it's wrapped inside Vite, we do not have the option of providing an array of RollupConfiguration to make build each entry independently, as we do for the build of govuk-frontend

This behaviour is unfortunately not controlled by the looks like Rollup doesn't allow todisable the automatic creation of chunks , which is a shame. Because it's wrapped inside Vite, we do not have the option of providing an array of RollupConfiguration to build each entry independently.

If it's Rollup under the hood, do we need to look at Vite

While we do have tests for Rollup already, Vite has in mind to switch to a Rust port of Rollup (called Rolldown) in the future, so I'd say it's worth keeping an eye on Vite specifically.

What to do then?

Easiest is to have separate config files, as well as scripts (because vite cleans the output directory) and entry in the GitHub action matrix for each entry we want to build. For this, I've opened #4981.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done 🏁
Development

Successfully merging a pull request may close this issue.

1 participant