Skip to content

Commit 7ccc89b

Browse files
mjbvzRyanCavanaugh
authored andcommitted
Check to make sure default npm exists at path before trying to use it (#30910)
* Check to make sure default npm exists at path before trying to use it **Bug** If the typings installer is run under a copy of node that does not include npm—but on a machine that does have npm installed—it will incorrectly try to use the npm that does not exist next to its running version of node **Fix** Make sure that we check that npm we select exists before trying to use it as the default. Otherwise, fall back to using plain old `npm` * Add command line flag to gate new behavior * Fix missing semicolon
1 parent d987907 commit 7ccc89b

File tree

3 files changed

+25
-9
lines changed

3 files changed

+25
-9
lines changed

src/jsTyping/shared.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ namespace ts.server {
2121
* typingsInstaller will run the command with `${npmLocation} install ...`.
2222
*/
2323
export const NpmLocation = "--npmLocation";
24+
/**
25+
* Flag indicating that the typings installer should try to validate the default npm location.
26+
* If the default npm is not found when this flag is enabled, fallback to `npm install`
27+
*/
28+
export const ValidateDefaultNpmLocation = "--validateDefaultNpmLocation";
2429
}
2530

2631
export function hasArgument(argumentName: string) {

src/tsserver/server.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@ namespace ts.server {
242242
readonly typingSafeListLocation: string,
243243
readonly typesMapLocation: string,
244244
private readonly npmLocation: string | undefined,
245+
private readonly validateDefaultNpmLocation: boolean,
245246
private event: Event) {
246247
}
247248

@@ -297,6 +298,9 @@ namespace ts.server {
297298
if (this.npmLocation) {
298299
args.push(Arguments.NpmLocation, this.npmLocation);
299300
}
301+
if (this.validateDefaultNpmLocation) {
302+
args.push(Arguments.ValidateDefaultNpmLocation);
303+
}
300304

301305
const execArgv: string[] = [];
302306
for (const arg of process.execArgv) {
@@ -497,7 +501,7 @@ namespace ts.server {
497501

498502
const typingsInstaller = disableAutomaticTypingAcquisition
499503
? undefined
500-
: new NodeTypingsInstaller(telemetryEnabled, logger, host, getGlobalTypingsCacheLocation(), typingSafeListLocation, typesMapLocation, npmLocation, event);
504+
: new NodeTypingsInstaller(telemetryEnabled, logger, host, getGlobalTypingsCacheLocation(), typingSafeListLocation, typesMapLocation, npmLocation, validateDefaultNpmLocation, event);
501505

502506
super({
503507
host,
@@ -933,6 +937,7 @@ namespace ts.server {
933937
const typingSafeListLocation = findArgument(Arguments.TypingSafeListLocation)!; // TODO: GH#18217
934938
const typesMapLocation = findArgument(Arguments.TypesMapLocation) || combinePaths(getDirectoryPath(sys.getExecutingFilePath()), "typesMap.json");
935939
const npmLocation = findArgument(Arguments.NpmLocation);
940+
const validateDefaultNpmLocation = hasArgument(Arguments.ValidateDefaultNpmLocation);
936941

937942
function parseStringArray(argName: string): ReadonlyArray<string> {
938943
const arg = findArgument(argName);

src/typingsInstaller/nodeTypingsInstaller.ts

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,17 @@ namespace ts.server.typingsInstaller {
2929
}
3030

3131
/** Used if `--npmLocation` is not passed. */
32-
function getDefaultNPMLocation(processName: string) {
32+
function getDefaultNPMLocation(processName: string, validateDefaultNpmLocation: boolean, host: InstallTypingHost): string {
3333
if (path.basename(processName).indexOf("node") === 0) {
34-
return `"${path.join(path.dirname(process.argv[0]), "npm")}"`;
35-
}
36-
else {
37-
return "npm";
34+
const npmPath = path.join(path.dirname(process.argv[0]), "npm");
35+
if (!validateDefaultNpmLocation) {
36+
return npmPath;
37+
}
38+
if (host.fileExists(npmPath)) {
39+
return `"${npmPath}"`;
40+
}
3841
}
42+
return "npm";
3943
}
4044

4145
interface TypesRegistryFile {
@@ -79,15 +83,15 @@ namespace ts.server.typingsInstaller {
7983

8084
private delayedInitializationError: InitializationFailedResponse | undefined;
8185

82-
constructor(globalTypingsCacheLocation: string, typingSafeListLocation: string, typesMapLocation: string, npmLocation: string | undefined, throttleLimit: number, log: Log) {
86+
constructor(globalTypingsCacheLocation: string, typingSafeListLocation: string, typesMapLocation: string, npmLocation: string | undefined, validateDefaultNpmLocation: boolean, throttleLimit: number, log: Log) {
8387
super(
8488
sys,
8589
globalTypingsCacheLocation,
8690
typingSafeListLocation ? toPath(typingSafeListLocation, "", createGetCanonicalFileName(sys.useCaseSensitiveFileNames)) : toPath("typingSafeList.json", __dirname, createGetCanonicalFileName(sys.useCaseSensitiveFileNames)),
8791
typesMapLocation ? toPath(typesMapLocation, "", createGetCanonicalFileName(sys.useCaseSensitiveFileNames)) : toPath("typesMap.json", __dirname, createGetCanonicalFileName(sys.useCaseSensitiveFileNames)),
8892
throttleLimit,
8993
log);
90-
this.npmPath = npmLocation !== undefined ? npmLocation : getDefaultNPMLocation(process.argv[0]);
94+
this.npmPath = npmLocation !== undefined ? npmLocation : getDefaultNPMLocation(process.argv[0], validateDefaultNpmLocation, this.installTypingHost);
9195

9296
// If the NPM path contains spaces and isn't wrapped in quotes, do so.
9397
if (stringContains(this.npmPath, " ") && this.npmPath[0] !== `"`) {
@@ -96,6 +100,7 @@ namespace ts.server.typingsInstaller {
96100
if (this.log.isEnabled()) {
97101
this.log.writeLine(`Process id: ${process.pid}`);
98102
this.log.writeLine(`NPM location: ${this.npmPath} (explicit '${Arguments.NpmLocation}' ${npmLocation === undefined ? "not " : ""} provided)`);
103+
this.log.writeLine(`validateDefaultNpmLocation: ${validateDefaultNpmLocation}`);
99104
}
100105
({ execSync: this.nodeExecSync } = require("child_process"));
101106

@@ -229,6 +234,7 @@ namespace ts.server.typingsInstaller {
229234
const typingSafeListLocation = findArgument(Arguments.TypingSafeListLocation);
230235
const typesMapLocation = findArgument(Arguments.TypesMapLocation);
231236
const npmLocation = findArgument(Arguments.NpmLocation);
237+
const validateDefaultNpmLocation = hasArgument(Arguments.ValidateDefaultNpmLocation);
232238

233239
const log = new FileLog(logFilePath);
234240
if (log.isEnabled()) {
@@ -242,7 +248,7 @@ namespace ts.server.typingsInstaller {
242248
}
243249
process.exit(0);
244250
});
245-
const installer = new NodeTypingsInstaller(globalTypingsCacheLocation!, typingSafeListLocation!, typesMapLocation!, npmLocation, /*throttleLimit*/5, log); // TODO: GH#18217
251+
const installer = new NodeTypingsInstaller(globalTypingsCacheLocation!, typingSafeListLocation!, typesMapLocation!, npmLocation, validateDefaultNpmLocation, /*throttleLimit*/5, log); // TODO: GH#18217
246252
installer.listen();
247253

248254
function indent(newline: string, str: string): string {

0 commit comments

Comments
 (0)