Skip to content

Commit b79b0f0

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 #23135.
1 parent f6b52e4 commit b79b0f0

File tree

5 files changed

+218
-13
lines changed

5 files changed

+218
-13
lines changed

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

+12-6
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,10 @@
88

99
import { join } from 'path';
1010
import yargs, { Argv } from 'yargs';
11-
import {
12-
CommandModule,
13-
CommandModuleImplementation,
14-
CommandScope,
15-
} from '../../command-builder/command-module';
11+
import { CommandModule, CommandModuleImplementation } from '../../command-builder/command-module';
1612
import { addCommandModuleToYargs } from '../../command-builder/utilities/command';
1713
import { colors } from '../../utilities/color';
18-
import { initializeAutocomplete } from '../../utilities/completion';
14+
import { hasGlobalCliInstall, initializeAutocomplete } from '../../utilities/completion';
1915

2016
export class CompletionCommandModule extends CommandModule implements CommandModuleImplementation {
2117
command = 'completion';
@@ -44,6 +40,16 @@ Appended \`source <(ng completion script)\` to \`${rcFile}\`. Restart your termi
4440
`.trim(),
4541
);
4642

43+
if ((await hasGlobalCliInstall()) === false) {
44+
this.context.logger.warn(
45+
'Setup completed successfully, but there does not seem to be a global install of the' +
46+
' Angular CLI. For autocompletion to work, the CLI will need to be on your `$PATH`, which' +
47+
' is typically done with the `-g` flag in `npm install -g @angular/cli`.' +
48+
'\n\n' +
49+
'For more information, see https://angular.io/cli/completion#global-install',
50+
);
51+
}
52+
4753
return 0;
4854
}
4955
}

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

+69
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
*/
88

99
import { json, logging } 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';
@@ -78,6 +79,16 @@ Appended \`source <(ng completion script)\` to \`${rcFile}\`. Restart your termi
7879
`.trim(),
7980
);
8081

82+
if ((await hasGlobalCliInstall()) === false) {
83+
logger.warn(
84+
'Setup completed successfully, but there does not seem to be a global install of the' +
85+
' Angular CLI. For autocompletion to work, the CLI will need to be on your `$PATH`, which' +
86+
' is typically done with the `-g` flag in `npm install -g @angular/cli`.' +
87+
'\n\n' +
88+
'For more information, see https://angular.io/cli/completion#global-install',
89+
);
90+
}
91+
8192
// Save configuration to remember that the user was prompted.
8293
await setCompletionConfig({ ...completionConfig, prompted: true });
8394

@@ -147,6 +158,12 @@ async function shouldPromptForAutocompletionSetup(
147158
return false; // Unknown shell.
148159
}
149160

161+
// Don't prompt if the user is missing a global CLI install. Autocompletion won't work after setup
162+
// anyway and could be annoying for users running one-off commands via `npx` or using `npm start`.
163+
if ((await hasGlobalCliInstall()) === false) {
164+
return false;
165+
}
166+
150167
// Check each RC file if they already use `ng completion script` in any capacity and don't prompt.
151168
for (const rcFile of rcFiles) {
152169
const contents = await fs.readFile(rcFile, 'utf-8').catch(() => undefined);
@@ -246,3 +263,55 @@ function getShellRunCommandCandidates(shell: string, home: string): string[] | u
246263
return undefined;
247264
}
248265
}
266+
267+
/**
268+
* Returns whether the user has a global CLI install or `undefined` if this can't be determined.
269+
* Execution from `npx` is *not* considered a global CLI install.
270+
*
271+
* This does *not* mean the current execution is from a global CLI install, only that a global
272+
* install exists on the system.
273+
*/
274+
export async function hasGlobalCliInstall(): Promise<boolean | undefined> {
275+
// List all binaries with the `ng` name on the user's `$PATH`.
276+
const proc = execFile('which', ['-a', 'ng']);
277+
let stdout = '';
278+
proc.stdout?.addListener('data', (content) => {
279+
stdout += content;
280+
});
281+
const exitCode = await new Promise<number | null>((resolve) => {
282+
proc.addListener('exit', (exitCode) => {
283+
resolve(exitCode);
284+
});
285+
});
286+
287+
switch (exitCode) {
288+
case 0:
289+
// Successfully listed all `ng` binaries on the `$PATH`. Look for at least one line which is a
290+
// global install. We can't easily identify global installs, but local installs are typically
291+
// placed in `node_modules/.bin` by NPM / Yarn. `npx` also currently caches files at
292+
// `~/.npm/_npx/*/node_modules/.bin/`, so the same logic applies.
293+
const lines = stdout.split('\n').filter((line) => line !== '');
294+
const hasGlobalInstall = lines.some((line) => {
295+
// A binary is a local install if it is a direct child of a `node_modules/.bin/` directory.
296+
const parent = path.parse(path.parse(line).dir);
297+
const grandparent = path.parse(parent.dir);
298+
const localInstall = grandparent.base === 'node_modules' && parent.base === '.bin';
299+
300+
return !localInstall;
301+
});
302+
303+
return hasGlobalInstall;
304+
case 1:
305+
// No instances of `ng` on the user's `$PATH`.
306+
return false;
307+
case null:
308+
// `which` was killed by a signal and did not exit gracefully. Maybe it hung or something else
309+
// went very wrong, so treat this as inconclusive.
310+
return undefined;
311+
default:
312+
// `which` returns exit code 2 if an invalid option is specified and `-a` doesn't appear to be
313+
// supported on all systems. Other exit codes mean unknown errors occurred. Can't tell whether
314+
// CLI is globally installed, so treat this as inconclusive.
315+
return undefined;
316+
}
317+
}

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

+61-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,58 @@ 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 localCliDir = path.join(projectDir, 'node_modules', '.bin');
410+
const localCliBinary = path.join(localCliDir, 'ng');
411+
const pathDirs = process.env['PATH'].split(':');
412+
const pathEnvVar = [...pathDirs, localCliDir].join(':');
413+
const { stdout } = await execWithEnv(localCliBinary, ['version'], {
414+
...DEFAULT_ENV,
415+
SHELL: '/bin/bash',
416+
HOME: home,
417+
PATH: pathEnvVar,
418+
});
419+
420+
if (AUTOCOMPLETION_PROMPT.test(stdout)) {
421+
throw new Error(
422+
'Execution without a global CLI install prompted for autocompletion setup but should' +
423+
' not have.',
424+
);
425+
}
426+
} finally {
427+
// Reinstall global CLI for remainder of the tests.
428+
await silentNpm(['install', '--global', '@angular/cli', `--registry=${testRegistry}`]);
429+
}
430+
});
371431
}
372432

373433
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)