Skip to content

prepend plugins instead of append when defined through CLI - #5359

Merged
sokra merged 4 commits into
webpack:nextfrom
esbenp:cli-prepend-plugins-4260
Sep 14, 2017
Merged

prepend plugins instead of append when defined through CLI#5359
sokra merged 4 commits into
webpack:nextfrom
esbenp:cli-prepend-plugins-4260

Conversation

@esbenp

@esbenp esbenp commented Jul 24, 2017

Copy link
Copy Markdown
Contributor

closes #4260

What kind of change does this PR introduce?

Gives plugins presedence over webpack config file when defined through the CLI

Did you add tests for your changes?

Unsure how to test this, but feel free til suggest. Will add if relevant

If relevant, link to documentation update:

Summary

Does this PR introduce a breaking change?

Other information

@sokra

sokra commented Jul 24, 2017

Copy link
Copy Markdown
Member

For testing see test/binCases. Best write a test having DefinePlugin TEST: "fail" in webpack.config.js and --define TEST=ok in CLI and require("./" + TEST + ".js") in entry.js. You can test the exit code for success.

@esbenp

esbenp commented Jul 24, 2017

Copy link
Copy Markdown
Contributor Author

Creative :-) added in b9c1f9b

@sokra

sokra commented Jul 24, 2017

Copy link
Copy Markdown
Member 
webpack/webpack/test/binCases/configFile/plugins-presedence/test.js
  7:30  error  Strings must use doublequote  quotes
  7:38  error  Missing semicolon             semi

@webpack-bot

webpack-bot commented Jul 24, 2017

Copy link
Copy Markdown
Contributor

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

@esbenp esbenp mentioned this pull request Jul 24, 2017
Closed
sokra
sokra requested changes Jul 25, 2017
View reviewed changes

@sokra sokra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

Comment thread bin/convert-argv.js Outdated
@@ -330,7 +334,7 @@ module.exports = function(yargs, argv, convertOptions) {
}, function() {
ensureArray(options, "plugins");

@sokra sokra Jul 25, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move the ensureArray(options, "plugin") into the addPlugin method to cleanup the code a bit.

@webpack-bot

webpack-bot commented Jul 25, 2017

Copy link
Copy Markdown
Contributor

@esbenp Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@sokra Please review the new changes.

@esbenp

esbenp commented Jul 25, 2017

Copy link
Copy Markdown
Contributor Author

Affirmative

@sokra sokra added this to the webpack 4 milestone Jul 25, 2017
@sokra
sokra changed the base branch from master to next September 11, 2017 09:27
@sokra
sokra merged commit 1ff24a7 into webpack:next Sep 14, 2017
@sokra

sokra commented Sep 14, 2017

Copy link
Copy Markdown
Member

Thanks

@sokra sokra mentioned this pull request Nov 20, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sokra sokra sokra approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

webpack 4

Development

Successfully merging this pull request may close these issues.

3 participants

@esbenp @sokra @webpack-bot