-
-
Notifications
You must be signed in to change notification settings - Fork 603
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
cli(init): webpack4 ready #356
Changes from 8 commits
c0997de
6aedd20
6554ffd
8175a9b
114a0c2
9f08f33
eafcb98
9553557
1177b44
4b1edb7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,9 +4,6 @@ const Generator = require("yeoman-generator"); | |
const chalk = require("chalk"); | ||
const logSymbols = require("log-symbols"); | ||
|
||
const createCommonsChunkPlugin = require("webpack-addons") | ||
.createCommonsChunkPlugin; | ||
|
||
const Input = require("webpack-addons").Input; | ||
const Confirm = require("webpack-addons").Confirm; | ||
const List = require("webpack-addons").List; | ||
|
@@ -43,7 +40,6 @@ module.exports = class InitGenerator extends Generator { | |
prompting() { | ||
const done = this.async(); | ||
const self = this; | ||
let oneOrMoreEntries; | ||
let regExpForStyles; | ||
let ExtractUseProps; | ||
let outputPath = "dist"; | ||
|
@@ -83,9 +79,9 @@ module.exports = class InitGenerator extends Generator { | |
return entryQuestions(self, entryTypeAnswer); | ||
}) | ||
.then(entryOptions => { | ||
this.configuration.config.webpackOptions.entry = entryOptions; | ||
oneOrMoreEntries = Object.keys(entryOptions); | ||
|
||
if (entryOptions !== "\"\"") { | ||
this.configuration.config.webpackOptions.entry = entryOptions; | ||
} | ||
return this.prompt([ | ||
Input( | ||
"outputType", | ||
|
@@ -94,8 +90,9 @@ module.exports = class InitGenerator extends Generator { | |
]); | ||
}) | ||
.then(outputTypeAnswer => { | ||
if (!this.configuration.config.webpackOptions.entry.length) { | ||
this.configuration.config.topScope.push(tooltip.commonsChunk()); | ||
// As entry is not required anymore and we dont set it to be an empty string or """"" | ||
// it can be undefined so falsy check is enough (vs entry.length); | ||
if (!this.configuration.config.webpackOptions.entry) { | ||
this.configuration.config.webpackOptions.output = { | ||
filename: "'[name].[chunkhash].js'", | ||
chunkFilename: "'[name].[chunkhash].js'" | ||
|
@@ -117,8 +114,7 @@ module.exports = class InitGenerator extends Generator { | |
}) | ||
.then(prodConfirmAnswer => { | ||
this.isProd = prodConfirmAnswer["prodConfirm"]; | ||
this.configuration.config.webpackOptions.mode = | ||
this.isProd ? `"${"production"}"` : `"${"development"}"`; | ||
this.configuration.config.webpackOptions.mode = this.isProd ? "'production'" : "'development'"; | ||
}) | ||
.then(() => { | ||
return this.prompt([ | ||
|
@@ -378,15 +374,28 @@ module.exports = class InitGenerator extends Generator { | |
); | ||
} | ||
} | ||
}) | ||
.then(() => { | ||
if (this.configuration.config.webpackOptions.entry.length === 0) { | ||
oneOrMoreEntries.forEach(prop => { | ||
this.configuration.config.webpackOptions.plugins.push( | ||
createCommonsChunkPlugin(prop) | ||
); | ||
}); | ||
} | ||
// add splitChunks options for transparency | ||
this.configuration.config.topScope.push(tooltip.splitChunks()); | ||
this.configuration.config.webpackOptions.optimization = { | ||
splitChunks: { | ||
chunks: "'async'", | ||
minSize: 30000, | ||
minChunks: 1, | ||
// for production name is recommended to be off | ||
name: !this.isProd, | ||
cacheGroups: { | ||
vendors: { | ||
test: "/[\\\\/]node_modules[\\\\/]/", | ||
priority: -10 | ||
}, | ||
}, | ||
default: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This didn't work when trying to build a web app using this PR. Removing |
||
minChunks: 2, | ||
priority: -20, | ||
reuseExistingChunk: true | ||
} | ||
} | ||
}; | ||
done(); | ||
}); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,7 @@ module.exports = (self, answer) => { | |
.prompt([ | ||
InputValidate( | ||
"multipleEntries", | ||
"Type the names you want for your modules (entry files), separated by comma [example: 'app,vendor']", | ||
"Type the names you want for your modules (entry files), separated by comma [example: app,vendor]", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use this to split groups of chunks. |
||
validate | ||
) | ||
]) | ||
|
@@ -36,11 +36,11 @@ module.exports = (self, answer) => { | |
if ( | ||
n[val].charAt(0) !== "(" && | ||
n[val].charAt(0) !== "[" && | ||
n[val].indexOf("function") < 0 && | ||
n[val].indexOf("path") < 0 && | ||
n[val].indexOf("process") < 0 | ||
!n[val].includes("function") && | ||
!n[val].includes("path") && | ||
!n[val].includes("process") | ||
) { | ||
n[val] = `"${n[val]}.js"`; | ||
n[val] = `"'${n[val]}.js'"`; | ||
} | ||
webpackEntryPoint[val] = n[val]; | ||
}); | ||
|
@@ -55,7 +55,7 @@ module.exports = (self, answer) => { | |
self.prompt([ | ||
InputValidate( | ||
`${entryProp}`, | ||
`What is the location of "${entryProp}"? [example: "./src/${entryProp}"]`, | ||
`What is the location of "${entryProp}"? [example: ./src/${entryProp}]`, | ||
validate | ||
) | ||
]) | ||
|
@@ -64,11 +64,11 @@ module.exports = (self, answer) => { | |
if ( | ||
propAns[val].charAt(0) !== "(" && | ||
propAns[val].charAt(0) !== "[" && | ||
propAns[val].indexOf("function") < 0 && | ||
propAns[val].indexOf("path") < 0 && | ||
propAns[val].indexOf("process") < 0 | ||
!propAns[val].includes("function") && | ||
!propAns[val].includes("path") && | ||
!propAns[val].includes("process") | ||
) { | ||
propAns[val] = `"${propAns[val]}.js"`; | ||
propAns[val] = `"'${propAns[val]}.js'"`; | ||
} | ||
webpackEntryPoint[val] = propAns[val]; | ||
}); | ||
|
@@ -80,8 +80,7 @@ module.exports = (self, answer) => { | |
.prompt([ | ||
InputValidate( | ||
"singularEntry", | ||
"Which module will be the first to enter the application? [example: './src/index']", | ||
validate | ||
"Which module will be the first to enter the application? [default: ./src/index]" | ||
) | ||
]) | ||
.then(singularAnswer => `"${singularAnswer["singularEntry"]}"`); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,13 +17,17 @@ module.exports = { | |
* | ||
*/`; | ||
}, | ||
commonsChunk: _ => { | ||
splitChunks: _ => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good stuff :3 |
||
return `/* | ||
* We've enabled commonsChunkPlugin for you. This allows your app to | ||
* load faster and it splits the modules you provided as entries across | ||
* different bundles! | ||
* SplitChunksPlugin is enabled by default and replaced | ||
* deprecated CommonsChunkPlugin. It automatically identifies modules which | ||
* should be splitted of chunk by heuristics using module duplication count and | ||
* module category (i. e. node_modules). And splits the chunks… | ||
* | ||
* https://webpack.js.org/plugins/commons-chunk-plugin/ | ||
* It is safe to remove "splitChunks" from the generated configuration | ||
* and was added as an educational example. | ||
* | ||
* https://webpack.js.org/plugins/split-chunks-plugin/ | ||
* | ||
*/`; | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
// Jest Snapshot v1, https://goo.gl/fbAQLP | ||
|
||
exports[`optimization transforms correctly using "optimization-0" data 1`] = ` | ||
"module.exports = { | ||
entry: 'index.js', | ||
|
||
output: { | ||
filename: 'bundle.js' | ||
}, | ||
|
||
optimization: { | ||
splitChunks: { | ||
minSize: 1, | ||
chunks: 'all' | ||
} | ||
} | ||
} | ||
" | ||
`; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
module.exports = { | ||
entry: 'index.js', | ||
output: { | ||
filename: 'bundle.js' | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
"use strict"; | ||
|
||
const utils = require("../../../utils/ast-utils"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is okay for now, GSOC participants will fix the rest of missing stuff =) |
||
|
||
/** | ||
* | ||
* Transform for optimization. Finds the optimization property from yeoman and creates a | ||
* property based on what the user has given us. | ||
* | ||
* @param j — jscodeshift API | ||
* @param ast - jscodeshift API | ||
* @param {any} webpackProperties - transformation object to scaffold | ||
* @param {String} action - action that indicates what to be done to the AST | ||
* @returns ast - jscodeshift API | ||
*/ | ||
|
||
module.exports = function profileTransform(j, ast, webpackProperties, action) { | ||
function createProfileProperty(p) { | ||
utils.pushCreateProperty(j, p, "optimization", j.objectExpression([])); | ||
return utils.pushObjectKeys(j, p, webpackProperties, "optimization"); | ||
} | ||
|
||
if (webpackProperties || typeof webpackProperties === "boolean") { | ||
if (action === "init" && typeof webpackProperties === "object") { | ||
return ast | ||
.find(j.ObjectExpression) | ||
.filter(p => utils.isAssignment(null, p, createProfileProperty)); | ||
} else if ( | ||
action === "init" && | ||
(webpackProperties.length || typeof webpackProperties === "boolean") | ||
) { | ||
return ast | ||
.find(j.ObjectExpression) | ||
.filter(p => | ||
utils.isAssignment( | ||
j, | ||
p, | ||
utils.pushCreateProperty, | ||
"optimization", | ||
webpackProperties | ||
) | ||
); | ||
} else if (action === "add") { | ||
// TODO | ||
} else if (action === "remove") { | ||
// TODO | ||
} else if (action === "update") { | ||
// TODO | ||
} | ||
} else { | ||
return ast; | ||
} | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
"use strict"; | ||
|
||
const defineTest = require("../../../utils/defineTest"); | ||
|
||
defineTest( | ||
__dirname, | ||
"optimization", | ||
"optimization-0", | ||
{ | ||
splitChunks: { | ||
minSize: 1, | ||
chunks: "'all'" | ||
} | ||
}, | ||
"init" | ||
); |
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.
Probably you chose these numbers based some logic or some reference. In case you did so, could add a comment for that (link to somewhere would be great)? So developers now why :)
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.
they are defaults from the docs, we also add a comment to that link here:
https://github.com/webpack/webpack-cli/pull/356/files#diff-f722437ab3fa8013b0afe4475d557c7fR27
and here is direct link: https://webpack.js.org/plugins/split-chunks-plugin/#optimization-splitchunks
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.
Oh yeah, I meant to ask if you put these links INTO the code as a comment :)
Sorry for misunderstanding. The link to webpack documentation I guess it's fine. What do you think? Makes sense?
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.
Yeah as a quick ref 👍