Don't mutate stats configuration (webpack 3.8.0+ compatibility) #1174
Conversation
The webpack configuration `stats` property has the same format as the `devServer.stats` property, so the same object is frequently reused to define the same stats output. Since webpack v3.8.0, the `stats` property is validated more strictly by webpack. webpack-dev-server is mutating this `stats` object by adding a `colors` property, which is an object and doesn't match the webpack configuration schema, causing a fatal error. The easy fix is to *not* mutate the `stats` object, so webpack isn't breaking with a fatal error. But we may also consider setting a valid `colors` property.
Codecov Report
@@ Coverage Diff @@
## master #1174 +/- ##
=======================================
Coverage 76.31% 76.31%
=======================================
Files 5 5
Lines 477 477
Branches 154 154
=======================================
Hits 364 364
Misses 113 113Continue to review full report at Codecov.
|
| @@ -294,7 +294,9 @@ function processOptions(webpackOptions) { | |||
| }; | |||
| } | |||
|
|
|||
| if (typeof options.stats === 'object' && typeof options.stats.colors === 'undefined') { options.stats.colors = argv.color; } | |||
| if (typeof options.stats === 'object' && typeof options.stats.colors === 'undefined') { | |||
| options.stats = Object.assign({}, options.stats, { colors: argv.color }); | |||
There was a problem hiding this comment.
I may be having a brain fart here, so please correct me if I'm wrong. this doesn't look like it'll avoid mutating the stats property. there's a larger issue here (which is resolved in the v3 beta branch) of the options passed being mutated on the whole:
var a = { stats: {} };
function doit (o) {
o.stats = Object.assign({}, a.stats, { color: '0' });
}
doit(a);
a;
// returns
// {stats: {…}}
// stats: {color: "0"}
// __proto__: ObjectI could be missing something, but I think the solution is to prevent the top level options object passed to devServer from being mutated.
There was a problem hiding this comment.
You are right, but it's the stats property value that shouldn't be mutated.
var stats = {};
var a = { stats: stats };
function doit (o) {
o.stats = Object.assign({}, a.stats, { color: '0' });
}
doit(a);
stats;
// {}There was a problem hiding this comment.
I see, I see. Hadn't considered that case. Could you put a simple test for this in /test?
There was a problem hiding this comment.
Sadly I cannot write a good test case because when running test/cli.test.js, the package supports-color detect that it doesn't run in a tty, so it returns false (which webpack sees as valid).
|
@BenoitZugmeyer thanks for your patience on this one. It's now merged. |
What kind of change does this PR introduce?
Bugfix
Did you add or update the
examples/?No
Summary
The webpack configuration
statsproperty has the same format as thedevServer.statsproperty, so the same object is frequently reused to define the same stats output.Since webpack v3.8.0, the
statsproperty is validated more strictly by webpack (see webpack/webpack@e0d4501#diff-c6d9e330d5284ccaee0ea220907a2280 and webpack/webpack@5761875#diff-c6d9e330d5284ccaee0ea220907a2280). webpack-dev-server is mutating thisstatsobject by adding acolorsproperty, which is an object and doesn't match the webpack configuration schema, causing a fatal error:The easy fix is to not mutate the
statsobject, so webpack isn't breaking with a fatal error. But we may also consider setting a validcolorsproperty.Does this PR introduce a breaking change?
No
Other information
Please see this minimal project to reproduce: https://gist.github.com/BenoitZugmeyer/29810768cc0f7a4413265d285a160027