fix(stats): allow stats to respect array of configs, also bump deps. #3524

Merged
merged 10 commits into from Jan 11, 2017

Projects

None yet

3 participants

@TheLarkInn
Member

What kind of change does this PR introduce?
Bugfix

Did you add tests for your changes?
Yes

If relevant, link to documentation update:

Summary

Fixes #3512

Does this PR introduce a breaking change?
No

Other information

@TheLarkInn TheLarkInn fix(stats): allow stats to respect config for MultiCompiler, MultiStats
a4106ea
@TheLarkInn TheLarkInn changed the title from fix(stats): allow stats to respect config for MultiCompiler, MultiStats to fix(stats): allow stats to respect array of configs, also bump deps. Dec 19, 2016
bin/webpack.js
@@ -160,11 +169,20 @@ function processOptions(options) {
return;
}
+ // var firstOptions = Array.isArray(options) ? (options[0] || {}) : options;
@sokra
sokra Dec 20, 2016 Member

remove

bin/webpack.js
+
+ if(Array.isArray(options)) {
+
+ options = options.map(function(singleOption) {
@sokra
sokra Dec 20, 2016 Member

a forEach could do the same job.

bin/webpack.js
+ options = options.map(function(singleOption) {
+ singleOption.stats = statsPresetToOptions(singleOption.stats);
+ return singleOption;
+ })
@sokra
sokra Dec 20, 2016 Member

; missing, but i wonder why this is not catched by the linting...

@TheLarkInn TheLarkInn chore(stats): remove comment, add semicolon, swap map with forEach
ce49851
bin/webpack.js
+ });
+ } else {
+ options.stats = statsPresetToOptions(options.stats);
+ }
}
var outputOptions = Object.create(options.stats || firstOptions.stats || {});
@sokra
sokra Dec 20, 2016 Member

Could this superseed the above code?

var outputOptions = Object.create(statsPresetToOptions(options.stats || firstOptions.stats || {}));
@TheLarkInn
TheLarkInn Dec 20, 2016 Member

If that function can take an array of options, probably.

@TheLarkInn
TheLarkInn Dec 20, 2016 edited Member

Only problem I see with that is that Stats.presetToOptions is really for just taking a string and converting it. I could add the array handling case but that could technically negate the truthyness of passing an empty array to stats: [] which should.eql('normal') style stats.

If we took it that far I could check to see if the array had a length to it to determine if it was actually an array of configs with stats strings inside, but feels like a lot of unneeded logic.

I could maybe see creating a Stats.presetArrayToOptionsArray. It would probably just iterate over the array and call Stats.statsToOptions on options.stats.

@TheLarkInn
TheLarkInn Dec 20, 2016 Member

Or if you are looking to not have the logic in webpack.js:

		if(Array.isArray(options)) {
			options = options.forEach(function(singleOption) {
				singleOption.stats = statsPresetToOptions(singleOption.stats);
			});
		} else {
			options.stats = statsPresetToOptions(options.stats);
		}

I think this much enough could be extracted and put into Stats.js as a static/object method.

@sokra
Member
sokra commented Dec 22, 2016

hmm. I think we have a bigger problem with stats for multiple compilations. The current approach and the current stats only allow a single stats options object, but you probably want different options per compilations:

[
 { ..., stats: "none" },
 { ..., stats: "verbose" }
]

But the current archtiture doesn't support this.

Could you split the unrelated changes into a separate PR so we could merge these at least?

@TheLarkInn
Member

Merge the version bump changes or merge these changes.

@TheLarkInn TheLarkInn Merge branch 'master' into bugfix/fix_stats_when_passed_array
9014901
@TheLarkInn
Member

@sokra Once #3643 merges, I'll update this PR to have a couple tests covering array cases.

TheLarkInn added some commits Jan 10, 2017
@TheLarkInn TheLarkInn Merge branch 'master' into bugfix/fix_stats_when_passed_array 03d3748
@TheLarkInn TheLarkInn Remove bad stats merge
225994b
@TheLarkInn TheLarkInn added additional array case f8ee770
@TheLarkInn TheLarkInn fix bad logic and created additional test to cover
ea2274a
bin/webpack.js
+}
+
+function hasPreset(optionsArray) {
+ options = Array.isArray(optionsArray) ? optionsArray : [optionsArray];
@Kovensky
Kovensky Jan 11, 2017 Collaborator

Hm... [].concat(optionsArray)? Too magic?

sokra added some commits Jan 11, 2017
@sokra sokra Merge branch 'master' into bugfix/fix_stats_when_passed_array 19c7260
@sokra sokra add support for multiple different stats objects
f1d56dc
@sokra sokra linter fixes
784af50
@TheLarkInn TheLarkInn merged commit aa90166 into master Jan 11, 2017

7 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.02%) to 93.742%
Details
licence/cla Contributor License Agreement is signed.
Details
@TheLarkInn TheLarkInn deleted the bugfix/fix_stats_when_passed_array branch Jan 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment