You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Thanks to some titanic efforts from @andriijas, @bugzpodder and others we finally have Webpack 4 in master. Hopefully CI will pass.
Let's use this task to keep track of followup things to do? At the very least this looks suboptimal to me:
Why are we shipping a few hundred bytes of the runtime as a separate chunk instead of e.g. embedding it into the HTML? I suppose it would always be small so we could avoid the extra request.
I also think #4490 disabled scope hoisting (?) so we might want to keep track of what's necessary to get it enabled again.
Was there anything else?
Can we stop bundling runtime as a separate file?
When can we enable scope hoisting (and do we need to?)
Values of true and single are equal for projects with a single entry point.
I think it is still worth to keep it separated because this part of the code won't change unless webpack version is updated.
I agree it looks verbose on newly created apps - we could document better the purpose of the runtime though. My understanding is that its better to have the runtime separated when you start to divide your app into multiple chunks (clients only need to download updated chunks when you deploy)
Since runtime changes rarely, serving it with HTML each time would be a waste. Currently we can enable caching and only download it once, and this happens while the other chunks are being downloaded. This also slightly reduces initial HTML download time, so requests for other bundles start earlier. It can also be pushed with HTTP/2 if needed.
Reacted by Tom Crockett, Mike Morris, Graham McGregor and Jeroen Visser
A heads up about inlining scripts. This would be a breaking change for people who are using a strong Content Security Policy (CSP) that blocks all inline scripts.
There's a big performance regression that impacts Webpack 4, which is a concern for our Font Awesome icon packs and tree-shaking. See this issue against terser, the new uglify-es.
Webpack 4.11.0 depends upon uglify-es@3.3.9, which exhibits the perf regression. Right now, create-react-app devs won't feel the pain because they're on Webpack 3 whose uglify-js dependency performs as expected.
Once CRA moves to Webpack 4, unless this perf regression has been fixed, devs using Font Awesome will be frustrated. And apparently, the only ways for them to resolve the problem will be to side-step that version of CRA: either downgrade to a CRA that uses Webpack 3, or eject from CRA and modify their Webpack configs.
So I'm wincing, and hoping this gets resolved in Webpack 4's dependencies before CRA comes out with Webpack 4.
Hey @andriijas! The stats.toJson().warnings, for example when you type something that does not respect an eslint rule which is set as warning. (This happens thanks to the eslint-loader).
I tested the output of formatWebpackMessages and it outputs only the filename, not the actual text, but maybe I'm doing something wrong, could you help me test it?
Also the parameter of the toJson() function here are not anymore necessary, here are the docs
This assumption is not true, the runtime is the chunk that changes most often. The runtime contains a mapping from chunk id to chunk filename (which contains a hash of the content). That's why it always changes when any other chunk changed.
That's usually the reason why you opt-in to a separate runtime chunk which is inlined. But this is not required to do. By default the runtime is put into the app chunk. So without separate runtime chunk your app chunk will change on any other chunk change. If you app chunk isn't too big this can be acceptable and better than a separate runtime chunk.
Webpack 4.11.0 depends upon uglify-es@3.3.9, which exhibits the perf regression. Right now, create-react-app devs won't feel the pain because they're on Webpack 3 whose uglify-js dependency performs as expected.
Once CRA moves to Webpack 4, unless this perf regression has been fixed, devs using Font Awesome will be frustrated. And apparently, the only ways for them to resolve the problem will be to side-step that version of CRA: either downgrade to a CRA that uses Webpack 3, or eject from CRA and modify their Webpack configs.
So I'm wincing, and hoping this gets resolved in Webpack 4's dependencies before CRA comes out with Webpack 4.
You should use the terser-webpack-plugin if possible. webpack won't change the default for v4 because it would be a breaking change. We switch to terser for v5. see optimization.minimizer
Also the parameter of the toJson() function here are not anymore necessary, here are the docs
You can pass arguments to the toJson call to limit the fields. i. e. toJson({all: false, warnings: true, errors: true }). This is more performant, since constructing the stats object is expensive.
Reacted by Andreas Cederström, Marco Fugaro, Joe Haddad and Mike Wilkerson
I get "Super expression must either be null or a function, not undefined" when I am building to production but not when I am running locally. That external library works fine with our current webpack 2 setup. So it feels like a problem webpack 4 maybe introduced.
I have seen others solving that by setting "keep_fnames: true" in the uglify settings. But that didn't solve it for me when ejecting and trying that setting. airbnb/react-with-styles#151
Activity
marcofugaro commentedon May 21, 2018
Maybe we could try the autodll-webpack-plugin?They're still working on supporting webpack 4It will be deprecated when webpack5 comes out
miraage commentedon May 21, 2018
Regarding
Can we stop building runtime as a separate file?.Docs:
https://webpack.js.org/configuration/optimization/#optimization-runtimechunk
Values of
trueandsingleare equal for projects with a single entry point.I think it is still worth to keep it separated because this part of the code won't change unless webpack version is updated.
andriijas commentedon May 21, 2018
I agree it looks verbose on newly created apps - we could document better the purpose of the runtime though. My understanding is that its better to have the runtime separated when you start to divide your app into multiple chunks (clients only need to download updated chunks when you deploy)
gaearon commentedon May 21, 2018
I agree it’s good to separate it from the chunks but couldn’t it be always inlined in the HTML?
miraage commentedon May 21, 2018
It could be. AFAIK https://github.com/numical/script-ext-html-webpack-plugin provides an option to inline a certain script into HTML.
clearjs commentedon May 21, 2018
Since runtime changes rarely, serving it with HTML each time would be a waste. Currently we can enable caching and only download it once, and this happens while the other chunks are being downloaded. This also slightly reduces initial HTML download time, so requests for other bundles start earlier. It can also be pushed with HTTP/2 if needed.
gaearon commentedon May 21, 2018
cc @sokra @TheLarkInn
Graham42 commentedon May 22, 2018
A heads up about inlining scripts. This would be a breaking change for people who are using a strong Content Security Policy (CSP) that blocks all inline scripts.
My preference would to be avoid inlining.
Fix scope hoisting (#152)
mlwilkerson commentedon Jun 7, 2018
There's a big performance regression that impacts Webpack 4, which is a concern for our Font Awesome icon packs and tree-shaking. See this issue against
terser, the newuglify-es.Webpack 4.11.0 depends upon
uglify-es@3.3.9, which exhibits the perf regression. Right now,create-react-appdevs won't feel the pain because they're on Webpack 3 whoseuglify-jsdependency performs as expected.Once CRA moves to Webpack 4, unless this perf regression has been fixed, devs using Font Awesome will be frustrated. And apparently, the only ways for them to resolve the problem will be to side-step that version of CRA: either downgrade to a CRA that uses Webpack 3, or eject from CRA and modify their Webpack configs.
So I'm wincing, and hoping this gets resolved in Webpack 4's dependencies before CRA comes out with Webpack 4.
marcofugaro commentedon Jun 12, 2018
Also I think the
react-dev-utils/formatWebpackMessages.jsneeds to be updated, did anyone test it with the warnings?andriijas commentedon Jun 12, 2018
@marcofugaro which warnings are you referring to? thanks
marcofugaro commentedon Jun 12, 2018
Hey @andriijas! The
stats.toJson().warnings, for example when you type something that does not respect an eslint rule which is set as warning. (This happens thanks to the eslint-loader).I tested the output of
formatWebpackMessagesand it outputs only the filename, not the actual text, but maybe I'm doing something wrong, could you help me test it?Also the parameter of the toJson() function here are not anymore necessary, here are the docs
2 remaining items
marcofugaro commentedon Jun 20, 2018
Ok, I investigated, the PR is the #4656, I noticed it doesn't happen with webpack v4.8 but with newer versions.
4iAmAve commentedon Sep 17, 2018
I got CRA running with webpack 4 after an eject and two forks (InterpolateHTMLPlugin and WatchMissingNodeModulesPlugin). See here
Maybe, it helps you.
sokra commentedon Sep 17, 2018
This assumption is not true, the runtime is the chunk that changes most often. The runtime contains a mapping from chunk id to chunk filename (which contains a hash of the content). That's why it always changes when any other chunk changed.
That's usually the reason why you opt-in to a separate runtime chunk which is inlined. But this is not required to do. By default the runtime is put into the app chunk. So without separate runtime chunk your app chunk will change on any other chunk change. If you app chunk isn't too big this can be acceptable and better than a separate runtime chunk.
You should use the
terser-webpack-pluginif possible. webpack won't change the default for v4 because it would be a breaking change. We switch to terser for v5. seeoptimization.minimizerYou can pass arguments to the
toJsoncall to limit the fields. i. e.toJson({all: false, warnings: true, errors: true }). This is more performant, since constructing the stats object is expensive.johdah commentedon Sep 22, 2018
I get "Super expression must either be null or a function, not undefined" when I am building to production but not when I am running locally. That external library works fine with our current webpack 2 setup. So it feels like a problem webpack 4 maybe introduced.
I have seen others solving that by setting "keep_fnames: true" in the uglify settings. But that didn't solve it for me when ejecting and trying that setting.
airbnb/react-with-styles#151
gaearon commentedon Sep 22, 2018
@johdah We'd need a minimal reproducing example.
Optimize webpack rebuild speed
Optimize webpack rebuild speed (#5065)
Optimize webpack rebuild speed (react#5065)