-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
hotfix: extractCSS error in dev mode #4892
Conversation
|
||
plugins.push(new VueLoader.VueLoaderPlugin()) | ||
|
||
Array.prototype.push.apply(plugins, this.options.build.plugins || []) |
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.
@clarkdo do you know why we can't use plugins.push
here?
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.
It was a legacy issue 😆 I think it is now even safe to use rest operator.
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.
@manniL No special reason, it's old code, I didn't notice it
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.
I'll update it
Codecov Report
@@ Coverage Diff @@
## dev #4892 +/- ##
==========================================
- Coverage 91.89% 91.89% -0.01%
==========================================
Files 72 72
Lines 2407 2406 -1
Branches 594 593 -1
==========================================
- Hits 2212 2211 -1
Misses 177 177
Partials 18 18
Continue to review full report at Codecov.
|
@pi0 @manniL How do you think below code ? I think it's less readable and not perform as good as push. plugins() {
return [
...(this.options.dev ? new TimeFixPlugin() : []),
...this.options.build.extractCSS
? new ExtractCssChunksPlugin(Object.assign({
filename: this.getFileName('css'),
chunkFilename: this.getFileName('css'),
// TODO: https://github.com/faceyspacey/extract-css-chunks-webpack-plugin/issues/132
reloadAll: true
}, this.options.build.extractCSS))
: [],
new VueLoader.VueLoaderPlugin(),
...(this.options.build.plugins || []),
// Hide warnings about plugins without a default export (#1179)
new WarnFixPlugin(),
new WebpackBar({
name: this.name,
color: this.colors[this.name],
reporters: [
'basic',
'fancy',
'profile',
'stats'
],
basic: !this.options.build.quiet && env.minimalCLI,
fancy: !this.options.build.quiet && !env.minimalCLI,
profile: !this.options.build.quiet && this.options.build.profile,
stats: !this.options.build.quiet && !this.options.dev && this.options.build.stats,
reporter: {
change: (_, { shortPath }) => {
if (!this.isServer) {
this.nuxt.callHook('bundler:change', shortPath)
}
},
done: (context) => {
if (context.hasErrors) {
this.nuxt.callHook('bundler:error')
}
},
allDone: () => {
this.nuxt.callHook('bundler:done')
}
}
}),
...this.options.build.hardSource
? new HardSourcePlugin(Object.assign({}, this.options.build.hardSource))
: []
]
} |
@clarkdo Yeah. Hard to read and maintain. Current push should be better. |
Agree with @pi0 |
Types of changes
Description
Fix #4885.
Related #4888
As a hotfix, we've already disabled extractCSS in dev mode, this ps is a better fix for solving the root error, but for stability, we may test more and discuss about if there is any value for enabling extractCSS in dev/HMR mode
Checklist: