Skip to content

Commit 5203a38

Browse files
committed
fix(@angular/cli): skip prompt or warn when setting up autocompletion without a global CLI install
If the user does not have a global install of the Angular CLI, the autocompletion prompt is skipped and `ng completion` emits a warning. The reasoning for this is that `source <(ng completion script)` won't work without `ng` on the `$PATH`, which is only really viable with a global install. Local executions like `git clone ... && npm install && npm start` or ephemeral executions like `npx @angular/cli` don't benefit from autocompletion and unnecessarily impede users. A global install of the Angular CLI is detected by running `which -a ng`, which appears to be a cross-platform means of listing all `ng` commands on the `$PATH`. We then look over all binaries in the list and exclude anything which is a directo child of a `node_modules/.bin/` directory. These include local executions and `npx`, so the only remaining locations should be global installs (`/usr/bin/ng`, NVM, etc.). The tests are a little awkward since `ng` is installed globally by helper functions before tests start. These tests uninstall the global CLI and install a local, project-specific version to verify behavior, before restoring the global version. Hypothetically this could be emulated by manipulating the `$PATH` variable, but `which` needs to be available (so we can't clobber the whole `$PATH`) and `node` exists in the same directory as the global `ng` command (so we can't remove that directory anyways). There's also no good way of testing the case where `which` fails to run. Closes angular#23135.
1 parent 2b41802 commit 5203a38

File tree

5 files changed

+215
-16
lines changed

5 files changed

+215
-16
lines changed

packages/angular/cli/src/commands/completion/cli.ts

+21-7
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,13 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9+
import { tags } from '@angular-devkit/core';
910
import { join } from 'path';
1011
import yargs, { Argv } from 'yargs';
11-
import {
12-
CommandModule,
13-
CommandModuleImplementation,
14-
CommandScope,
15-
} from '../../command-builder/command-module';
12+
import { CommandModule, CommandModuleImplementation } from '../../command-builder/command-module';
1613
import { addCommandModuleToYargs } from '../../command-builder/utilities/command';
1714
import { colors } from '../../utilities/color';
18-
import { initializeAutocomplete } from '../../utilities/completion';
15+
import { hasGlobalCliInstall, initializeAutocomplete } from '../../utilities/completion';
1916

2017
export class CompletionCommandModule extends CommandModule implements CommandModuleImplementation {
2118
command = 'completion';
@@ -38,12 +35,29 @@ export class CompletionCommandModule extends CommandModule implements CommandMod
3835

3936
this.context.logger.info(
4037
`
41-
Appended \`source <(ng completion script)\` to \`${rcFile}\`. Restart your terminal or run the following to autocomplete \`ng\` commands:
38+
${tags.oneLine`
39+
Appended \`source <(ng completion script)\` to \`${rcFile}\`. Restart your terminal or run the
40+
following to autocomplete \`ng\` commands:
41+
`}
4242
4343
${colors.yellow('source <(ng completion script)')}
4444
`.trim(),
4545
);
4646

47+
if ((await hasGlobalCliInstall()) === false) {
48+
this.context.logger.warn(
49+
`
50+
${tags.oneLine`
51+
Setup completed successfully, but there does not seem to be a global install of the Angular CLI.
52+
For autocompletion to work, the CLI will need to be on your \`$PATH\`, which is typically done
53+
with the \`-g\` flag in \`npm install -g @angular/cli\`.
54+
`}
55+
56+
See https://angular.io/cli/completion#global-install for more info.
57+
`.trim(),
58+
);
59+
}
60+
4761
return 0;
4862
}
4963
}

packages/angular/cli/src/utilities/completion.ts

+61-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import { json, logging } from '@angular-devkit/core';
9+
import { json, logging, tags } from '@angular-devkit/core';
10+
import { execFile } from 'child_process';
1011
import { promises as fs } from 'fs';
1112
import * as path from 'path';
1213
import { env } from 'process';
@@ -72,12 +73,29 @@ Ok, you won't be prompted again. Should you change your mind, the following comm
7273
// Notify the user autocompletion was set up successfully.
7374
logger.info(
7475
`
75-
Appended \`source <(ng completion script)\` to \`${rcFile}\`. Restart your terminal or run the following to autocomplete \`ng\` commands:
76+
${tags.oneLine`
77+
Appended \`source <(ng completion script)\` to \`${rcFile}\`. Restart your terminal or run the
78+
following to autocomplete \`ng\` commands:
79+
`}
7680
7781
${colors.yellow(`source <(ng completion script)`)}
7882
`.trim(),
7983
);
8084

85+
if ((await hasGlobalCliInstall()) === false) {
86+
logger.warn(
87+
`
88+
${tags.oneLine`
89+
Setup completed successfully, but there does not seem to be a global install of the Angular CLI.
90+
For autocompletion to work, the CLI will need to be on your \`$PATH\`, which is typically done
91+
with the \`-g\` flag in \`npm install -g @angular/cli\`.
92+
`}
93+
94+
See https://angular.io/cli/completion#global-install for more info.
95+
`.trim(),
96+
);
97+
}
98+
8199
// Save configuration to remember that the user was prompted.
82100
await setCompletionConfig({ ...completionConfig, prompted: true });
83101

@@ -147,6 +165,12 @@ async function shouldPromptForAutocompletionSetup(
147165
return false; // Unknown shell.
148166
}
149167

168+
// Don't prompt if the user is missing a global CLI install. Autocompletion won't work after setup
169+
// anyway and could be annoying for users running one-off commands via `npx` or using `npm start`.
170+
if (!(await hasGlobalCliInstall())) {
171+
return false;
172+
}
173+
150174
// Check each RC file if they already use `ng completion script` in any capacity and don't prompt.
151175
for (const rcFile of rcFiles) {
152176
const contents = await fs.readFile(rcFile, 'utf-8').catch(() => undefined);
@@ -246,3 +270,38 @@ function getShellRunCommandCandidates(shell: string, home: string): string[] | u
246270
return undefined;
247271
}
248272
}
273+
274+
/**
275+
* Returns whether the user has a global CLI install or `undefined` if this can't be determined.
276+
* Execution from `npx` is *not* considered a global CLI install.
277+
*
278+
* This does *not* mean the current execution is from a global CLI install, only that a global
279+
* install exists on the system.
280+
*/
281+
export async function hasGlobalCliInstall(): Promise<boolean | undefined> {
282+
// List all binaries with the `ng` name on the user's `$PATH`.
283+
const proc = execFile('which', ['-a', 'ng']);
284+
let stdout = '';
285+
proc.stdout?.addListener('data', (content) => {
286+
stdout += content;
287+
});
288+
const exitCode = await new Promise((resolve) => {
289+
proc.addListener('exit', (exitCode) => {
290+
resolve(exitCode);
291+
});
292+
});
293+
294+
if (exitCode !== 0) {
295+
// `which -a` might not be supported some some systems, can't tell whether CLI is globally
296+
// installed.
297+
return undefined;
298+
}
299+
300+
// Look for at least one line which is a global install. We can't easily identify global installs,
301+
// but local installs are typically placed in `node_modules/.bin` by NPM / Yarn. `npx` also
302+
// currently caches files at `~/.npm/_npx/*/node_modules/.bin/`, so the same logic applies.
303+
const lines = stdout.split('\n').filter((line) => line !== '');
304+
const globalInstalls = lines.filter((line) => !line.endsWith('node_modules/.bin'));
305+
306+
return globalInstalls.length >= 1;
307+
}

tests/legacy-cli/e2e/tests/misc/completion-prompt.ts

+57-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,13 @@ import { promises as fs } from 'fs';
22
import * as os from 'os';
33
import * as path from 'path';
44
import { env } from 'process';
5-
import { execAndCaptureError, execWithEnv } from '../../utils/process';
5+
import { getGlobalVariable } from '../../utils/env';
6+
import {
7+
execAndCaptureError,
8+
execAndWaitForOutputToMatch,
9+
execWithEnv,
10+
silentNpm,
11+
} from '../../utils/process';
612

713
const AUTOCOMPLETION_PROMPT = /Would you like to enable autocompletion\?/;
814
const DEFAULT_ENV = Object.freeze({
@@ -18,6 +24,8 @@ const DEFAULT_ENV = Object.freeze({
1824
NG_CLI_ANALYTICS: 'false',
1925
});
2026

27+
const testRegistry = getGlobalVariable('package-registry');
28+
2129
export default async function () {
2230
// Windows Cmd and Powershell do not support autocompletion. Run a different set of tests to
2331
// confirm autocompletion skips the prompt appropriately.
@@ -368,6 +376,54 @@ source <(ng completion script)
368376
);
369377
}
370378
});
379+
380+
// Prompts when a global CLI install is present on the system.
381+
await mockHome(async (home) => {
382+
const bashrc = path.join(home, '.bashrc');
383+
await fs.writeFile(bashrc, `# Other content...`);
384+
385+
await execAndWaitForOutputToMatch('ng', ['version'], AUTOCOMPLETION_PROMPT, {
386+
...DEFAULT_ENV,
387+
SHELL: '/bin/bash',
388+
HOME: home,
389+
});
390+
});
391+
392+
// Does *not* prompt when a global CLI install is missing from the system.
393+
await mockHome(async (home) => {
394+
try {
395+
// Temporarily uninstall the global CLI binary from the system.
396+
await silentNpm(['uninstall', '--global', '@angular/cli', `--registry=${testRegistry}`]);
397+
398+
// Setup a fake project directory with a local install of the CLI.
399+
const projectDir = path.join(home, 'project');
400+
await fs.mkdir(projectDir);
401+
await silentNpm(['init', '-y', `--registry=${testRegistry}`], { cwd: projectDir });
402+
await silentNpm(['install', '@angular/cli', `--registry=${testRegistry}`], {
403+
cwd: projectDir,
404+
});
405+
406+
const bashrc = path.join(home, '.bashrc');
407+
await fs.writeFile(bashrc, `# Other content...`);
408+
409+
const localCliBinary = path.join(projectDir, 'node_modules', '.bin', 'ng');
410+
const { stdout } = await execWithEnv(localCliBinary, ['version'], {
411+
...DEFAULT_ENV,
412+
SHELL: '/bin/bash',
413+
HOME: home,
414+
});
415+
416+
if (AUTOCOMPLETION_PROMPT.test(stdout)) {
417+
throw new Error(
418+
'Execution without a global CLI install prompted for autocompletion setup but should' +
419+
' not have.',
420+
);
421+
}
422+
} finally {
423+
// Reinstall global CLI for remainder of the tests.
424+
await silentNpm(['install', '--global', '@angular/cli', `--registry=${testRegistry}`]);
425+
}
426+
});
371427
}
372428

373429
async function windowsTests(): Promise<void> {

tests/legacy-cli/e2e/tests/misc/completion.ts

+54-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,16 @@
1+
import { execFile } from 'child_process';
12
import { promises as fs } from 'fs';
23
import * as os from 'os';
34
import * as path from 'path';
4-
import { execAndCaptureError, execAndWaitForOutputToMatch } from '../../utils/process';
5+
import { getGlobalVariable } from '../../utils/env';
6+
import {
7+
execAndCaptureError,
8+
execAndWaitForOutputToMatch,
9+
execWithEnv,
10+
silentNpm,
11+
} from '../../utils/process';
12+
13+
const testRegistry = getGlobalVariable('package-registry');
514

615
export default async function () {
716
// Windows Cmd and Powershell do not support autocompletion. Run a different set of tests to
@@ -332,6 +341,50 @@ source <(ng completion script)
332341
throw new Error(`Expected unknown \`$SHELL\` error message, but got:\n\n${err.message}`);
333342
}
334343
});
344+
345+
// Does *not* warn when a global CLI install is present on the system.
346+
await mockHome(async (home) => {
347+
const { stdout } = await execWithEnv('ng', ['completion'], {
348+
...process.env,
349+
'SHELL': '/usr/bin/zsh',
350+
'HOME': home,
351+
});
352+
353+
if (stdout.includes('there does not seem to be a global install of the Angular CLI')) {
354+
throw new Error(`CLI warned about missing global install, but one should exist.`);
355+
}
356+
});
357+
358+
// Warns when a global CLI install is *not* present on the system.
359+
await mockHome(async (home) => {
360+
try {
361+
// Temporarily uninstall the global CLI binary from the system.
362+
await silentNpm(['uninstall', '--global', '@angular/cli', `--registry=${testRegistry}`]);
363+
364+
// Setup a fake project directory with a local install of the CLI.
365+
const projectDir = path.join(home, 'project');
366+
await fs.mkdir(projectDir);
367+
await silentNpm(['init', '-y', `--registry=${testRegistry}`], { cwd: projectDir });
368+
await silentNpm(['install', '@angular/cli', `--registry=${testRegistry}`], {
369+
cwd: projectDir,
370+
});
371+
372+
// Invoke the local CLI binary.
373+
const localCliBinary = path.join(projectDir, 'node_modules', '.bin', 'ng');
374+
const { stdout } = await execWithEnv(localCliBinary, ['completion'], {
375+
...process.env,
376+
'SHELL': '/usr/bin/zsh',
377+
'HOME': home,
378+
});
379+
380+
if (stdout.includes('there does not seem to be a global install of the Angular CLI')) {
381+
throw new Error(`CLI warned about missing global install, but one should exist.`);
382+
}
383+
} finally {
384+
// Reinstall global CLI for remainder of the tests.
385+
await silentNpm(['install', '--global', '@angular/cli', `--registry=${testRegistry}`]);
386+
}
387+
});
335388
}
336389

337390
async function windowsTests(): Promise<void> {

tests/legacy-cli/e2e/utils/process.ts

+22-5
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ interface ExecOptions {
1212
waitForMatch?: RegExp;
1313
env?: { [varname: string]: string };
1414
stdin?: string;
15+
cwd?: string;
1516
}
1617

1718
let _processes: child_process.ChildProcess[] = [];
@@ -28,7 +29,7 @@ function _exec(options: ExecOptions, cmd: string, args: string[]): Promise<Proce
2829

2930
let stdout = '';
3031
let stderr = '';
31-
const cwd = process.cwd();
32+
const cwd = options.cwd ?? process.cwd();
3233
const env = options.env;
3334
console.log(
3435
`==========================================================================================`,
@@ -108,8 +109,8 @@ function _exec(options: ExecOptions, cmd: string, args: string[]): Promise<Proce
108109
),
109110
);
110111
});
111-
childProcess.on('error', (error) => {
112-
err.message += `${error}...\n\nSTDOUT:\n${stdout}\n\nSTDERR:\n${stderr}\n`;
112+
childProcess.on('error', (err) => {
113+
err.message += `${err}...\n\nSTDOUT:\n${stdout}\n\nSTDERR:\n${stderr}\n`;
113114
reject(err);
114115
});
115116

@@ -257,8 +258,24 @@ export function silentNg(...args: string[]) {
257258
return _exec({ silent: true }, 'ng', args);
258259
}
259260

260-
export function silentNpm(...args: string[]) {
261-
return _exec({ silent: true }, 'npm', args);
261+
export function silentNpm(...args: string[]): Promise<ProcessOutput>;
262+
export function silentNpm(args: string[], options?: { cwd?: string }): Promise<ProcessOutput>;
263+
export function silentNpm(
264+
...args: string[] | [args: string[], options?: { cwd?: string }]
265+
): Promise<ProcessOutput> {
266+
if (Array.isArray(args[0])) {
267+
const [params, options] = args;
268+
return _exec(
269+
{
270+
silent: true,
271+
cwd: (options as { cwd?: string } | undefined)?.cwd,
272+
},
273+
'npm',
274+
params,
275+
);
276+
} else {
277+
return _exec({ silent: true }, 'npm', args as string[]);
278+
}
262279
}
263280

264281
export function silentYarn(...args: string[]) {

0 commit comments

Comments
 (0)