feat(new): check for installed package managers

[onurravli]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[X] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior?

Currently, the CLI tool asks for all package managers (npm, yarn, and pnpm) even if they aren't installed.

What is the new behavior?

Now, CLI checks for installed package managers and then asks for those that are installed, not for the ones that are not installed.

Does this PR introduce a breaking change?

[ ] Yes
[X] No

[onurravli]

Hey @kamilmysliwiec, can you check this out?

[micalevisk]

Suggested change

	} catch (error) {
	} catch {

[onurravli]

56493f6.

[Tony133]

Suggested change

	if (isThisPackageManagerInstalled('npm'))
	installedPackageManagers.push(PackageManager.NPM);
	if (isThisPackageManagerInstalled('yarn'))
	installedPackageManagers.push(PackageManager.YARN);
	if (isThisPackageManagerInstalled('pnpm'))
	installedPackageManagers.push(PackageManager.PNPM);
	if (isThisPackageManagerInstalled('npm')) {
	installedPackageManagers.push(PackageManager.NPM);
	}
	if (isThisPackageManagerInstalled('yarn')) {
	installedPackageManagers.push(PackageManager.YARN);
	}
	if (isThisPackageManagerInstalled('pnpm')) {
	installedPackageManagers.push(PackageManager.PNPM);
	}

[onurravli]

c517e57.

[kamilmysliwiec]

Not sure how I feel about this change. Isn't this confusing that some options just won't show up for certain people? Like one might be aware that yarn is a thing but doesn't have it installed on let's say their machine at work. Then they generate a NestJS app and they only see NPM showing up which will make them think using yarn was never an option (simply because it's hidden).

Shouldn't we just leave this decision to the developer since the developer should be aware of what tool they have installed (and even if they don't they can always terminate the command, install it, and start over) and want to use for their projects?

[Tony133]

Yes, this is a bit of a "divisive" change, to one 50 percent it can be helpful while the other 50 percent can be confusing 😄

[onurravli]

Not sure how I feel about this change. Isn't this confusing that some options just won't show up for certain people? Like one might be aware that yarn is a thing but doesn't have it installed on let's say their machine at work. Then they generate a NestJS app and they only see NPM showing up which will make them think using yarn was never an option (simply because it's hidden).

Shouldn't we just leave this decision to the developer since the developer should be aware of what tool they have installed (and even if they don't they can always terminate the command, install it, and start over) and want to use for their projects?

I made this change after I realize there is an error when you continue with yarn even yarn is not installed, as below.

If it's not a good idea to hide package managers, it might be a better idea to warn the user if, for example, yarn is not installed, but to build the project (without installing packages).

[kamilmysliwiec]

Even if the installation fails, everything else should be properly generated and you should be good to go as long as you manually install dependencies

[onurravli]

Yes, I mean the error is not clear. In my firsr use of nest-cli, I thought project wasn't built.

[kamilmysliwiec]

So maybe instead of hiding package managers we should rather improve our error messages; let users know that even if the installation failed they're generally (almost) all set

[onurravli]

Agree. Should I close this PR and open a new one, or continue on this?

[kamilmysliwiec]

Feel free to continue in this PR