-
-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add hook build:config #6349
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #6349 +/- ##
==========================================
- Coverage 95.71% 95.68% -0.04%
==========================================
Files 79 79
Lines 2662 2665 +3
Branches 685 686 +1
==========================================
+ Hits 2548 2550 +2
- Misses 98 99 +1
Partials 16 16
Continue to review full report at Codecov.
|
I'm wondering whether we should be more explicit (e.g. Also, we need docs (as usual) explaining the difference between the different methods of adding stuff to the webpack config (e.g. via extends). |
const additionalConfigs = await this.buildContext.nuxt.callHook('build:config') | ||
|
||
if (additionalConfigs) { | ||
webpackConfigs.push(...[].concat(additionalConfigs)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the [].concat
? If the array contains objects then after the concat its still an array to the same references right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The concat here is for supporting return value either array or object as single or multiple builds
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, actually I didnt know you could return values with callHook
. Maybe I am just overseeing something simple but there is no return statement or whatever, so how does it even return a value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's true, it has been changed for a while, I've changed in new pr
@manniL Sorry, I don’t know the new hook pattern decision, is the new pattern corresponding to package? So maybe use hook name as ‘webpack:config’ ? |
Will open another pr |
|
Types of changes
Description
I didn't add any param for now since we already have
extend
for customising webpack config and this hook should avoid introducing more side effects.Checklist: