-
-
Notifications
You must be signed in to change notification settings - Fork 627
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): Refactor Yeoman #323
Changes from 6 commits
5a4e3e1
5c0b7c1
6969f5f
77f8f53
ee66351
363d29a
7fee593
0cfa9e0
9fc6c52
5d19f45
5aaac64
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 |
---|---|---|
@@ -1,65 +1,152 @@ | ||
"use strict"; | ||
|
||
const yeoman = require("yeoman-environment"); | ||
const Generator = require("yeoman-generator"); | ||
const path = require("path"); | ||
const defaultGenerator = require("../generators/init-generator"); | ||
const runTransform = require("./transformations/index"); | ||
const j = require("jscodeshift"); | ||
const chalk = require("chalk"); | ||
const pEachSeries = require("p-each-series"); | ||
|
||
const runPrettier = require("../utils/run-prettier"); | ||
|
||
const entryTransform = require("./transformations/entry/entry"); | ||
const outputTransform = require("./transformations/output/output"); | ||
const contextTransform = require("./transformations/context/context"); | ||
const resolveTransform = require("./transformations/resolve/resolve"); | ||
const devtoolTransform = require("./transformations/devtool/devtool"); | ||
const targetTransform = require("./transformations/target/target"); | ||
const watchTransform = require("./transformations/watch/watch"); | ||
const watchOptionsTransform = require("./transformations/watch/watchOptions"); | ||
const externalsTransform = require("./transformations/externals/externals"); | ||
const nodeTransform = require("./transformations/node/node"); | ||
const performanceTransform = require("./transformations/performance/performance"); | ||
const statsTransform = require("./transformations/stats/stats"); | ||
const amdTransform = require("./transformations/other/amd"); | ||
const bailTransform = require("./transformations/other/bail"); | ||
const cacheTransform = require("./transformations/other/cache"); | ||
const profileTransform = require("./transformations/other/profile"); | ||
const mergeTransform = require("./transformations/other/merge"); | ||
const parallelismTransform = require("./transformations/other/parallelism"); | ||
const recordsInputPathTransform = require("./transformations/other/recordsInputPath"); | ||
const recordsOutputPathTransform = require("./transformations/other/recordsOutputPath"); | ||
const recordsPathTransform = require("./transformations/other/recordsPath"); | ||
const moduleTransform = require("./transformations/module/module"); | ||
const pluginsTransform = require("./transformations/plugins/plugins"); | ||
const topScopeTransform = require("./transformations/top-scope/top-scope"); | ||
const devServerTransform = require("./transformations/devServer/devServer"); | ||
const modeTransform = require("./transformations/mode/mode"); | ||
const resolveLoaderTransform = require("./transformations/resolveLoader/resolveLoader"); | ||
|
||
const transformsObject = { | ||
entryTransform, | ||
outputTransform, | ||
contextTransform, | ||
resolveTransform, | ||
devtoolTransform, | ||
targetTransform, | ||
watchTransform, | ||
watchOptionsTransform, | ||
externalsTransform, | ||
nodeTransform, | ||
performanceTransform, | ||
statsTransform, | ||
amdTransform, | ||
bailTransform, | ||
cacheTransform, | ||
profileTransform, | ||
moduleTransform, | ||
pluginsTransform, | ||
topScopeTransform, | ||
mergeTransform, | ||
devServerTransform, | ||
modeTransform, | ||
parallelismTransform, | ||
recordsInputPathTransform, | ||
recordsOutputPathTransform, | ||
recordsPathTransform, | ||
resolveLoaderTransform | ||
}; | ||
|
||
/** | ||
* | ||
* Runs yeoman and runs the transformations based on the object | ||
* built up from an author/user | ||
* Runs the transformations from an object we get from yeoman | ||
* | ||
* @param {String} options - An path to the given generator | ||
* @returns {Function} runTransform - Run transformations based on the finished | ||
* yeoman instance | ||
* @param {Object} webpackProperties - Configuration to transform | ||
* @param {String} action - Action to be done on the given ast | ||
* @returns {Promise} - A promise that writes each transform, runs prettier | ||
* and writes the file | ||
*/ | ||
|
||
function creator(options) { | ||
let env = yeoman.createEnv("webpack", null); | ||
const generatorName = options | ||
? replaceGeneratorName(path.basename(options[0])) | ||
: "webpack-default-generator"; | ||
if (options) { | ||
const WebpackGenerator = class extends Generator { | ||
initializing() { | ||
options.forEach(path => { | ||
return this.composeWith(require.resolve(path)); | ||
}); | ||
} | ||
}; | ||
env.registerStub(WebpackGenerator, generatorName); | ||
} else { | ||
env.registerStub(defaultGenerator, "webpack-default-generator"); | ||
} | ||
|
||
env.run(generatorName).on("end", _ => { | ||
if (generatorName !== "webpack-default-generator") { | ||
//HACK / FIXME | ||
env = env.options.env; | ||
return runTransform(env.configuration); | ||
} else { | ||
return runTransform(env.getArgument("configuration")); | ||
} | ||
module.exports = function runTransform(webpackProperties, action) { | ||
// webpackOptions.name sent to nameTransform if match | ||
const webpackConfig = Object.keys(webpackProperties).filter(p => { | ||
return p !== "configFile" && p !== "configPath"; | ||
}); | ||
} | ||
webpackConfig.forEach(scaffoldPiece => { | ||
const config = webpackProperties[scaffoldPiece]; | ||
const transformations = Object.keys(transformsObject) | ||
.map(k => { | ||
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. What would be 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. Yup, sometimes it is kinda misleading, can we have more descriptive names? |
||
const stringVal = k.substr(0, k.indexOf("Transform")); | ||
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. can we move this into a separate method or a much better way to handle this? 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. On it |
||
if (Object.keys(config.webpackOptions).length) { | ||
if (config.webpackOptions[stringVal]) { | ||
return [transformsObject[k], config.webpackOptions[stringVal]]; | ||
} else { | ||
return [transformsObject[k], config[stringVal]]; | ||
} | ||
} else { | ||
return [transformsObject[k]]; | ||
} | ||
}) | ||
.filter(e => e[1]); | ||
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. Using 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. It's working as intended 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. Yeah, It'll get cleared anyways because of falsy iteration. 👍 |
||
const ast = j( | ||
action && action !== "init" | ||
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 condition is used multiple times. We can DRY the code and move it inside a variable like 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. on it |
||
? webpackProperties.configFile | ||
: "module.exports = {}" | ||
); | ||
const transformAction = action || null; | ||
|
||
/* | ||
* @function replaceGeneratorName | ||
* | ||
* Replaces the webpack-addons pattern with the end of the addons name merged | ||
* with 'generator' | ||
* | ||
* @param { String } name - name of the generator | ||
* @returns { String } name - replaced pattern of the name | ||
*/ | ||
return pEachSeries(transformations, f => { | ||
if (!f[1]) { | ||
return f[0](j, ast, transformAction); | ||
} else { | ||
return f[0](j, ast, f[1], transformAction); | ||
} | ||
}) | ||
.then(_ => { | ||
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.
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. Noop functions are 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. But this is not a NOOP function. 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. yeah it is kinda misleading provided we will also have lodash defined in 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. It is rather inconsistent 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. Ok! |
||
let configurationName; | ||
if (!config.configName) { | ||
configurationName = "webpack.config.js"; | ||
} else { | ||
configurationName = "webpack." + config.configName + ".js"; | ||
} | ||
|
||
function replaceGeneratorName(name) { | ||
return name.replace(/(webpack-addons)?([^:]+)(:.*)?/g, "generator$2"); | ||
} | ||
const outputPath = | ||
action && action !== "init" | ||
? webpackProperties.configPath | ||
: path.join(process.cwd(), configurationName); | ||
const source = ast.toSource({ | ||
quote: "single" | ||
}); | ||
|
||
module.exports = { | ||
creator, | ||
replaceGeneratorName | ||
runPrettier(outputPath, source); | ||
}) | ||
.catch(err => { | ||
console.error(err.message ? err.message : err); | ||
}); | ||
}); | ||
if (action === "add" && webpackProperties.config.item) { | ||
process.stdout.write( | ||
"\n" + | ||
chalk.green( | ||
`Congratulations! ${ | ||
webpackProperties.config.item | ||
} has been ${action}ed!\n` | ||
) | ||
); | ||
} else { | ||
process.stdout.write( | ||
"\n" + | ||
chalk.green( | ||
"Congratulations! Your new webpack configuration file has been created!\n" | ||
) | ||
); | ||
} | ||
}; |
This file was deleted.
This file was deleted.
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.
Could add a
TODO
here as a reminder to revert it oncev4.0.0
is out?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 prefer such hard-coded strings in a variable rather hard-coded. Easier to remember, easier to change and yes for
TODO
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 think this is pretty intuitive