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

feat(new): support to skip install #1458

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
57 changes: 26 additions & 31 deletions actions/new.action.ts
Expand Up @@ -141,42 +141,35 @@ const installPackages = async (
(option) => option.name === 'package-manager',
)!.value as string;

let packageManager: AbstractPackageManager;
if (dryRunMode) {
console.info();
console.info(chalk.green(MESSAGES.DRY_RUN_MODE));
console.info();
return;
}
if (inputPackageManager !== undefined) {
try {
packageManager = PackageManagerFactory.create(inputPackageManager);
await packageManager.install(installDirectory, inputPackageManager);
} catch (error) {
if (error && error.message) {
console.error(chalk.red(error.message));
}
try {
const packageManagerName =
inputPackageManager || (await askForPackageManager())['package-manager'];
const packageManager: AbstractPackageManager | undefined =
packageManagerName === PackageManager.NONE
? undefined
: PackageManagerFactory.create(packageManagerName);
// will not install if user select `none`
await packageManager?.install(installDirectory, packageManagerName);
} catch (error) {
if (error && error.message) {
console.error(chalk.red(error.message));
}
} else {
packageManager = await selectPackageManager();
await packageManager.install(
installDirectory,
packageManager.name.toLowerCase(),
);
}
};

const selectPackageManager = async (): Promise<AbstractPackageManager> => {
const answers: Answers = await askForPackageManager();
return PackageManagerFactory.create(answers['package-manager']);
};

const askForPackageManager = async (): Promise<Answers> => {
const questions: Question[] = [
generateSelect('package-manager')(MESSAGES.PACKAGE_MANAGER_QUESTION)([
PackageManager.NPM,
PackageManager.YARN,
PackageManager.PNPM
PackageManager.PNPM,
PackageManager.NONE,
]),
];
const prompt = inquirer.createPromptModule();
Expand Down Expand Up @@ -225,16 +218,18 @@ const printCollective = () => {
emptyLine();
};

const print = (color: string | null = null) => (str = '') => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prettier automatically format it.

const terminalCols = retrieveCols();
const strLength = str.replace(/\u001b\[[0-9]{2}m/g, '').length;
const leftPaddingLength = Math.floor((terminalCols - strLength) / 2);
const leftPadding = ' '.repeat(Math.max(leftPaddingLength, 0));
if (color) {
str = (chalk as any)[color](str);
}
console.log(leftPadding, str);
};
const print =
(color: string | null = null) =>
(str = '') => {
const terminalCols = retrieveCols();
const strLength = str.replace(/\u001b\[[0-9]{2}m/g, '').length;
const leftPaddingLength = Math.floor((terminalCols - strLength) / 2);
const leftPadding = ' '.repeat(Math.max(leftPaddingLength, 0));
if (color) {
str = (chalk as any)[color](str);
}
console.log(leftPadding, str);
};

export const retrieveCols = () => {
const defaultCols = 80;
Expand Down
3 changes: 2 additions & 1 deletion lib/package-managers/package-manager.ts
@@ -1,5 +1,6 @@
export enum PackageManager {
NPM = 'npm',
YARN = 'yarn',
PNPM = 'pnpm'
PNPM = 'pnpm',
NONE = 'none (skip installing packages)',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just notice something. If none is selected, then npm will be used by default on generated files (eg: README.md with usage commands) due to the following lines on application schematics

https://github.com/nestjs/schematics/blob/440d06d6217e40ca334be635d5264aac6dda8002/src/lib/application/application.factory.ts#L43-L46

thus, wouldn't be better if

Suggested change
NONE = 'none (skip installing packages)',
NONE = 'none (use NPM but without installing packages)',

or even

Suggested change
NONE = 'none (skip installing packages)',
NONE = 'NPM but without installing packages',

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, the selected option is not taken in count for now but this is bug that was already fixed by #1457, so you could think it is.

}