Skip to content
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

Even though Yarn is detected, it creates a npm lockfile #747

Open
silvenon opened this issue Jan 10, 2019 · 6 comments
Open

Even though Yarn is detected, it creates a npm lockfile #747

silvenon opened this issue Jan 10, 2019 · 6 comments
Assignees
Labels
Milestone

Comments

@silvenon
Copy link
Member

No description provided.

@silvenon silvenon added the bug label Jan 10, 2019
@UlisesGascon UlisesGascon added this to the v4.1.0 milestone Jun 3, 2019
@ilicmarko
Copy link
Contributor

This can be solved with a simple child process check. But I would maybe give the user an option to choose. At one point Yarn made a lot of sense but the tables have turned and NPM has caught up to Yarn. It provides some additional functionality like audit fix and npm ci etc..

For this reason I would let users choose between

  • npm
  • yarn
  • pnpm

If you are OK with this I can get right on it @silvenon.

@UlisesGascon
Copy link
Member

@ilicmarko thanks a lot for your support :-)

Maybe it is better to handle first this bug and in a separate issue/PR we can discuss about let the user choose the CLI ;-)

What do you think @silvenon ?

@silvenon
Copy link
Member Author

@ilicmarko I like the idea, are you still up for doing this?

@ilicmarko
Copy link
Contributor

@silvenon I will sit down on the weekend and put in some work.

@ilicmarko
Copy link
Contributor

ilicmarko commented Dec 6, 2019

Sorry for a delay, had some construction work in the house so no JS for me :(

Anyways, got something done after work. It was really strange to debug as the npm install was getting called whatever I do. After some sweat, I found it was coming from another side.

this.composeWith(
require.resolve(
`generator-${this.options['test-framework']}/generators/app`
),
{
'skip-install': this.options['skip-install']
}
);
}

It's interesting that even if you send the skip-install option to the mocha generator it will do a dev install.

https://github.com/yeoman/generator-mocha/blob/21da7926613dba0d404135798cfb8d8b4161acb1/generators/app/index.js#L70-L88

Looking at line 86.

https://github.com/yeoman/generator-mocha/blob/21da7926613dba0d404135798cfb8d8b4161acb1/generators/app/index.js#L86

Anyways what do you suggest as the fix?

@silvenon @UlisesGascon

@silvenon
Copy link
Member Author

silvenon commented Mar 1, 2020

Sorry for my late response 😅 Thank you for figuring out where the error is coming from! 🏆 We definitely need to add Yarn support to those generators, feel free to send PRs there if you're up for it. As for the --skip-install option, ideally in that case you should still get chai and mocha in your package.json, but without npm install/yarn install. We need to unify these three projects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants