Webpack 4 followup

[gaearon]

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?)
• Reload times in DEV are still pretty slow

[marcofugaro]

Reload times in DEV are still pretty slow

Maybe we could try the autodll-webpack-plugin?

They're still working on supporting webpack 4

It will be deprecated when webpack5 comes out

[miraage]

Regarding Can we stop building runtime as a separate file?.

Docs:
https://webpack.js.org/configuration/optimization/#optimization-runtimechunk

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.

[andriijas]

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]

I agree it’s good to separate it from the chunks but couldn’t it be always inlined in the HTML?

[miraage]

It could be. AFAIK https://github.com/numical/script-ext-html-webpack-plugin provides an option to inline a certain script into HTML.

[clearjs]

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]

cc @sokra @TheLarkInn

[Graham42]

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.

[mlwilkerson]

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.

[marcofugaro]

Also I think the react-dev-utils/formatWebpackMessages.js needs to be updated, did anyone test it with the warnings?

[andriijas]

@marcofugaro which warnings are you referring to? thanks

[marcofugaro]

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

[andriijas]

Yeah this one probably slipped through due to the skeleton app complying to all eslint rules. Sorry about that. Would you like to make a pr @marcofugaro

[marcofugaro]

Sure thing! I'll get my hands dirty when I have time in the next few days. Thanks @andriijas

[marcofugaro]

Ok, I investigated, the PR is the #4656, I noticed it doesn't happen with webpack v4.8 but with newer versions.

[4iAmAve]

I got CRA running with webpack 4 after an eject and two forks (InterpolateHTMLPlugin and WatchMissingNodeModulesPlugin). See here

Maybe, it helps you.

[sokra]

Since runtime changes rarely

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.

[johdah]

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]

@johdah We'd need a minimal reproducing example.