Refactor create-project to support testing the cli and middleware integration #488
Conversation
| } | ||
|
|
||
| if (this.data.linter) { | ||
| this.spawnCommandSync('neutrino', ['lint', '--fix'], { stdio: 'ignore' }); | ||
| this.spawnCommandSync('neutrino', ['lint', '--fix'], { stdio: this.options.stdio, cwd: this.options.directory }); |
There was a problem hiding this comment.
Would this output to the user when linting fails to fix? I think we shouldn't output any linting errors when we're in the installing phase.
There was a problem hiding this comment.
This is normally set to ignore, but during testing we want to see the output so it fails the test.
There was a problem hiding this comment.
Agreed. What you said makes a lot of sense :)
| this.log(`\n${chalk.green('Success!')} Created ${chalk.cyan(this.options.directory)} at ${chalk.yellow(process.cwd())}`); | ||
| this.log(`To get started, change your current working directory to: ${chalk.cyan(this.options.directory)}`); | ||
| this.log(`\n${chalk.green('Hooray, I successfully created your project!')}`); | ||
| this.log(`To get started, change your current working directory to:\n ${chalk.cyan(this.options.directory)}`); |
There was a problem hiding this comment.
"Hooray" :)
If the user is installing an app, what do you think if we log something like:
"To start your application, run yarn start" (or npm start if isYarn evaluates to false)
| @@ -6,18 +6,15 @@ module.exports = () => [ | |||
| choices: [ | |||
| { | |||
| name: 'A web or Node.js application', | |||
| value: 'application', | |||
| checked: true | |||
There was a problem hiding this comment.
Would omitting the checked property default to having the first option checked? Not having at least one checked property would maybe require the user to press the keyboard down arrow before seeing one of the options selected.
There was a problem hiding this comment.
Yes, this causes the first option to be selected. I think checked only works for checkable items, which these weren't.
| @@ -1,25 +1,30 @@ | |||
| const projects = { | |||
| '@neutrinojs/airbnb': { | |||
| type: 'linting', | |||
There was a problem hiding this comment.
What do you think if we store the types in a constant.
const TYPES = {
LINTING: 'linting',
PROJECT: 'project',
TESTING: 'testing'
}
|
Full output image: |
|
(Using the |
|
Looks great! I assume the directory name you chose when taking the screenshot was |
|
Haha yeah, I use a random directory every time. I use an alias to generate the directory: alias random="node -e \"process.stdout.write(require('crypto').randomBytes(8).toString('hex'))"\" |
|
I love it! Thanks a lot for sharing 🙏 Added it to my list of aliases! |
|
Revised PR again, and will also need to update when @helfi92's extensions PR lands. I've done some work to the Vue preset to make it pass the tests, and have also made some changes to the tests themselves to get them to run better in parallel, making it faster. |
|
Included in this PR were some additions of: I'm not really sure what that path if trying to reference? It seems like it might not be taking into account hoisting properly. It might be worth adding some comments alongside them? |
|
When running create-project from the tests, packages are linked instead of installed, since we don't want non-existent versions installed in the repo, but rather to test the current local versions of the packages. But when linking the vue package, the vue-loader had a lot of trouble trying to find its deps because of hoisting combined with a different linked structure, and adding |
|
Either way, adding more explanatory in comments would be a good thing, and I'll start doing that. |
|
Ah ok. I'm concerned that it will now mean projects using Neutrino starting looking for |
|
@edmorley oooooooo great point. 🤔 Maybe we should instead add this only when working in the monorepo, maybe with an environment variable or Neutrino option? |
|
Is this specific to working with lerna or monorepos in general? I wonder if you can just sniff for |
|
@jaridmargolin you make an excellent point, and one which highlights a greater need for #516. If the package.json gets injected into options and is accessible by middleware, it then becomes super easy to determine if this is the monorepo, because we can just look at the name! .add(MODULES)
.when(neutrino.options.packageJson.name === 'neutrino-dev', (modules) => {
modules.add(NEUTRINO_MODULES);
}) |
Fixes #472. The tests take a little while, but it runs pretty well!
We attempt to run each preset, along with tests, and linting in the project and make sure they all pass for each project type. There's more that can be done here as well, such as making sure the package.json and .neutrinorc.js have the right contents.