Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit b2b360a

Browse filesBrowse files
sheetalkamatsheetalkamat
authored
Merge pull request #29816 from Microsoft/ti
Use execFileSync in typing installer
2 parents d517713 + bc386c1 commit b2b360a
Copy full SHA for b2b360a

3 files changed

+26-19Lines changed: 26 additions & 19 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎src/testRunner/unittests/tsserver/typingsInstaller.ts‎

Copy file name to clipboardExpand all lines: src/testRunner/unittests/tsserver/typingsInstaller.ts
+6-6Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1684,19 +1684,19 @@ namespace ts.projectSystem {
16841684
TI.getNpmCommandForInstallation(npmPath, tsVersion, packageNames, packageNames.length - Math.ceil(packageNames.length / 2)).command
16851685
];
16861686
it("works when the command is too long to install all packages at once", () => {
1687-
const commands: string[] = [];
1688-
const hasError = TI.installNpmPackages(npmPath, tsVersion, packageNames, command => {
1689-
commands.push(command);
1687+
const commands: [string, string[]][] = [];
1688+
const hasError = TI.installNpmPackages(npmPath, tsVersion, packageNames, (file, args) => {
1689+
commands.push([file, args]);
16901690
return false;
16911691
});
16921692
assert.isFalse(hasError);
16931693
assert.deepEqual(commands, expectedCommands, "commands");
16941694
});
16951695

16961696
it("installs remaining packages when one of the partial command fails", () => {
1697-
const commands: string[] = [];
1698-
const hasError = TI.installNpmPackages(npmPath, tsVersion, packageNames, command => {
1699-
commands.push(command);
1697+
const commands: [string, string[]][] = [];
1698+
const hasError = TI.installNpmPackages(npmPath, tsVersion, packageNames, (file, args) => {
1699+
commands.push([file, args]);
17001700
return commands.length === 1;
17011701
});
17021702
assert.isTrue(hasError);
Collapse file

‎src/typingsInstaller/nodeTypingsInstaller.ts‎

Copy file name to clipboardExpand all lines: src/typingsInstaller/nodeTypingsInstaller.ts
+8-8Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,10 @@ namespace ts.server.typingsInstaller {
7070
cwd: string;
7171
encoding: "utf-8";
7272
}
73-
type ExecSync = (command: string, options: ExecSyncOptions) => string;
73+
type ExecFileSync = (file: string, args: string[], options: ExecSyncOptions) => string;
7474

7575
export class NodeTypingsInstaller extends TypingsInstaller {
76-
private readonly nodeExecSync: ExecSync;
76+
private readonly nodeExecFileSync: ExecFileSync;
7777
private readonly npmPath: string;
7878
readonly typesRegistry: Map<MapLike<string>>;
7979

@@ -97,15 +97,15 @@ namespace ts.server.typingsInstaller {
9797
this.log.writeLine(`Process id: ${process.pid}`);
9898
this.log.writeLine(`NPM location: ${this.npmPath} (explicit '${Arguments.NpmLocation}' ${npmLocation === undefined ? "not " : ""} provided)`);
9999
}
100-
({ execSync: this.nodeExecSync } = require("child_process"));
100+
({ execFileSync: this.nodeExecFileSync } = require("child_process"));
101101

102102
this.ensurePackageDirectoryExists(globalTypingsCacheLocation);
103103

104104
try {
105105
if (this.log.isEnabled()) {
106106
this.log.writeLine(`Updating ${typesRegistryPackageName} npm package...`);
107107
}
108-
this.execSyncAndLog(`${this.npmPath} install --ignore-scripts ${typesRegistryPackageName}@${this.latestDistTag}`, { cwd: globalTypingsCacheLocation });
108+
this.execFileSyncAndLog(this.npmPath, ["install", "--ignore-scripts", `${typesRegistryPackageName}@${this.latestDistTag}`], { cwd: globalTypingsCacheLocation });
109109
if (this.log.isEnabled()) {
110110
this.log.writeLine(`Updated ${typesRegistryPackageName} npm package`);
111111
}
@@ -189,20 +189,20 @@ namespace ts.server.typingsInstaller {
189189
this.log.writeLine(`#${requestId} with arguments'${JSON.stringify(packageNames)}'.`);
190190
}
191191
const start = Date.now();
192-
const hasError = installNpmPackages(this.npmPath, version, packageNames, command => this.execSyncAndLog(command, { cwd }));
192+
const hasError = installNpmPackages(this.npmPath, version, packageNames, (file, args) => this.execFileSyncAndLog(file, args, { cwd }));
193193
if (this.log.isEnabled()) {
194194
this.log.writeLine(`npm install #${requestId} took: ${Date.now() - start} ms`);
195195
}
196196
onRequestCompleted(!hasError);
197197
}
198198

199199
/** Returns 'true' in case of error. */
200-
private execSyncAndLog(command: string, options: Pick<ExecSyncOptions, "cwd">): boolean {
200+
private execFileSyncAndLog(file: string, args: string[], options: Pick<ExecSyncOptions, "cwd">): boolean {
201201
if (this.log.isEnabled()) {
202-
this.log.writeLine(`Exec: ${command}`);
202+
this.log.writeLine(`Exec: ${file} ${args.join(" ")}`);
203203
}
204204
try {
205-
const stdout = this.nodeExecSync(command, { ...options, encoding: "utf-8" });
205+
const stdout = this.nodeExecFileSync(file, args, { ...options, encoding: "utf-8" });
206206
if (this.log.isEnabled()) {
207207
this.log.writeLine(` Succeeded. stdout:${indent(sys.newLine, stdout)}`);
208208
}
Collapse file

‎src/typingsInstallerCore/typingsInstaller.ts‎

Copy file name to clipboardExpand all lines: src/typingsInstallerCore/typingsInstaller.ts
+12-5Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,28 +31,35 @@ namespace ts.server.typingsInstaller {
3131
}
3232

3333
/*@internal*/
34-
export function installNpmPackages(npmPath: string, tsVersion: string, packageNames: string[], install: (command: string) => boolean) {
34+
export function installNpmPackages(npmPath: string, tsVersion: string, packageNames: string[], install: (file: string, args: string[]) => boolean) {
3535
let hasError = false;
3636
for (let remaining = packageNames.length; remaining > 0;) {
3737
const result = getNpmCommandForInstallation(npmPath, tsVersion, packageNames, remaining);
3838
remaining = result.remaining;
39-
hasError = install(result.command) || hasError;
39+
hasError = install(result.command[0], result.command[1]) || hasError;
4040
}
4141
return hasError;
4242
}
4343

44+
function getUserAgent(tsVersion: string) {
45+
return `--user-agent="typesInstaller/${tsVersion}"`;
46+
}
47+
const npmInstall = "install", ignoreScripts = "--ignore-scripts", saveDev = "--save-dev";
48+
const commandBaseLength = npmInstall.length + ignoreScripts.length + saveDev.length + getUserAgent("").length + 5;
4449
/*@internal*/
4550
export function getNpmCommandForInstallation(npmPath: string, tsVersion: string, packageNames: string[], remaining: number) {
4651
const sliceStart = packageNames.length - remaining;
47-
let command: string, toSlice = remaining;
52+
let packages: string[], toSlice = remaining;
4853
while (true) {
49-
command = `${npmPath} install --ignore-scripts ${(toSlice === packageNames.length ? packageNames : packageNames.slice(sliceStart, sliceStart + toSlice)).join(" ")} --save-dev --user-agent="typesInstaller/${tsVersion}"`;
50-
if (command.length < 8000) {
54+
packages = toSlice === packageNames.length ? packageNames : packageNames.slice(sliceStart, sliceStart + toSlice);
55+
const commandLength = npmPath.length + commandBaseLength + packages.join(" ").length + tsVersion.length;
56+
if (commandLength < 8000) {
5157
break;
5258
}
5359

5460
toSlice = toSlice - Math.floor(toSlice / 2);
5561
}
62+
const command: [string, string[]] = [npmPath, [npmInstall, ignoreScripts, ...packages, saveDev, getUserAgent(tsVersion)]];
5663
return { command, remaining: remaining - toSlice };
5764
}
5865

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.