Infrastructure: Update build tooling to use webpack v5 #5792
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
swissspidy
added
Type: Infrastructure
|
Size Change: -1.33 MB (-33%) 🎉 Total Size: 2.75 MB
ℹ️ View Unchanged
|
swissspidy
force-pushed
the
chore/update-webpack
branch
from
08aa268
to
d2f8fa3
Compare
swissspidy
force-pushed
the
chore/update-webpack
branch
from
d2f8fa3
to
8af1aec
Compare
Only the 5.0.0-alpha version is compatible with webpack 5, and it does not support the alternative usage we’ve been using anymore. Reverts #2887 because of that.
`process` is no longer defined in webpack 5
Used by React Testing Library. Since webpack 5 does no longer polyfill `process`, we need to to that ourselves.
Needed because webpack 5 no longer polyfills `global` to be `window`.
swissspidy
force-pushed
the
chore/update-webpack
branch
from
1ffd907
to
8af1aec
Compare
This comment has been minimized.
This comment has been minimized.
|
Added some SVG docs but lmk if there's a better place for this. Didn't see any webpack-specific docs so figured adding a new file may make sense. |
swissspidy
changed the title
Did a first pass locally looking good. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
samwhale
approved these changes
Feb 28, 2022
There was a problem hiding this comment.
Storybook and editor are working. I'm seeing this warning in the logs after uploading an image in the editor.
Not sure why webpack is looking for the sourcemap to the worker script.
💭 Should we add the worker script as another ignored file for the sourcemap loader?
* add removed empty line in story-editor/package.json * remove unneeded named export in svgrMock * remove unneeded comment
swissspidy
commented
Feb 28, 2022
.storybook/main.cjs
Outdated
| @@ -124,7 +124,6 @@ module.exports = { | |||
| ); | |||
|
|
|||
| // Ensure SVGR is the only loader used for files with .svg extension. | |||
| // TODO: Figure out why this is needed; only the first-matching rule ought to be used. | |||
There was a problem hiding this comment.
@samwhale So was my assumption incorrect that webpack uses only the first-matching rule?
There was a problem hiding this comment.
Oh sorry! I thought I added this line in, and I thought we didn't need it anymore so I deleted it.
webpack was matching all of the svg rules, so I grouped the svg rules together in a oneOf block: https://github.com/GoogleForCreators/web-stories-wp/pull/5792/files#diff-b724a559a3d863e949957e731407f96e486b45139afbdee10bc1082f5bd229e3R112-R117
Found webpack-contrib/worker-loader#245 for this. They pointed to an open Chromium bug. So yeah sounds easiest to just ignore it & point to that ticket. |
Looks like it's not related to the source-map-loader! 😬 I'll take a look at possibly disabling source maps in the worker loader. Not sure if that would be possible from there since it seems to be related to the browser settings specifically. |
|
If it's too complicated we can do it in a follow-up ticket. |
💭 I think it won't happen when the worker is loaded in without the worker-loader. Happy to add a ticket to fix it, but maybe it wouldn't be worth it since it'll be fixed in #10687? |
timarney
approved these changes
Mar 2, 2022
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.
Summary
Updates webpack from v4 to v5 which contains a ton of major changes, which means there are major changes to our build tooling as well.
On the plus side, there are significant bundle size and build performance improvements.
Many webpack plugins haven't kept up yet, so this PR is a draft for the time being.
webpack 5 migration guide: https://webpack.js.org/migrate/5/
storybook + webpack 5 migration guide: https://storybook.js.org/blog/storybook-for-webpack-5/
Relevant Technical Choices
Prevent stylis plugin warnings
Stylis has been throwing warnings because the stylis RTL plugin doesn't have a "name" (anymore?). Fixed this by adding
Object.defineProperty(stylisRTLPlugin, 'name', { value: 'stylisRTLPlugin' });everywhere, which seems to be the recommended approach.Replace usage ofglobal.*withwindow.*This was split out into Don't use node globals in browser context #10487
Replace usage ofprocess.envwith custom variablesThis was split out into Don't use node globals in browser context #10487
Replaces
optimize-css-assets-webpack-pluginwithcss-minimizer-webpack-pluginThe former does not support webpack 5, and recommends the latter going forward.
Replaces
url-loaderandfile-loaderwith asset modulesThis changes the way SVG imports are written.
Replaces
expose-loaderwith themangleExportsoptionUpdates webpack config to remove deprecated fields and maintain existing behavior
The webpack config schema has been updated significantly. This PR removes things like
jsonpFunction(deprecated) and adds things likeresolve.fullySpecified(allows importing ES modules without file extensions)Changes how Karma tests are run⚠️
karma-webpackneeded to be updated to its next major version (5.0.0) in order to support webpack 5. This meant changing the way we run tests, because the "alternative usage" option we've been following is no longer supported. This essentially reverts Karma: use a single webpack bundle for all tests #2887. However, I don't see a negative performance impact. On the plus side: this should allow running individual tests again.Update storybook to use webpack 5
Since there are still upstream issues like Webpack5 + type: module = require is not defined storybookjs/storybook#14877, this contains a couple of workarounds that we can hopefully remove soon.
Addresses various deprecation warnings and other minor code quality issues
To-do
User-facing changes
N/A
Testing Instructions
Verify that all tests still pass, and editor still works.
Storybook should work.
SVG icons should look fine.
Fixes #6304