ES6, new way to extend YO generator #161
Conversation
…package.json that it would be generate for the this generator. This dependencies are working perfect in the project that the generator creates. .ignore: i added a few files that it should be ignored. Note: The next dependencies are breaking the tests and/or it would have to do a few changes in the templetes for using it because of the new versions have a new approach for use it. In this way, dont update the next dependencies : babel-jest jest-cli mockgoose
- eslint added to the main project. eslint config file added and removed from the package.json - main project es6 implemented ( changed vars for consts, let and es6 literal objects and functions ) - Main project tests and generator template were broken because the new version of Yeoman has a different way to extend the generator and spawnCommand was moved to another path. - blueBird promises removed.. Now we are going to use js native promises. Note: Main project tests have been fixed but output tests are broken. Probably it is happening because the dependecies update ( new way to use them, ect .)
There was a problem hiding this comment.
Thank you very much, @maustand. That's an awesome work.
I made some suggestions on code.
Most of the errors on tests seems to be related to mockgoose. Also, [email protected] has broken some of the mongoose plugins we are using, see diegohaz/mongoose-keywords#12. I think they've fixed it on v5.0.10. Not sure.
generators/api/index.js
Outdated
| const recast = require('recast'); | ||
| const reservedWords = require('reserved-words'); | ||
|
|
||
| module.exports = class extends generator { |
There was a problem hiding this comment.
Since generator is a class, let's make it capitalized: Generator
generators/api/index.js
Outdated
| this.props.lower = lowerCase(this.props.camel); | ||
| this.props.lowers = lowerCase(this.props.camels); | ||
| this.props.start = upperFirst(this.props.lower); | ||
| this.props.starts = upperFirst(this.props.lowers); |
There was a problem hiding this comment.
Does that pass eslint? I don't think we need those alignments.
There was a problem hiding this comment.
we can create our custom rules...
Currently i am using eslint recommend, but we could use another rules about indentation, spaces, ect..
feel free to modify it.
generators/api/index.js
Outdated
| }) : []; | ||
| this.props.modelFields.split(',').map((field) => { | ||
| return field.trim(); | ||
| }) : []; |
There was a problem hiding this comment.
Shorten:
this.props.modelFields.split(',').map(field => field.trim())
generators/api/index.js
Outdated
| const copyTpl = this.fs.copyTpl.bind(this.fs); | ||
| const tPath = this.templatePath.bind(this); | ||
| const dPath = this.destinationPath.bind(this); | ||
| const filepath = (filename) => { | ||
| return path.join(props.dir, props.kebab, filename); | ||
| }; |
There was a problem hiding this comment.
I think we can make it shorter too: filename => path.join(...)
generators/app/index.js
Outdated
| module.exports = yeoman.Base.extend({ | ||
| prompting: function () { | ||
| var that = this; | ||
| module.exports = class extends generator { |
There was a problem hiding this comment.
I changed this.. probably i forgot to do it in this file...
generators/app/index.js
Outdated
| @@ -147,15 +160,15 @@ module.exports = yeoman.Base.extend({ | |||
| tPath('api/password-reset'), | |||
| dPath(props.srcDir + '/' + props.apiDir + '/password-reset'), | |||
| props | |||
| ); | |||
| ); | |||
|
@diegohaz In general i did not modify the code in order to avoid problems. By the way, we should find a way to have the dependencies of the output in a only one place.. So i should change this files in my fork or it is automatic ?? let me know. |
|
@maustand It's not automatic. You should make the changes in your fork. The package.json problem is really annoying. I'll try to think in another solution. In the meanwhile, we'll still need to update both package.json. |
|
@maustand, I fixed tests on the branch https://github.com/diegohaz/rest/tree/maustand-master (tried to push to your repo, but I guess you've created the PR without allowing changes, so I don't have access. In this case, you need to merge I needed to downgrade Mongoose and Mockgoose. It seems to not be an easy migration, so let's try to upgrade them alter in another PR. And, for some reason, coverage reports are empty (for the generator). I'm not sure why. Maybe upgrading some gulp plugin caused this. |
Hi @diegohaz
This commit has a lots of changes..
I have updated all the dependencies again.
I changed all the generator code to es6 ( changing var to let and consts )
I put eslint and the last but one , i have updated the way that you create yeoman generators because you was using YO 0.24.1 and i updated for the last one..
Please you can review the code the code and do the merge...
Important:
Tests are not working, even running npm test in a fresh clone..
I am trying to fix it... meanwhile you can take a look :
Regards