Switch from webpack, babel, and jest to vite and vitest #3031
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is the first of a number of commits to switch the Dashboard
from using webpack + babel to using vite. This will simplify
maintenance as there are significantly fewer dependencies involved,
and should also provide some performance and other benefits of a
more modern toolset with updated defaults for modern browsers.
This initial commit gets the application building and running in
both dev and production modes, but results in tests and a number of
other scripts being broken. They will be addressed in subsequent
commits, to be merged in the same PR.
- Install vite and related plugins
- Add base vite config including replacement for webpack-dev-server,
and production minification (using defaults for now)
- Rename all files containing JSX to have a .jsx extension
This is required by vite by default, and helps with performance
as it only runs the React / JSX processing on those files
- Update format of YAML files to conform to expectations of the YAML plugin
- Move the HTML template and index file to the root as expected by vite
- Replace use of `export default from …` with `export { default } from …`
which is more standard and doesn't require additional config / plugins
- Replace use of `process.env` with `import.meta.env`
- Replace webpack magic comments for dynamic imports
- Update npm scripts to use vite instead of webpack for building
- Add new script for enabling pseudo-locale for i18n testing
- Add new preview script to serve the production bundles from the
dev server, allowing for quicker iteration (reduces need to build
and deploy the Dashboard backend during development)
This is part of a series of commits switching from webpack and babel to vite. Tests and some other scripts are broken with this change, but will be addressed in subsequent commits. - Remove all dependencies and devDependencies related to webpack and babel - Remove all config files related to webpack and babel
Swap the webpack bundle analyzer for a vite-compatible one. It over-reports the module sizes as its data is provided before vite's final minification and some other optimisations, but is still useful to keep an eye on the relative size of modules included in the bundles.
- Update to latest Storybook release - Add dependencies and config required to work with vite as the builder - Update Graph component imports to resolve bug caused by subtle difference in module resolution in vite vs. webpack. Without this change the new Graph story would actually load the legacy Graph component and as a result would not render correcntly.
There are some compatibility issues with jest + vite, for example, a common package providing support between the two packages does not currently work on Windows. Switch to vitest instead, which is mostly compatible with jest's approach and syntax. `jest.fn`, `jest.spyOn`, and similar are replaced with their vitest equivalents, `vi.fn`, `vi.spyOn`, etc. With this change there are a small number of failing tests. They will be addressed in a subsequent commit to ease review. Vitest will automatically detect when it's running in CI and run in non-interactive mode so there's no need for our `test:ci` script anymore, we can use `npm test` for both local dev and CI. Continue using Istanbul for code coverage. The v8 option (using c8) is faster but works differently, has some limitations at the moment, and produces quite different results. We can review at a later date if necessary, but there difference in performance is quite small, on the order of a few seconds. Remove jest mock files and custom loaders as they're no longer needed.
The switch to vitest uncovered an issue with the way we were loading the YAML language file for CodeMirror which caused the tests to break due to multiple versions of an internal module being loaded. Update to use the newer approach for loading the language file which also fixes the tests.
- Run `lint:fix` to fix formatting - DIsable some new noisy rules now that we're enabling es2021 With this change there's 1 remaining error in the extractMessages.js script which will be resolve in a subsequent commit that rewrites that script to remove remaining references to babel.
tekton-robot
added
kind/misc
- Remove unnecessary wrapper object in message bundles - Replace config.json with .env files - Add new script to run in 'i18n mode' with all build locales supported - Update dev readme with new scripts and config files - Update message extraction script to use formatjs cli instead of babel - Add formatjs linter
7 tasks
briangleeson
approved these changes
Aug 11, 2023
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: briangleeson The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tekton-robot
added
the
approved
7 tasks
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Replaces #3030, does not include changes to lazy-load the YAMLEditor, add
nvm, or extract the commonLoadingcomponent. They will be handled in separate PRs after this is merged.PR contains multiple commits for easier review, but all are required to pass CI jobs.
Summary of changes:
i18n:extractscript for externalising messagesEach commit has a more detailed commit message explaining the changes and some of the choices that were made.
This significantly reduces the number of dependencies installed.
Before (output of
npm ci):added 2102 packages, and audited 2107 packages in 5mAfter:
added 1654 packages, and audited 1659 packages in 4mAlso looking at recent CI integration tests, the build time has gone from ~2 minutes to ~1 minute. This is from a small sample, but seems consistent so far.
/kind misc
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
functionality, content, code)
/kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes