feat: support config functions with multi-compiler #5196
Conversation
AndersDJohnson
force-pushed
the
multi-compiler-functions
branch
from
4590240
to
5236a5b
Compare
AndersDJohnson
force-pushed
the
multi-compiler-functions
branch
5 times, most recently
from
4978d14
to
d5e4595
Compare
|
Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon. |
lib/prepareOptions.js
Outdated
| }; | ||
|
|
||
| function handleFunction(options, argv) { | ||
| var isES6DefaultExportedFunc = ( |
There was a problem hiding this comment.
the es6 default handling should be before the array handling.
test with
exports.default = [
function() {
return {};
}
]There was a problem hiding this comment.
@sokra Good catch. Updated and added test.
| @@ -0,0 +1,22 @@ | |||
| module.exports = function prepareOptions(options, argv) { | |||
There was a problem hiding this comment.
Write new files in ES6.
- "use strict"
- const/let
- arrow function
There was a problem hiding this comment.
@sokra Thanks, I wasn't sure if non-ES6 was intentional. Updated, though still using functions for top-level rather than const + arrow - let me know if I should change. We should consider adding staged-only linting for this.
There was a problem hiding this comment.
@AndersDJohnson What did you mean by the following:
We should consider adding staged-only linting for this.
Looking to add an issue for this so a contributor can pick it up (unless you would like to)
There was a problem hiding this comment.
@TheLarkInn Something like https://github.com/okonet/lint-staged.
test/ConfigTestCases.test.js
Outdated
| const optionsArr = [].concat(options); | ||
| let options = require(path.join(testDirectory, "webpack.config.js")); | ||
| options = prepareOptions(options); | ||
| let optionsArr = [].concat(options); |
There was a problem hiding this comment.
@sokra Done by wrapping the require directly in a call to prepareOptions, if that's cool.
AndersDJohnson
force-pushed
the
multi-compiler-functions
branch
from
d5e4595
to
24ac48d
Compare
|
@AndersDJohnson Thanks for your update. I labeled the Pull Request so reviewers will review it again. @sokra Please review the new changes. |
|
Thanks |
|
Looks like we still need to support this for the programmatic API (i.e. |
|
Function support for the programmatic API proposed in #5585. |
What kind of change does this PR introduce?
feature
Did you add tests for your changes?
Yes, but let me know if inadequate.
If relevant, link to documentation update:
No documentation updates yet, but can provide. Arguably, the docs already imply that multiple functions are supported.
Summary
We already support configurations arrays for the multi-compiler, and configurations as functions in the singular case, but they don't seem to work together - until now.
Does this PR introduce a breaking change?
No.
Other information
Feel free to ask.