-
-
Notifications
You must be signed in to change notification settings - Fork 648
Feature/yeoman generators #93
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
Conversation
Changes the structure to the init feature a bit. We’re now making the parser validate everything, as we can later run the inquirer prompt after running yeoman, reading from a json file. This will allow us to have a more fluid process and removes a lot of «trash» code. Disabled listing for now, as this is work in progress.
|
|
||
| class WebpackGenerator extends Generator { | ||
| constructor(args, opts) { | ||
| super(args, opts); |
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.
No need for constructor in this case.
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.
Yeoman needs it, and I'm passing something to it later
Makes it possible to make a inquirer prompt with a remote package.
lib/creator/generators/index.js
Outdated
| @@ -1,4 +1,5 @@ | |||
| const Generator = require('yeoman-generator'); | |||
| // eslint-disable-next-line | |||
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 have you disabled the next line? AFAIK destructuring won't work in Node 4.x
| "webpack-addons-preact" | ||
| ] | ||
| } | ||
| } No newline at end of file |
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.
Please add a new line
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.
this is deleted.
lib/creator/generators/index.js
Outdated
| inject() {} | ||
| } | ||
|
|
||
| module.exports = WebpackGenerator; |
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 not export directly in the declaration at L5?
lib/creator/index.js
Outdated
| * @returns { <Function|Error> } initTransform - Initializes the scaffold in yeoman | ||
| */ | ||
|
|
||
| module.exports = function creator(pkgPaths,opts) { |
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.
White space after ,
lib/creator/init-transform.js
Outdated
| //eslint-disable-next-line | ||
| } catch (e) {} | ||
| } | ||
| else if(!options) { |
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.
This else if isn't needed.
lib/creator/init-transform.js
Outdated
| } catch (e) {} | ||
| } | ||
| else if(!options) { | ||
| const env = yeoman.createEnv(); |
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.
Can be extracted from here and L18
lib/inquirer-prompt.js
Outdated
|
|
||
| module.exports = function prompt(questions, config) { | ||
| inquirer.prompt(questions).ui.process | ||
| .reduce(function (newOpts, ans) { |
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.
function -> =>
lib/inquirer-prompt.js
Outdated
| module.exports = function prompt(questions, config) { | ||
| inquirer.prompt(questions).ui.process | ||
| .reduce(function (newOpts, ans) { | ||
| return attachAnswers(newOpts, ans); |
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.
Actually, function(newOpts, ans) {...} -> attachAnswers
lib/creator/validate-options.spec.js
Outdated
| describe('validate-options', () => { | ||
| //eslint-disable-next-line | ||
| const {validateOptions} = require('../../__mocks__/parser/validate-options.mock'); | ||
| const {validateOptions} = require('../../__mocks__/creator/validate-options.mock'); |
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.
Still eslint-disabled. This won't work in Node 4.x
| */ | ||
|
|
||
| module.exports = function initializeInquirer(pkg) { | ||
| if(pkg.length == 0) { |
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.
Should we check if pkg is Array? Otherwise this will be a run-time error.
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.
Is default in yargs, in the future, when we're revamping the ui, perhaps.
lib/initialize.js
Outdated
| return prompt(Rx.Observable.from(questions), initialConfig); | ||
| return creator(); | ||
| } | ||
| else { |
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.
No need for this else 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.
Old code now
Removes inquirer, we’re not using it explicitly anymore. Fixed some review mistakes.
|
@okonet I've removed |
okonet
left a comment
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.
![]()
Changes the structure to the init feature a bit. We’re now making the parser validate everything, as we can later run the inquirer prompt after running yeoman, reading from a json file. This will allow us to have a more fluid process and removes a lot of «trash» code. Disabled listing for now, as this is work in progress.
This is the groundwork for me to align my work with the AST transformations @pksjce is doing. We're further gonna integrate the prompting from yeoman and we expect to get a config object back from the generator we run.