-
-
Notifications
You must be signed in to change notification settings - Fork 59
Upgrade to webpack 5. #216
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
Conversation
| libraryTarget: "system", | ||
| path: path.resolve(process.cwd(), "dist"), | ||
| jsonpFunction: `webpackJsonp_${opts.projectName}`, | ||
| // jsonpFunction: `webpackJsonp_${opts.projectName}`, |
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 option was removed. Not sure if it was renamed to something else. We should look into this more.
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.
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.
From what I read awhile ago, it’s based on the name field in package.json
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.
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.
Ah thanks I'll switch to that.
| jsonpFunction: `webpackJsonp_${opts.projectName}`, | ||
| // jsonpFunction: `webpackJsonp_${opts.projectName}`, | ||
| devtoolNamespace: `${opts.projectName}`, | ||
| publicPath: "", |
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.
webpack@5 has an "auto" public path option. unfortunately it doesn't work with loading via systemjs right now. Since "auto" is the new default for publicPath, I had to specify the public path as an empty string so that the public path comes from systemjs-webpack-interop instead of the broken "auto" functionality
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.
What makes auto incompatible with systemJs?
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.
didn't look into it
| ], | ||
| }, | ||
| devtool: "sourcemap", | ||
| devtool: "source-map", |
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.
webpack 5 renamed this
| "Access-Control-Allow-Origin": "*", | ||
| }, | ||
| disableHostCheck: true, | ||
| firewall: false, |
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.
| disableHostCheck: true, | ||
| firewall: false, | ||
| client: { | ||
| port: 8080, |
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.
sockPort was removed - this is the equivalent.
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.
It's much much better not to have to hard code the port here, but let the user specify it via CLI arg - we likely will need to fix webpack-dev-server in order for it to support this. It's one of the things holding up this PR
| new BundleAnalyzerPlugin({ | ||
| analyzerMode: webpackConfigEnv.analyze ? "server" : "disabled", | ||
| }), | ||
| new UnusedFilesWebpackPlugin({ |
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 plugin doesn't work with webpack 5 (rip). See tomchentw/unused-files-webpack-plugin#39
|
The main thing holding up this PR is that it relies on webpack-dev-server@4, which is unreleased |
|
Ah it looks like [email protected] was just released! I'll try to get this PR ready to merge then - there is still the issue of having to specify port. |
|
This is now fully updated and the tests should (hopefully) pass - the only thing remaining is the port issue. I plan on making a pull request to webpack-dev-server fixing it. |
|
The webpack-dev-server issue is here: webpack/webpack-dev-server#2873 |
|
I created a PR to webpack-cli which will resolve the port issue: webpack/webpack-cli#2147 Thumbs up on the PR are appreciated - webpack-cli is a hard project to get code merged into. |
|
I think we can merge this and publish a beta of create-single-spa with it, noting the caveat when using default port. I'll mark this as ready for review |
|
This is released in https://github.com/single-spa/create-single-spa/releases/tag/v2.0.0-beta.0, which includes instructions for upgrading existing projects. |
This is a WIP.
The steps that each project needs to take to upgrade:
yarn add webpack@latest webpack-cli@latest webpack-dev-server@https://github.com/webpack/webpack-dev-server#v4webpack-dev-servertowebpack --serveWe should probably wait for webpack-dev-server@4 before releasing this because we need the fix in webpack/webpack-dev-server#2839, which will be released once webpack/webpack-dev-server#2592 is merged and released.
We should also look into why the devServer.client.port option has to be specified - ideally it wouldn't be.