-
Notifications
You must be signed in to change notification settings - Fork 162
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
Upgrade webpack v4 → v5 #1520
Upgrade webpack v4 → v5 #1520
Conversation
8710447
to
2a30e49
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this work Victor -- nice to keep these things updated periodically!
It looks like it's working with auspice build && auspice view
but not auspice develop
. Have you tried building nextstrain.org and/or auspice.us with it? The extension architecture was quite tricky to get integrated with webpack & I could imagine won't work out-of-the-box with webpack v5.
When it comes time to merge, we should make sure we spend a bit of time testing on nextstrain canary to see if there are any strange bugs, especially cache-related ones.
ae8ce43
to
9c1df8b
Compare
👍 thanks Victor -- I'll re-review this before the end of the week |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the efforts here Victor! Building auspice itself seems to work well - I couldn't detect any regressions using the app - but dev mode + extensions are still broken.
9c1df8b
to
d44b47d
Compare
Split the changes into 3 PRs:
|
d44b47d
to
6b9fddf
Compare
6b9fddf
to
a8856bf
Compare
a8856bf
to
4fa98cb
Compare
I went through all webpack-related packages and looked for the earliest version that supports both webpack 4 and webpack 5. If no version matched that, then I picked the latest version that supports webpack 4. - clean-webpack-plugin: https://github.com/johnagan/clean-webpack-plugin/blob/v4.0.0/package.json#L50 - compression-webpack-plugin: https://github.com/webpack-contrib/compression-webpack-plugin/blob/v3.1.0/package.json#L41 - html-webpack-plugin: https://github.com/jantimon/html-webpack-plugin/blob/v4.5.2/package.json#L49 - No version that supports both webpack 4 and 5. This is the latest version that supports webpack 4. - lodash-webpack-plugin: https://github.com/lodash/lodash-webpack-plugin/blob/0.11.6/package.json#L46 - webpack-bundle-analyzer: https://github.com/webpack-contrib/webpack-bundle-analyzer/blob/v3.9.1/package.json#L89 - No version that supports both webpack 4 and 5. This is the latest version that supports webpack 4. Note that it is released as 3.9.0 on NPM. - webpack-dev-middleware: https://github.com/webpack/webpack-dev-middleware/blob/v3.7.3/package.json#L31 - webpack-hot-middleware: https://github.com/webpack-contrib/webpack-hot-middleware/blob/v2.25.2/package.json#L42 - No version that supports both webpack 4 and 5. This is the latest version that supports webpack 4. Command used: npm install --save \ [email protected] \ [email protected] \ [email protected] \ [email protected] \ [email protected] \ [email protected] \ [email protected]
4fa98cb
to
e4fec8e
Compare
From the migration doc¹: > If you have rules defined for loading assets using raw-loader, > url-loader, or file-loader, please use Asset Modules instead as > they're going to be deprecated in near future. ¹ https://webpack.js.org/migrate/5/
5592aec
to
33b5cbd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks victor - this stuff is a huge amount of work and involves so many testing iterations!
It's working well for me for auspice on it's own, but see above re: auspice.us. The only aspect I didn't test explicitly was the cacheGroups
chunk behavior (as they have longer cache max-ages, although I can't seem to find where we're setting this anymore 🤷 ); there's no obvious changes to these chunk groups here so it should be ok.
Context: in webpack v5, the files associated with the "core-vendors" custom group are classified as initial (entry-point) chunk files [1] when that was not the case in webpack v4. Not sure why. Add [name] to filename to be more consistent with chunkFilename, and so it's easier to understand the association between files and cacheGroups. Update the bundlesize config glob patterns to match these new names. Since the exact reason for filename vs. chunkFilename isn't clear to me, I updated all glob patterns to match either. Before: % ls -1 dist/*.js dist/auspice.bundle.9317bbd7a88738acbd84.js dist/auspice.bundle.a643293e8d8d26eb8631.js dist/auspice.chunk.106.bundle.db610e43983c64187457.js dist/auspice.chunk.145.bundle.4e68f1faa6878e1ced83.js dist/auspice.chunk.236.bundle.aafe143fec2bd894590f.js dist/auspice.chunk.383.bundle.79ae23992e50e870b00b.js dist/auspice.chunk.452.bundle.b75a285a5a08ef86511b.js dist/auspice.chunk.550.bundle.7a8bceea70ffde7edbf6.js dist/auspice.chunk.601.bundle.9a996464f910afbe8d17.js dist/auspice.chunk.718.bundle.8a109e6ba40b88251b95.js dist/auspice.chunk.815.bundle.2020e527c8ed3ebe83c5.js dist/auspice.chunk.971.bundle.a4099131fc6b718b0b0e.js dist/auspice.chunk.978.bundle.a3c6f896f16908bb753d.js dist/auspice.chunk.locales.bundle.49f1377f1c9b33ffa9c3.js dist/auspice.chunk.other-vendors.bundle.c38604b639c95e112558.js After: % ls -1 dist/*.js dist/auspice.chunk.106.bundle.db610e43983c64187457.js dist/auspice.chunk.145.bundle.4e68f1faa6878e1ced83.js dist/auspice.chunk.236.bundle.aafe143fec2bd894590f.js dist/auspice.chunk.383.bundle.79ae23992e50e870b00b.js dist/auspice.chunk.452.bundle.b75a285a5a08ef86511b.js dist/auspice.chunk.550.bundle.7a8bceea70ffde7edbf6.js dist/auspice.chunk.601.bundle.9a996464f910afbe8d17.js dist/auspice.chunk.718.bundle.8a109e6ba40b88251b95.js dist/auspice.chunk.815.bundle.2020e527c8ed3ebe83c5.js dist/auspice.chunk.971.bundle.a4099131fc6b718b0b0e.js dist/auspice.chunk.978.bundle.a3c6f896f16908bb753d.js dist/auspice.chunk.locales.bundle.49f1377f1c9b33ffa9c3.js dist/auspice.chunk.other-vendors.bundle.c38604b639c95e112558.js dist/auspice.core-vendors.bundle.209f45d95b5b37faa31c.js dist/auspice.main.bundle.9317bbd7a88738acbd84.js [1]: https://webpack.js.org/configuration/output/#outputchunkfilename
After upgrading webpack and plugins/loaders, there was an error from react-hot-loader: ReferenceError: module is not defined A known workaround is to enable safetyNet [1] so the code referencing `module` is not executed. I'm not sure why this was not shown before the webpack upgrades, but don't feel like digging deeper to find out. [1]: gaearon/react-hot-loader#1102 (comment)
33b5cbd
to
1d2825d
Compare
NPM 7 comes with a new lockfile version which we will be using for a more consistent and up-to-date developer experience. NPM 8 is also supported since it uses the same lockfile version. The lockfile was upgraded from a fresh environment (node v16.17.1, npm 8.15.0) using `npm ci --ignore-scripts && npm install --ignore-scripts`.
Now that NPM 7 tries to install peer dependencies, having an old version of react-addons-css-transition-group alongside a newer version of React will not work with `npm install` as it used to. However, simply upgrading the old package does not work since there are no newer versions compatible with React 16. Instead, it has been deprecated in favor of react-transition-group¹. Luckily, version 1 of the new package provides a drop-in replacement. Commands used²: npm uninstall react-addons-css-transition-group --legacy-peer-deps npm install react-transition-group@1 --legacy-peer-deps ¹ https://www.npmjs.com/package/react-addons-css-transition-group ² --legacy-peer-deps is necessary with NPM 7 until other peer dependency issues are resolved by future commits.
Now that NPM 7 tries to install peer dependencies, having an old version of css-loader alongside a newer version of webpack will not work with `npm install` as it used to. This is the earliest version of css-loader that supports the installed webpack version. Command used¹: npm info css-loader@3 peerDependencies npm install --legacy-peer-deps \ [email protected] ¹ --legacy-peer-deps is necessary with NPM 7 until other peer dependency issues are resolved by future commits.
Now that NPM 7 tries to install peer dependencies, having an old version of eslint alongside a newer version of @babel/eslint-parser will not work with `npm install` as it used to. This is the earliest version of eslint that is supported by the installed eslint-parser version. Command used¹: npm info @babel/[email protected] peerDependencies npm install --legacy-peer-deps \ [email protected] ¹ --legacy-peer-deps is necessary with NPM 7 until other peer dependency issues are resolved by future commits.
Now that NPM 7 tries to install peer dependencies, having an old version of these packages alongside a newer version of eslint will not work with `npm install` as it used to. These are the earliest versions that support the installed eslint version. Commands used¹: npm info eslint-config-airbnb@18 peerDependencies npm info eslint-plugin-jsx-a11y@6 peerDependencies npm info eslint-plugin-react-hooks@4 peerDependencies npm install \ [email protected] \ [email protected] \ [email protected]
The package version upgrades in previous commits introduced some new rules. Disabling them for now. Command used: npx eslint src --format json \ | jq -r '.[] | .messages[] | .ruleId' \ | sort | uniq
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is now working in auspice.us - thanks!
I'm going to merge without the regex fixup mentioned above preferring to move those to #1611
…kfile Upgrade to NPM 7/8, lockfile v2
Description of proposed changes
webpack 4 does not support Node.js v17 or above without the
--openssl-legacy-provider
option.Related issue(s)
Support for Node.js 18 (#1553) relies on this webpack upgrade.
Pre-merge tasks
Testing
auspice develop
and hot-reloading works (comment)auspice-client
works after pre-merge tasks (comment)Other notable discussion threads
auspice develop
(ref)