From 4b9e5544947e826f01e28b1b15138c5c0e4b9028 Mon Sep 17 00:00:00 2001 From: Jason Ramsay Date: Fri, 16 Sep 2016 15:17:11 -0700 Subject: [PATCH 1/3] Add throttle limit to typings installer requests --- .../typingsInstaller/nodeTypingsInstaller.ts | 59 +++++++++++++++---- 1 file changed, 49 insertions(+), 10 deletions(-) diff --git a/src/server/typingsInstaller/nodeTypingsInstaller.ts b/src/server/typingsInstaller/nodeTypingsInstaller.ts index 0d22b1b36436f..1e3448e06856e 100644 --- a/src/server/typingsInstaller/nodeTypingsInstaller.ts +++ b/src/server/typingsInstaller/nodeTypingsInstaller.ts @@ -1,8 +1,14 @@ -/// +/// /// namespace ts.server.typingsInstaller { + interface RunInstallRequest { + readonly cachePath: string; + readonly typingsToInstall: string[]; + readonly postInstallAction: (installedTypings: string[]) => void; + } + const throttleLimit = 5; const fs: { appendFileSync(file: string, content: string): void } = require("fs"); @@ -25,6 +31,8 @@ namespace ts.server.typingsInstaller { private npmBinPath: string; private installRunCount = 1; + private throttleCount = 0; + private delayedRunInstallRequests: RunInstallRequest[] = []; readonly installTypingHost: InstallTypingHost = sys; constructor(globalTypingsCacheLocation: string, log: Log) { @@ -90,24 +98,55 @@ namespace ts.server.typingsInstaller { } protected runInstall(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void): void { + if (this.throttleCount === throttleLimit) { + const request = { + cachePath: cachePath, + typingsToInstall: typingsToInstall, + postInstallAction: postInstallAction + }; + this.delayedRunInstallRequests.push(request); + return; + } const id = this.installRunCount; this.installRunCount++; let execInstallCmdCount = 0; const filteredTypings: string[] = []; + const delayedTypingsToInstall: string[] = []; for (const typing of typingsToInstall) { - const command = `npm view @types/${typing} --silent name`; - this.execAsync("npm view", command, cachePath, id, (err, stdout, stderr) => { - if (stdout) { - filteredTypings.push(typing); - } - execInstallCmdCount++; - if (execInstallCmdCount === typingsToInstall.length) { - installFilteredTypings(this, filteredTypings); - } + if (this.throttleCount === throttleLimit) { + delayedTypingsToInstall.push(typing); + continue; + } + execNpmViewTyping(this, typing); + } + + function execNpmViewTyping(self: NodeTypingsInstaller, typing: string) { + self.throttleCount++; + const command = `npm view @types/${typing} --silent name`; + self.execAsync("npm view", command, cachePath, id, (err, stdout, stderr) => { + if (stdout) { + filteredTypings.push(typing); + } + execInstallCmdCount++; + self.throttleCount--; + if (delayedTypingsToInstall.length > 0) { + return execNpmViewTyping(self, delayedTypingsToInstall.pop()); + } + if (execInstallCmdCount === typingsToInstall.length) { + installFilteredTypings(self, filteredTypings); + if (self.delayedRunInstallRequests.length > 0) { + const request = self.delayedRunInstallRequests.pop(); + return self.runInstall(request.cachePath, request.typingsToInstall, request.postInstallAction); + } + } }); } function installFilteredTypings(self: NodeTypingsInstaller, filteredTypings: string[]) { + if (filteredTypings.length === 0) { + reportInstalledTypings(self); + return; + } const command = `npm install ${filteredTypings.map(t => "@types/" + t).join(" ")} --save-dev`; self.execAsync("npm install", command, cachePath, id, (err, stdout, stderr) => { if (stdout) { From 609e56ed8e533cd2a37a85f357f919dc7c74326c Mon Sep 17 00:00:00 2001 From: Jason Ramsay Date: Mon, 19 Sep 2016 13:56:30 -0700 Subject: [PATCH 2/3] - Reorganize nodeTypingsInstaller and typingsInstaller for testing purposes - Add throttle tests - Add full npm path --- .../unittests/tsserverProjectSystem.ts | 54 ++- src/harness/unittests/typingsInstaller.ts | 387 ++++++++++++++++-- .../typingsInstaller/nodeTypingsInstaller.ts | 122 +----- .../typingsInstaller/typingsInstaller.ts | 95 ++++- 4 files changed, 478 insertions(+), 180 deletions(-) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index e22f1752ce440..5275c4c89fc9f 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -13,6 +13,13 @@ namespace ts.projectSystem { }) }; + export interface PostExecAction { + readonly error: Error; + readonly stdout: string; + readonly stderr: string; + readonly callback: (err: Error, stdout: string, stderr: string) => void; + } + export function notImplemented(): any { throw new Error("Not yet implemented"); } @@ -47,13 +54,13 @@ namespace ts.projectSystem { } safeFileList = safeList.path; - postInstallActions: ((map: (t: string[]) => string[]) => void)[] = []; + protected postExecActions: PostExecAction[] = []; - runPostInstallActions(map: (t: string[]) => string[]) { - for (const f of this.postInstallActions) { - f(map); + runPostExecActions() { + for (const action of this.postExecActions) { + action.callback(action.error, action.stdout, action.stderr); } - this.postInstallActions = []; + this.postExecActions = []; } onProjectClosed(p: server.Project) { @@ -67,14 +74,16 @@ namespace ts.projectSystem { return this.installTypingHost; } - isPackageInstalled(packageName: string) { - return true; - } - - runInstall(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void) { - this.postInstallActions.push(map => { - postInstallAction(map(typingsToInstall)); - }); + execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void): void { + switch (prefix) { + case "npm view": + case "npm install": + case "npm ls": + break; + default: + throw new Error("TypingsInstaller: execAsync command not yet implemented"); + } + this.addPostExecAction("success", cb); } sendResponse(response: server.SetTypings | server.InvalidateCachedTypings) { @@ -85,6 +94,25 @@ namespace ts.projectSystem { const request = server.createInstallTypingsRequest(project, typingOptions, this.globalTypingsCacheLocation); this.install(request); } + + addPostExecAction(stdout: string | string[], cb: (err: Error, stdout: string, stderr: string) => void) { + const out = typeof stdout === "string" ? stdout : createNpmPackageJsonString(stdout); + const action: PostExecAction = { + error: undefined, + stdout: out, + stderr: "", + callback: cb + }; + this.postExecActions.push(action); + } + } + + function createNpmPackageJsonString(installedTypings: string[]): string { + const dependencies: MapLike = {}; + for (const typing of installedTypings) { + dependencies[typing] = "1.0.0"; + } + return JSON.stringify({ dependencies: dependencies }); } export function getExecutingFilePathFromLibFile(libFilePath: string): string { diff --git a/src/harness/unittests/typingsInstaller.ts b/src/harness/unittests/typingsInstaller.ts index 47373bfd26173..4ff70e0d3dc5b 100644 --- a/src/harness/unittests/typingsInstaller.ts +++ b/src/harness/unittests/typingsInstaller.ts @@ -1,9 +1,26 @@ -/// +/// /// /// namespace ts.projectSystem { - describe("typings installer", () => { + describe("typingsInstaller", () => { + function execHelper(self: TestTypingsInstaller, host: TestServerHost, installedTypings: string[], typingFiles: FileOrFolder[], prefix: string, cb: (err: Error, stdout: string, stderr: string) => void): boolean { + let execSuper = true; + switch (prefix) { + case "npm install": + for (const file of typingFiles) { + host.createFileOrFolder(file, /*createParentDirectory*/ true); + } + break; + case "npm ls": + self.addPostExecAction(installedTypings, cb); + execSuper = false; + break; + default: + break; + } + return execSuper; + } it("configured projects (typings installed) 1", () => { const file1 = { path: "/a/b/app.js", @@ -34,9 +51,20 @@ namespace ts.projectSystem { path: "/a/data/node_modules/@types/jquery/index.d.ts", content: "declare const $: { x: number }" }; - const host = createServerHost([file1, tsconfig, packageJson]); - const installer = new TestTypingsInstaller("/a/data/", host); + const installer = new (class extends TestTypingsInstaller { + constructor() { + super("/a/data/", host); + } + execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void): void { + const installedTypings = ["@types/jquery"]; + const typingFiles = [jquery]; + if (execHelper(this, host, installedTypings, typingFiles, prefix, cb)) { + super.execAsync(prefix, command, cwd, requestId, cb); + } + } + })(); + const projectService = createProjectService(host, { useSingleInferredProject: true, typingsInstaller: installer }); projectService.openClientFile(file1.path); @@ -46,11 +74,8 @@ namespace ts.projectSystem { assert(host.fileExists(combinePaths(installer.globalTypingsCacheLocation, "package.json"))); - installer.runPostInstallActions(t => { - assert.deepEqual(t, ["jquery"]); - host.createFileOrFolder(jquery, /*createParentDirectory*/ true); - return ["@types/jquery"]; - }); + installer.runPostExecActions(); + checkNumberOfProjects(projectService, { configuredProjects: 1 }); checkProjectActualFiles(p, [file1.path, jquery.path]); }); @@ -75,7 +100,18 @@ namespace ts.projectSystem { content: "declare const $: { x: number }" }; const host = createServerHost([file1, packageJson]); - const installer = new TestTypingsInstaller("/a/data/", host); + const installer = new (class extends TestTypingsInstaller { + constructor() { + super("/a/data/", host); + } + execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void): void { + const installedTypings = ["@types/jquery"]; + const typingFiles = [jquery]; + if (execHelper(this, host, installedTypings, typingFiles, prefix, cb)) { + super.execAsync(prefix, command, cwd, requestId, cb); + } + } + })(); const projectService = createProjectService(host, { useSingleInferredProject: true, typingsInstaller: installer }); projectService.openClientFile(file1.path); @@ -86,11 +122,8 @@ namespace ts.projectSystem { assert(host.fileExists(combinePaths(installer.globalTypingsCacheLocation, "package.json"))); - installer.runPostInstallActions(t => { - assert.deepEqual(t, ["jquery"]); - host.createFileOrFolder(jquery, /*createParentDirectory*/ true); - return ["@types/jquery"]; - }); + installer.runPostExecActions(); + checkNumberOfProjects(projectService, { inferredProjects: 1 }); checkProjectActualFiles(p, [file1.path, jquery.path]); }); @@ -155,6 +188,10 @@ namespace ts.projectSystem { path: "/a/b/app.ts", content: "" }; + const jquery = { + path: "/a/data/node_modules/@types/jquery/index.d.ts", + content: "declare const $: { x: number }" + }; const host = createServerHost([file1]); let enqueueIsCalled = false; let runInstallIsCalled = false; @@ -166,10 +203,15 @@ namespace ts.projectSystem { enqueueIsCalled = true; super.enqueueInstallTypingsRequest(project, typingOptions); } - runInstall(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void): void { - assert.deepEqual(typingsToInstall, ["node"]); - runInstallIsCalled = true; - super.runInstall(cachePath, typingsToInstall, postInstallAction); + execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void): void { + const installedTypings = ["@types/jquery"]; + const typingFiles = [jquery]; + if (execHelper(this, host, installedTypings, typingFiles, prefix, cb)) { + super.execAsync(prefix, command, cwd, requestId, cb); + } + if (prefix === "npm install") { + runInstallIsCalled = true; + } } })(); @@ -181,6 +223,9 @@ namespace ts.projectSystem { rootFiles: [toExternalFile(file1.path)], typingOptions: { enableAutoDiscovery: true, include: ["node"] } }); + + installer.runPostExecActions(); + // autoDiscovery is set in typing options - use it even if project contains only .ts files projectService.checkNumberOfProjects({ externalProjects: 1 }); assert.isTrue(enqueueIsCalled, "expected 'enqueueIsCalled' to be true"); @@ -214,7 +259,18 @@ namespace ts.projectSystem { }; const host = createServerHost([file1, file2, file3]); - const installer = new TestTypingsInstaller("/a/data/", host); + const installer = new (class extends TestTypingsInstaller { + constructor() { + super("/a/data/", host); + } + execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void): void { + const installedTypings = ["@types/lodash", "@types/react"]; + const typingFiles = [lodash, react]; + if (execHelper(this, host, installedTypings, typingFiles, prefix, cb)) { + super.execAsync(prefix, command, cwd, requestId, cb); + } + } + })(); const projectFileName = "/a/app/test.csproj"; const projectService = createProjectService(host, { typingsInstaller: installer }); @@ -229,12 +285,7 @@ namespace ts.projectSystem { projectService.checkNumberOfProjects({ externalProjects: 1 }); checkProjectActualFiles(p, [file1.path, file2.path, file3.path]); - installer.runPostInstallActions(t => { - assert.deepEqual(t, ["lodash", "react"]); - host.createFileOrFolder(lodash, /*createParentDirectory*/ true); - host.createFileOrFolder(react, /*createParentDirectory*/ true); - return ["@types/lodash", "@types/react"]; - }); + installer.runPostExecActions(); checkNumberOfProjects(projectService, { externalProjects: 1 }); checkProjectActualFiles(p, [file1.path, file2.path, file3.path, lodash.path, react.path]); @@ -255,7 +306,6 @@ namespace ts.projectSystem { const host = createServerHost([file1, file2]); let enqueueIsCalled = false; let runInstallIsCalled = false; - let runPostInstallIsCalled = false; const installer = new (class extends TestTypingsInstaller { constructor() { super("/a/data/", host); @@ -264,9 +314,15 @@ namespace ts.projectSystem { enqueueIsCalled = true; super.enqueueInstallTypingsRequest(project, typingOptions); } - runInstall(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void): void { - runInstallIsCalled = true; - super.runInstall(cachePath, typingsToInstall, postInstallAction); + execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void): void { + const installedTypings: string[] = []; + const typingFiles: FileOrFolder[] = []; + if (execHelper(this, host, installedTypings, typingFiles, prefix, cb)) { + super.execAsync(prefix, command, cwd, requestId, cb); + } + if (prefix === "npm install") { + runInstallIsCalled = true; + } } })(); @@ -283,16 +339,12 @@ namespace ts.projectSystem { projectService.checkNumberOfProjects({ externalProjects: 1 }); checkProjectActualFiles(p, [file1.path, file2.path]); - installer.runPostInstallActions(t => { - runPostInstallIsCalled = true; - return []; - }); + installer.runPostExecActions(); checkNumberOfProjects(projectService, { externalProjects: 1 }); checkProjectActualFiles(p, [file1.path, file2.path]); assert.isFalse(enqueueIsCalled, "expected 'enqueueIsCalled' to be false"); assert.isFalse(runInstallIsCalled, "expected 'runInstallIsCalled' to be false"); - assert.isFalse(runPostInstallIsCalled, "expected 'runPostInstallIsCalled' to be false"); }); it("external project - with typing options, with only js, d.ts files", () => { @@ -340,7 +392,18 @@ namespace ts.projectSystem { }; const host = createServerHost([file1, file2, file3, packageJson]); - const installer = new TestTypingsInstaller("/a/data/", host); + const installer = new (class extends TestTypingsInstaller { + constructor() { + super("/a/data/", host); + } + execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void): void { + const installedTypings = ["@types/commander", "@types/express", "@types/jquery", "@types/moment"]; + const typingFiles = [commander, express, jquery, moment]; + if (execHelper(this, host, installedTypings, typingFiles, prefix, cb)) { + super.execAsync(prefix, command, cwd, requestId, cb); + } + } + })(); const projectFileName = "/a/app/test.csproj"; const projectService = createProjectService(host, { typingsInstaller: installer }); @@ -355,17 +418,253 @@ namespace ts.projectSystem { projectService.checkNumberOfProjects({ externalProjects: 1 }); checkProjectActualFiles(p, [file1.path, file2.path, file3.path]); - installer.runPostInstallActions(t => { - assert.deepEqual(t, ["jquery", "moment", "express", "commander" ]); - host.createFileOrFolder(commander, /*createParentDirectory*/ true); - host.createFileOrFolder(express, /*createParentDirectory*/ true); - host.createFileOrFolder(jquery, /*createParentDirectory*/ true); - host.createFileOrFolder(moment, /*createParentDirectory*/ true); - return ["@types/commander", "@types/express", "@types/jquery", "@types/moment"]; - }); + installer.runPostExecActions(); checkNumberOfProjects(projectService, { externalProjects: 1 }); checkProjectActualFiles(p, [file1.path, file2.path, file3.path, commander.path, express.path, jquery.path, moment.path]); }); + + it("Throttle - delayed typings to install", () => { + const file1 = { + path: "/a/b/lodash.js", + content: "" + }; + const file2 = { + path: "/a/b/commander.js", + content: "" + }; + const file3 = { + path: "/a/b/file3.d.ts", + content: "" + }; + const packageJson = { + path: "/a/b/package.json", + content: JSON.stringify({ + name: "test", + dependencies: { + express: "^3.1.0", + cordova: "1.0.0", + grunt: "1.0.0", + gulp: "1.0.0", + forever: "1.0.0", + } + }) + }; + + const commander = { + path: "/a/data/node_modules/@types/commander/index.d.ts", + content: "declare const commander: { x: number }" + }; + const express = { + path: "/a/data/node_modules/@types/express/index.d.ts", + content: "declare const express: { x: number }" + }; + const jquery = { + path: "/a/data/node_modules/@types/jquery/index.d.ts", + content: "declare const jquery: { x: number }" + }; + const moment = { + path: "/a/data/node_modules/@types/moment/index.d.ts", + content: "declare const moment: { x: number }" + }; + const lodash = { + path: "/a/data/node_modules/@types/lodash/index.d.ts", + content: "declare const lodash: { x: number }" + }; + const cordova = { + path: "/a/data/node_modules/@types/cordova/index.d.ts", + content: "declare const cordova: { x: number }" + }; + const grunt = { + path: "/a/data/node_modules/@types/grunt/index.d.ts", + content: "declare const grunt: { x: number }" + }; + const gulp = { + path: "/a/data/node_modules/@types/gulp/index.d.ts", + content: "declare const gulp: { x: number }" + }; + const forever = { + path: "/a/data/node_modules/@types/forever/index.d.ts", + content: "declare const forever: { x: number }" + }; + + let npmViewCount = 0; + let npmInstallCount = 0; + const host = createServerHost([file1, file2, file3, packageJson]); + const installer = new (class extends TestTypingsInstaller { + constructor() { + super("/a/data/", host); + } + execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void): void { + if (prefix === "npm view") { + npmViewCount++; + } + if (prefix === "npm install") { + npmInstallCount++; + } + const installedTypings = ["@types/commander", "@types/express", "@types/jquery", "@types/moment", "@types/lodash", "@types/cordova", "@types/grunt", "@types/gulp", "@types/forever"]; + const typingFiles = [commander, express, jquery, moment, lodash, cordova, grunt, gulp, forever]; + if (execHelper(this, host, installedTypings, typingFiles, prefix, cb)) { + super.execAsync(prefix, command, cwd, requestId, cb); + } + } + })(); + + const projectFileName = "/a/app/test.csproj"; + const projectService = createProjectService(host, { typingsInstaller: installer }); + projectService.openExternalProject({ + projectFileName, + options: { allowJS: true, moduleResolution: ModuleResolutionKind.NodeJs }, + rootFiles: [toExternalFile(file1.path), toExternalFile(file2.path), toExternalFile(file3.path)], + typingOptions: { include: ["jquery", "moment"] } + }); + + const p = projectService.externalProjects[0]; + projectService.checkNumberOfProjects({ externalProjects: 1 }); + checkProjectActualFiles(p, [file1.path, file2.path, file3.path]); + // The npm view count should be 5 even though there are 9 typings to acquire. + // The throttle limit has been reached so no more execAsync requests will be + // queued until these have been processed. + assert.isTrue(npmViewCount === 5); + assert.isTrue(npmInstallCount === 0); + + installer.runPostExecActions(); + + assert.isTrue(npmViewCount === 9); + assert.isTrue(npmInstallCount === 1); + checkNumberOfProjects(projectService, { externalProjects: 1 }); + checkProjectActualFiles(p, [file1.path, file2.path, file3.path, commander.path, express.path, jquery.path, moment.path, lodash.path, cordova.path, grunt.path, gulp.path, forever.path]); + }); + + it("Throttle - delayed run install requests", () => { + const file1 = { + path: "/a/b/lodash.js", + content: "" + }; + const file2 = { + path: "/a/b/commander.js", + content: "" + }; + const file3 = { + path: "/a/b/file3.d.ts", + content: "" + }; + + const commander = { + path: "/a/data/node_modules/@types/commander/index.d.ts", + content: "declare const commander: { x: number }" + }; + const express = { + path: "/a/data/node_modules/@types/express/index.d.ts", + content: "declare const express: { x: number }" + }; + const jquery = { + path: "/a/data/node_modules/@types/jquery/index.d.ts", + content: "declare const jquery: { x: number }" + }; + const moment = { + path: "/a/data/node_modules/@types/moment/index.d.ts", + content: "declare const moment: { x: number }" + }; + const lodash = { + path: "/a/data/node_modules/@types/lodash/index.d.ts", + content: "declare const lodash: { x: number }" + }; + const cordova = { + path: "/a/data/node_modules/@types/cordova/index.d.ts", + content: "declare const cordova: { x: number }" + }; + const grunt = { + path: "/a/data/node_modules/@types/grunt/index.d.ts", + content: "declare const grunt: { x: number }" + }; + const gulp = { + path: "/a/data/node_modules/@types/gulp/index.d.ts", + content: "declare const gulp: { x: number }" + }; + const forever = { + path: "/a/data/node_modules/@types/forever/index.d.ts", + content: "declare const forever: { x: number }" + }; + + let npmViewCount = 0; + let npmInstallCount = 0; + let hasRunInstall2Typings = false; + const host = createServerHost([file1, file2, file3]); + const installer = new (class extends TestTypingsInstaller { + constructor() { + super("/a/data/", host); + } + execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void): void { + switch (prefix) { + case "npm view": + if (command.match("grunt|gulp|forever")) { + hasRunInstall2Typings = true; + } + npmViewCount++; + break; + case "npm install": + npmInstallCount++; + break; + } + + let installedTypings: string[]; + let typingFiles: FileOrFolder[]; + if (npmInstallCount <= 1) { + installedTypings = ["@types/commander", "@types/express", "@types/jquery", "@types/moment", "@types/lodash", "@types/cordova"]; + typingFiles = [commander, express, jquery, moment, lodash, cordova]; + } + else { + installedTypings = ["@types/grunt", "@types/gulp", "@types/forever"]; + typingFiles = [grunt, gulp, forever]; + } + + if (execHelper(this, host, installedTypings, typingFiles, prefix, cb)) { + super.execAsync(prefix, command, cwd, requestId, cb); + } + } + })(); + + // Create project #1 with 6 typings + const projectService = createProjectService(host, { typingsInstaller: installer }); + const projectFileName1 = "/a/app/test1.csproj"; + projectService.openExternalProject({ + projectFileName: projectFileName1, + options: { allowJS: true, moduleResolution: ModuleResolutionKind.NodeJs }, + rootFiles: [toExternalFile(file1.path), toExternalFile(file2.path), toExternalFile(file3.path)], + typingOptions: { include: ["jquery", "moment", "express", "cordova"] } + }); + + // Create project #2 with 3 typings + const projectFileName2 = "/a/app/test2.csproj"; + projectService.openExternalProject({ + projectFileName: projectFileName2, + options: { allowJS: true, moduleResolution: ModuleResolutionKind.NodeJs }, + rootFiles: [toExternalFile(file3.path)], + typingOptions: { include: ["grunt", "gulp", "forever"] } + }); + + const p1 = projectService.externalProjects[0]; + const p2 = projectService.externalProjects[1]; + projectService.checkNumberOfProjects({ externalProjects: 2 }); + checkProjectActualFiles(p1, [file1.path, file2.path, file3.path]); + checkProjectActualFiles(p2, [file3.path]); + + // The npm view count should be 5 even though there are 9 typings to acquire. + // The throttle limit has been reached so no more run execAsync requests will be + // queued until these have been processed. Assert that typings from the second + // run install request have not been queued. + assert.isTrue(npmViewCount === 5); + assert.isTrue(npmInstallCount === 0); + assert.isFalse(hasRunInstall2Typings); + + installer.runPostExecActions(); + + assert.isTrue(npmViewCount === 9); + assert.isTrue(npmInstallCount === 2); + assert.isTrue(hasRunInstall2Typings); + checkProjectActualFiles(p1, [file1.path, file2.path, file3.path, commander.path, express.path, jquery.path, moment.path, lodash.path, cordova.path]); + checkProjectActualFiles(p2, [file3.path, grunt.path, gulp.path, forever.path]); + }); }); } \ No newline at end of file diff --git a/src/server/typingsInstaller/nodeTypingsInstaller.ts b/src/server/typingsInstaller/nodeTypingsInstaller.ts index 1e3448e06856e..3dc4be7e3dad4 100644 --- a/src/server/typingsInstaller/nodeTypingsInstaller.ts +++ b/src/server/typingsInstaller/nodeTypingsInstaller.ts @@ -2,13 +2,6 @@ /// namespace ts.server.typingsInstaller { - interface RunInstallRequest { - readonly cachePath: string; - readonly typingsToInstall: string[]; - readonly postInstallAction: (installedTypings: string[]) => void; - } - - const throttleLimit = 5; const fs: { appendFileSync(file: string, content: string): void } = require("fs"); @@ -26,13 +19,8 @@ namespace ts.server.typingsInstaller { } export class NodeTypingsInstaller extends TypingsInstaller { - private execSync: { (command: string, options: { stdio: "ignore" | "pipe", cwd?: string }): Buffer | string }; private exec: { (command: string, options: { cwd: string }, callback?: (error: Error, stdout: string, stderr: string) => void): any }; - private npmBinPath: string; - private installRunCount = 1; - private throttleCount = 0; - private delayedRunInstallRequests: RunInstallRequest[] = []; readonly installTypingHost: InstallTypingHost = sys; constructor(globalTypingsCacheLocation: string, log: Log) { @@ -40,25 +28,12 @@ namespace ts.server.typingsInstaller { if (this.log.isEnabled()) { this.log.writeLine(`Process id: ${process.pid}`); } - const { exec, execSync } = require("child_process"); - this.execSync = execSync; + const { exec } = require("child_process"); this.exec = exec; } init() { super.init(); - try { - this.npmBinPath = this.execSync("npm -g bin", { stdio: "pipe" }).toString().trim(); - if (this.log.isEnabled()) { - this.log.writeLine(`Global npm bin path '${this.npmBinPath}'`); - } - } - catch (e) { - this.npmBinPath = ""; - if (this.log.isEnabled()) { - this.log.writeLine(`Error when getting npm bin path: ${e}. Set bin path to ""`); - } - } process.on("message", (req: DiscoverTypings | CloseProject) => { switch (req.kind) { case "discover": @@ -70,23 +45,6 @@ namespace ts.server.typingsInstaller { }); } - protected isPackageInstalled(packageName: string) { - try { - const output = this.execSync(`npm list --silent --global --depth=1 ${packageName}`, { stdio: "pipe" }).toString(); - if (this.log.isEnabled()) { - this.log.writeLine(`IsPackageInstalled::stdout '${output}'`); - } - return true; - } - catch (e) { - if (this.log.isEnabled()) { - this.log.writeLine(`IsPackageInstalled::err::stdout '${e.stdout && e.stdout.toString()}'`); - this.log.writeLine(`IsPackageInstalled::err::stderr '${e.stdout && e.stderr.toString()}'`); - } - return false; - } - } - protected sendResponse(response: SetTypings | InvalidateCachedTypings) { if (this.log.isEnabled()) { this.log.writeLine(`Sending response: ${JSON.stringify(response)}`); @@ -97,83 +55,7 @@ namespace ts.server.typingsInstaller { } } - protected runInstall(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void): void { - if (this.throttleCount === throttleLimit) { - const request = { - cachePath: cachePath, - typingsToInstall: typingsToInstall, - postInstallAction: postInstallAction - }; - this.delayedRunInstallRequests.push(request); - return; - } - const id = this.installRunCount; - this.installRunCount++; - let execInstallCmdCount = 0; - const filteredTypings: string[] = []; - const delayedTypingsToInstall: string[] = []; - for (const typing of typingsToInstall) { - if (this.throttleCount === throttleLimit) { - delayedTypingsToInstall.push(typing); - continue; - } - execNpmViewTyping(this, typing); - } - - function execNpmViewTyping(self: NodeTypingsInstaller, typing: string) { - self.throttleCount++; - const command = `npm view @types/${typing} --silent name`; - self.execAsync("npm view", command, cachePath, id, (err, stdout, stderr) => { - if (stdout) { - filteredTypings.push(typing); - } - execInstallCmdCount++; - self.throttleCount--; - if (delayedTypingsToInstall.length > 0) { - return execNpmViewTyping(self, delayedTypingsToInstall.pop()); - } - if (execInstallCmdCount === typingsToInstall.length) { - installFilteredTypings(self, filteredTypings); - if (self.delayedRunInstallRequests.length > 0) { - const request = self.delayedRunInstallRequests.pop(); - return self.runInstall(request.cachePath, request.typingsToInstall, request.postInstallAction); - } - } - }); - } - - function installFilteredTypings(self: NodeTypingsInstaller, filteredTypings: string[]) { - if (filteredTypings.length === 0) { - reportInstalledTypings(self); - return; - } - const command = `npm install ${filteredTypings.map(t => "@types/" + t).join(" ")} --save-dev`; - self.execAsync("npm install", command, cachePath, id, (err, stdout, stderr) => { - if (stdout) { - reportInstalledTypings(self); - } - }); - } - - function reportInstalledTypings(self: NodeTypingsInstaller) { - const command = "npm ls -json"; - self.execAsync("npm ls", command, cachePath, id, (err, stdout, stderr) => { - let installedTypings: string[]; - try { - const response = JSON.parse(stdout); - if (response.dependencies) { - installedTypings = getOwnKeys(response.dependencies); - } - } - catch (e) { - self.log.writeLine(`Error parsing installed @types dependencies. Error details: ${e.message}`); - } - postInstallAction(installedTypings || []); - }); - } - } - - private execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void) { + protected execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void) { if (this.log.isEnabled()) { this.log.writeLine(`#${requestId} running command '${command}'.`); } diff --git a/src/server/typingsInstaller/typingsInstaller.ts b/src/server/typingsInstaller/typingsInstaller.ts index e4870a568c4f7..24139436932fe 100644 --- a/src/server/typingsInstaller/typingsInstaller.ts +++ b/src/server/typingsInstaller/typingsInstaller.ts @@ -1,4 +1,4 @@ -/// +/// /// /// /// @@ -8,11 +8,19 @@ namespace ts.server.typingsInstaller { devDependencies: MapLike; } + interface RunInstallRequest { + readonly cachePath: string; + readonly typingsToInstall: string[]; + readonly postInstallAction: (installedTypings: string[]) => void; + } + export interface Log { isEnabled(): boolean; writeLine(text: string): void; } + const throttleLimit = 5; + const nullLog: Log = { isEnabled: () => false, writeLine: () => {} @@ -24,10 +32,14 @@ namespace ts.server.typingsInstaller { } export abstract class TypingsInstaller { + private installRunCount = 1; + private throttleCount = 0; private packageNameToTypingLocation: Map = createMap(); private missingTypingsSet: Map = createMap(); private knownCachesSet: Map = createMap(); private projectWatchers: Map = createMap(); + private delayedRunInstallRequests: RunInstallRequest[] = []; + private npmPath: string; abstract readonly installTypingHost: InstallTypingHost; @@ -38,6 +50,8 @@ namespace ts.server.typingsInstaller { } init() { + const path = require("path"); + this.npmPath = `"${path.join(path.dirname(process.argv[0]), "npm")}"`; this.processCacheLocation(this.globalCachePath); } @@ -224,6 +238,82 @@ namespace ts.server.typingsInstaller { }); } + private runInstall(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void): void { + if (this.throttleCount === throttleLimit) { + const request = { + cachePath: cachePath, + typingsToInstall: typingsToInstall, + postInstallAction: postInstallAction + }; + this.delayedRunInstallRequests.push(request); + return; + } + const id = this.installRunCount; + this.installRunCount++; + let execInstallCmdCount = 0; + const filteredTypings: string[] = []; + const delayedTypingsToInstall: string[] = []; + for (const typing of typingsToInstall) { + if (this.throttleCount === throttleLimit) { + delayedTypingsToInstall.push(typing); + continue; + } + execNpmViewTyping(this, typing); + } + + function execNpmViewTyping(self: TypingsInstaller, typing: string) { + self.throttleCount++; + const command = `${self.npmPath} view @types/${typing} --silent name`; + self.execAsync("npm view", command, cachePath, id, (err, stdout, stderr) => { + if (stdout) { + filteredTypings.push(typing); + } + execInstallCmdCount++; + self.throttleCount--; + if (delayedTypingsToInstall.length > 0) { + return execNpmViewTyping(self, delayedTypingsToInstall.pop()); + } + if (execInstallCmdCount === typingsToInstall.length) { + installFilteredTypings(self, filteredTypings); + if (self.delayedRunInstallRequests.length > 0) { + const request = self.delayedRunInstallRequests.pop(); + return self.runInstall(request.cachePath, request.typingsToInstall, request.postInstallAction); + } + } + }); + } + + function installFilteredTypings(self: TypingsInstaller, filteredTypings: string[]) { + if (filteredTypings.length === 0) { + reportInstalledTypings(self); + return; + } + const command = `${self.npmPath} install ${filteredTypings.map(t => "@types/" + t).join(" ")} --save-dev`; + self.execAsync("npm install", command, cachePath, id, (err, stdout, stderr) => { + if (stdout) { + reportInstalledTypings(self); + } + }); + } + + function reportInstalledTypings(self: TypingsInstaller) { + const command = `${self.npmPath} ls -json`; + self.execAsync("npm ls", command, cachePath, id, (err, stdout, stderr) => { + let installedTypings: string[]; + try { + const response = JSON.parse(stdout); + if (response.dependencies) { + installedTypings = getOwnKeys(response.dependencies); + } + } + catch (e) { + self.log.writeLine(`Error parsing installed @types dependencies. Error details: ${e.message}`); + } + postInstallAction(installedTypings || []); + }); + } + } + private ensureDirectoryExists(directory: string, host: InstallTypingHost): void { const directoryName = getDirectoryPath(directory); if (!host.directoryExists(directoryName)) { @@ -267,8 +357,7 @@ namespace ts.server.typingsInstaller { }; } - protected abstract isPackageInstalled(packageName: string): boolean; + protected abstract execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void): void; protected abstract sendResponse(response: SetTypings | InvalidateCachedTypings): void; - protected abstract runInstall(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void): void; } } \ No newline at end of file From 7ca85e0e4ba3f9f40db8790828e0eccb48c1dac3 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Tue, 20 Sep 2016 14:14:51 -0700 Subject: [PATCH 3/3] move throttling to runAsync --- src/harness/unittests/compileOnSave.ts | 30 +- .../unittests/tsserverProjectSystem.ts | 43 ++- src/harness/unittests/typingsInstaller.ts | 353 ++++++++---------- .../typingsInstaller/nodeTypingsInstaller.ts | 26 +- .../typingsInstaller/typingsInstaller.ts | 120 +++--- 5 files changed, 271 insertions(+), 301 deletions(-) diff --git a/src/harness/unittests/compileOnSave.ts b/src/harness/unittests/compileOnSave.ts index fde83170f3645..24fd47ee0cbb5 100644 --- a/src/harness/unittests/compileOnSave.ts +++ b/src/harness/unittests/compileOnSave.ts @@ -3,6 +3,10 @@ /// namespace ts.projectSystem { + function createTestTypingsInstaller(host: server.ServerHost) { + return new TestTypingsInstaller("/a/data/", /*throttleLimit*/5, host); + } + describe("CompileOnSave affected list", () => { function sendAffectedFileRequestAndCheckResult(session: server.Session, request: server.protocol.Request, expectedFileList: { projectFileName: string, files: FileOrFolder[] }[]) { const response: server.protocol.CompileOnSaveAffectedFileListSingleProject[] = session.executeCommand(request).response; @@ -105,7 +109,7 @@ namespace ts.projectSystem { it("should contains only itself if a module file's shape didn't change, and all files referencing it if its shape changed", () => { const host = createServerHost([moduleFile1, file1Consumer1, file1Consumer2, globalFile3, moduleFile2, configFile, libFile]); - const typingsInstaller = new TestTypingsInstaller("/a/data/", host); + const typingsInstaller = createTestTypingsInstaller(host); const session = new server.Session(host, nullCancellationToken, /*useSingleInferredProject*/ false, typingsInstaller, Utils.byteLength, process.hrtime, nullLogger, /*canUseEvents*/ false); openFilesForSession([moduleFile1, file1Consumer1], session); @@ -130,7 +134,7 @@ namespace ts.projectSystem { it("should be up-to-date with the reference map changes", () => { const host = createServerHost([moduleFile1, file1Consumer1, file1Consumer2, globalFile3, moduleFile2, configFile, libFile]); - const typingsInstaller = new TestTypingsInstaller("/a/data/", host); + const typingsInstaller = createTestTypingsInstaller(host); const session = new server.Session(host, nullCancellationToken, /*useSingleInferredProject*/ false, typingsInstaller, Utils.byteLength, process.hrtime, nullLogger, /*canUseEvents*/ false); openFilesForSession([moduleFile1, file1Consumer1], session); @@ -177,7 +181,7 @@ namespace ts.projectSystem { it("should be up-to-date with changes made in non-open files", () => { const host = createServerHost([moduleFile1, file1Consumer1, file1Consumer2, globalFile3, moduleFile2, configFile, libFile]); - const typingsInstaller = new TestTypingsInstaller("/a/data/", host); + const typingsInstaller = createTestTypingsInstaller(host); const session = new server.Session(host, nullCancellationToken, /*useSingleInferredProject*/ false, typingsInstaller, Utils.byteLength, process.hrtime, nullLogger, /*canUseEvents*/ false); openFilesForSession([moduleFile1], session); @@ -195,7 +199,7 @@ namespace ts.projectSystem { it("should be up-to-date with deleted files", () => { const host = createServerHost([moduleFile1, file1Consumer1, file1Consumer2, globalFile3, moduleFile2, configFile, libFile]); - const typingsInstaller = new TestTypingsInstaller("/a/data/", host); + const typingsInstaller = createTestTypingsInstaller(host); const session = new server.Session(host, nullCancellationToken, /*useSingleInferredProject*/ false, typingsInstaller, Utils.byteLength, process.hrtime, nullLogger, /*canUseEvents*/ false); openFilesForSession([moduleFile1], session); @@ -210,7 +214,7 @@ namespace ts.projectSystem { it("should be up-to-date with newly created files", () => { const host = createServerHost([moduleFile1, file1Consumer1, file1Consumer2, globalFile3, moduleFile2, configFile, libFile]); - const typingsInstaller = new TestTypingsInstaller("/a/data/", host); + const typingsInstaller = createTestTypingsInstaller(host); const session = new server.Session(host, nullCancellationToken, /*useSingleInferredProject*/ false, typingsInstaller, Utils.byteLength, process.hrtime, nullLogger, /*canUseEvents*/ false); openFilesForSession([moduleFile1], session); @@ -247,7 +251,7 @@ namespace ts.projectSystem { }; const host = createServerHost([moduleFile1, file1Consumer1, configFile, libFile]); - const typingsInstaller = new TestTypingsInstaller("/a/data/", host); + const typingsInstaller = createTestTypingsInstaller(host); const session = new server.Session(host, nullCancellationToken, /*useSingleInferredProject*/ false, typingsInstaller, Utils.byteLength, process.hrtime, nullLogger, /*canUseEvents*/ false); openFilesForSession([moduleFile1, file1Consumer1], session); @@ -264,7 +268,7 @@ namespace ts.projectSystem { it("should return all files if a global file changed shape", () => { const host = createServerHost([moduleFile1, file1Consumer1, file1Consumer2, globalFile3, moduleFile2, configFile, libFile]); - const typingsInstaller = new TestTypingsInstaller("/a/data/", host); + const typingsInstaller = createTestTypingsInstaller(host); const session = new server.Session(host, nullCancellationToken, /*useSingleInferredProject*/ false, typingsInstaller, Utils.byteLength, process.hrtime, nullLogger, /*canUseEvents*/ false); openFilesForSession([globalFile3], session); @@ -290,7 +294,7 @@ namespace ts.projectSystem { }; const host = createServerHost([moduleFile1, file1Consumer1, file1Consumer2, configFile, libFile]); - const typingsInstaller = new TestTypingsInstaller("/a/data/", host); + const typingsInstaller = createTestTypingsInstaller(host); const session = new server.Session(host, nullCancellationToken, /*useSingleInferredProject*/ false, typingsInstaller, Utils.byteLength, process.hrtime, nullLogger, /*canUseEvents*/ false); openFilesForSession([moduleFile1], session); sendAffectedFileRequestAndCheckResult(session, moduleFile1FileListRequest, []); @@ -308,7 +312,7 @@ namespace ts.projectSystem { }; const host = createServerHost([moduleFile1, file1Consumer1, configFile, libFile]); - const typingsInstaller = new TestTypingsInstaller("/a/data/", host); + const typingsInstaller = createTestTypingsInstaller(host); const session = new server.Session(host, nullCancellationToken, /*useSingleInferredProject*/ false, typingsInstaller, Utils.byteLength, process.hrtime, nullLogger, /*canUseEvents*/ false); openFilesForSession([moduleFile1], session); @@ -337,7 +341,7 @@ namespace ts.projectSystem { }; const host = createServerHost([moduleFile1, file1Consumer1, configFile, libFile]); - const typingsInstaller = new TestTypingsInstaller("/a/data/", host); + const typingsInstaller = createTestTypingsInstaller(host); const session = new server.Session(host, nullCancellationToken, /*useSingleInferredProject*/ false, typingsInstaller, Utils.byteLength, process.hrtime, nullLogger, /*canUseEvents*/ false); openFilesForSession([moduleFile1], session); @@ -359,7 +363,7 @@ namespace ts.projectSystem { content: `import {y} from "./file1Consumer1";` }; const host = createServerHost([moduleFile1, file1Consumer1, file1Consumer1Consumer1, globalFile3, configFile, libFile]); - const typingsInstaller = new TestTypingsInstaller("/a/data/", host); + const typingsInstaller = createTestTypingsInstaller(host); const session = new server.Session(host, nullCancellationToken, /*useSingleInferredProject*/ false, typingsInstaller, Utils.byteLength, process.hrtime, nullLogger, /*canUseEvents*/ false); openFilesForSession([moduleFile1, file1Consumer1], session); @@ -392,7 +396,7 @@ namespace ts.projectSystem { export var t2 = 10;` }; const host = createServerHost([file1, file2, configFile]); - const typingsInstaller = new TestTypingsInstaller("/a/data/", host); + const typingsInstaller = createTestTypingsInstaller(host); const session = new server.Session(host, nullCancellationToken, /*useSingleInferredProject*/ false, typingsInstaller, Utils.byteLength, process.hrtime, nullLogger, /*canUseEvents*/ false); openFilesForSession([file1, file2], session); @@ -475,7 +479,7 @@ namespace ts.projectSystem { content: `{}` }; const host = createServerHost([file1, file2, configFile, libFile]); - const typingsInstaller = new TestTypingsInstaller("/a/data/", host); + const typingsInstaller = createTestTypingsInstaller(host); const session = new server.Session(host, nullCancellationToken, /*useSingleInferredProject*/ false, typingsInstaller, Utils.byteLength, process.hrtime, nullLogger, /*canUseEvents*/ false); openFilesForSession([file1, file2], session); diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 5275c4c89fc9f..363822ca1e873 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -2,6 +2,8 @@ /// namespace ts.projectSystem { + import TI = server.typingsInstaller; + const safeList = { path: "/safeList.json", content: JSON.stringify({ @@ -14,6 +16,7 @@ namespace ts.projectSystem { }; export interface PostExecAction { + readonly requestKind: TI.RequestKind; readonly error: Error; readonly stdout: string; readonly stderr: string; @@ -46,21 +49,27 @@ namespace ts.projectSystem { content: libFileContent }; - export class TestTypingsInstaller extends server.typingsInstaller.TypingsInstaller implements server.ITypingsInstaller { + export class TestTypingsInstaller extends TI.TypingsInstaller implements server.ITypingsInstaller { protected projectService: server.ProjectService; - constructor(readonly globalTypingsCacheLocation: string, readonly installTypingHost: server.ServerHost) { - super(globalTypingsCacheLocation, safeList.path); + constructor(readonly globalTypingsCacheLocation: string, throttleLimit: number, readonly installTypingHost: server.ServerHost) { + super(globalTypingsCacheLocation, "npm", safeList.path, throttleLimit); this.init(); } safeFileList = safeList.path; protected postExecActions: PostExecAction[] = []; - runPostExecActions() { - for (const action of this.postExecActions) { + executePendingCommands() { + const actionsToRun = this.postExecActions; + this.postExecActions = []; + for (const action of actionsToRun) { action.callback(action.error, action.stdout, action.stderr); } - this.postExecActions = []; + } + + checkPendingCommands(expected: TI.RequestKind[]) { + assert.equal(this.postExecActions.length, expected.length, `Expected ${expected.length} post install actions`); + this.postExecActions.forEach((act, i) => assert.equal(act.requestKind, expected[i], "Unexpected post install action")); } onProjectClosed(p: server.Project) { @@ -74,16 +83,15 @@ namespace ts.projectSystem { return this.installTypingHost; } - execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void): void { - switch (prefix) { - case "npm view": - case "npm install": - case "npm ls": + runCommand(requestKind: TI.RequestKind, requestId: number, command: string, cwd: string, cb: (err: Error, stdout: string, stderr: string) => void): void { + switch (requestKind) { + case TI.NpmViewRequest: + case TI.NpmInstallRequest: break; default: - throw new Error("TypingsInstaller: execAsync command not yet implemented"); + assert.isTrue(false, `request ${requestKind} is not supported`); } - this.addPostExecAction("success", cb); + this.addPostExecAction(requestKind, "success", cb); } sendResponse(response: server.SetTypings | server.InvalidateCachedTypings) { @@ -95,13 +103,14 @@ namespace ts.projectSystem { this.install(request); } - addPostExecAction(stdout: string | string[], cb: (err: Error, stdout: string, stderr: string) => void) { + addPostExecAction(requestKind: TI.RequestKind, stdout: string | string[], cb: TI.RequestCompletedAction) { const out = typeof stdout === "string" ? stdout : createNpmPackageJsonString(stdout); const action: PostExecAction = { error: undefined, stdout: out, stderr: "", - callback: cb + callback: cb, + requestKind }; this.postExecActions.push(action); } @@ -152,7 +161,7 @@ namespace ts.projectSystem { export function createSession(host: server.ServerHost, typingsInstaller?: server.ITypingsInstaller) { if (typingsInstaller === undefined) { - typingsInstaller = new TestTypingsInstaller("/a/data/", host); + typingsInstaller = new TestTypingsInstaller("/a/data/", /*throttleLimit*/5, host); } return new server.Session(host, nullCancellationToken, /*useSingleInferredProject*/ false, typingsInstaller, Utils.byteLength, process.hrtime, nullLogger, /*canUseEvents*/ false); } @@ -1643,7 +1652,7 @@ namespace ts.projectSystem { const host: TestServerHost & ModuleResolutionHost = createServerHost([file1, lib]); const resolutionTrace: string[] = []; host.trace = resolutionTrace.push.bind(resolutionTrace); - const projectService = createProjectService(host, { typingsInstaller: new TestTypingsInstaller("/a/cache", host) }); + const projectService = createProjectService(host, { typingsInstaller: new TestTypingsInstaller("/a/cache", /*throttleLimit*/5, host) }); projectService.setCompilerOptionsForInferredProjects({ traceResolution: true, allowJs: true }); projectService.openClientFile(file1.path); diff --git a/src/harness/unittests/typingsInstaller.ts b/src/harness/unittests/typingsInstaller.ts index 4ff70e0d3dc5b..9dd1247c8972e 100644 --- a/src/harness/unittests/typingsInstaller.ts +++ b/src/harness/unittests/typingsInstaller.ts @@ -3,23 +3,47 @@ /// namespace ts.projectSystem { + import TI = server.typingsInstaller; + + interface InstallerParams { + globalTypingsCacheLocation?: string; + throttleLimit?: number; + } + + class Installer extends TestTypingsInstaller { + constructor(host: server.ServerHost, p?: InstallerParams) { + super( + (p && p.globalTypingsCacheLocation) || "/a/data", + (p && p.throttleLimit) || 5, + host); + } + + installAll(expectedView: typeof TI.NpmViewRequest[], expectedInstall: typeof TI.NpmInstallRequest[]) { + this.checkPendingCommands(expectedView); + this.executePendingCommands(); + this.checkPendingCommands(expectedInstall); + this.executePendingCommands(); + } + } + describe("typingsInstaller", () => { - function execHelper(self: TestTypingsInstaller, host: TestServerHost, installedTypings: string[], typingFiles: FileOrFolder[], prefix: string, cb: (err: Error, stdout: string, stderr: string) => void): boolean { - let execSuper = true; - switch (prefix) { - case "npm install": - for (const file of typingFiles) { - host.createFileOrFolder(file, /*createParentDirectory*/ true); - } + function executeCommand(self: Installer, host: TestServerHost, installedTypings: string[], typingFiles: FileOrFolder[], requestKind: TI.RequestKind, cb: TI.RequestCompletedAction): void { + switch (requestKind) { + case TI.NpmInstallRequest: + self.addPostExecAction(requestKind, installedTypings, (err, stdout, stderr) => { + for (const file of typingFiles) { + host.createFileOrFolder(file, /*createParentDirectory*/ true); + } + cb(err, stdout, stderr); + }); break; - case "npm ls": - self.addPostExecAction(installedTypings, cb); - execSuper = false; + case TI.NpmViewRequest: + self.addPostExecAction(requestKind, installedTypings, cb); break; default: + assert.isTrue(false, `unexpected request kind ${requestKind}`); break; } - return execSuper; } it("configured projects (typings installed) 1", () => { const file1 = { @@ -52,16 +76,14 @@ namespace ts.projectSystem { content: "declare const $: { x: number }" }; const host = createServerHost([file1, tsconfig, packageJson]); - const installer = new (class extends TestTypingsInstaller { + const installer = new (class extends Installer { constructor() { - super("/a/data/", host); + super(host); } - execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void): void { + runCommand(requestKind: TI.RequestKind, requestId: number, command: string, cwd: string, cb: server.typingsInstaller.RequestCompletedAction) { const installedTypings = ["@types/jquery"]; const typingFiles = [jquery]; - if (execHelper(this, host, installedTypings, typingFiles, prefix, cb)) { - super.execAsync(prefix, command, cwd, requestId, cb); - } + executeCommand(this, host, installedTypings, typingFiles, requestKind, cb); } })(); @@ -72,9 +94,7 @@ namespace ts.projectSystem { const p = projectService.configuredProjects[0]; checkProjectActualFiles(p, [file1.path]); - assert(host.fileExists(combinePaths(installer.globalTypingsCacheLocation, "package.json"))); - - installer.runPostExecActions(); + installer.installAll([TI.NpmViewRequest], [TI.NpmInstallRequest]); checkNumberOfProjects(projectService, { configuredProjects: 1 }); checkProjectActualFiles(p, [file1.path, jquery.path]); @@ -100,16 +120,14 @@ namespace ts.projectSystem { content: "declare const $: { x: number }" }; const host = createServerHost([file1, packageJson]); - const installer = new (class extends TestTypingsInstaller { + const installer = new (class extends Installer { constructor() { - super("/a/data/", host); + super(host); } - execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void): void { + runCommand(requestKind: TI.RequestKind, requestId: number, command: string, cwd: string, cb: server.typingsInstaller.RequestCompletedAction) { const installedTypings = ["@types/jquery"]; const typingFiles = [jquery]; - if (execHelper(this, host, installedTypings, typingFiles, prefix, cb)) { - super.execAsync(prefix, command, cwd, requestId, cb); - } + executeCommand(this, host, installedTypings, typingFiles, requestKind, cb); } })(); @@ -120,9 +138,7 @@ namespace ts.projectSystem { const p = projectService.inferredProjects[0]; checkProjectActualFiles(p, [file1.path]); - assert(host.fileExists(combinePaths(installer.globalTypingsCacheLocation, "package.json"))); - - installer.runPostExecActions(); + installer.installAll([TI.NpmViewRequest], [TI.NpmInstallRequest]); checkNumberOfProjects(projectService, { inferredProjects: 1 }); checkProjectActualFiles(p, [file1.path, jquery.path]); @@ -134,9 +150,9 @@ namespace ts.projectSystem { content: "" }; const host = createServerHost([file1]); - const installer = new (class extends TestTypingsInstaller { + const installer = new (class extends Installer { constructor() { - super("", host); + super(host); } enqueueInstallTypingsRequest() { assert(false, "auto discovery should not be enabled"); @@ -150,6 +166,7 @@ namespace ts.projectSystem { options: {}, rootFiles: [toExternalFile(file1.path)] }); + installer.checkPendingCommands([]); // by default auto discovery will kick in if project contain only .js/.d.ts files // in this case project contain only ts files - no auto discovery projectService.checkNumberOfProjects({ externalProjects: 1 }); @@ -161,9 +178,9 @@ namespace ts.projectSystem { content: "" }; const host = createServerHost([file1]); - const installer = new (class extends TestTypingsInstaller { + const installer = new (class extends Installer { constructor() { - super("", host); + super(host); } enqueueInstallTypingsRequest() { assert(false, "auto discovery should not be enabled"); @@ -178,6 +195,7 @@ namespace ts.projectSystem { rootFiles: [toExternalFile(file1.path)], typingOptions: { include: ["jquery"] } }); + installer.checkPendingCommands([]); // by default auto discovery will kick in if project contain only .js/.d.ts files // in this case project contain only ts files - no auto discovery even if typing options is set projectService.checkNumberOfProjects({ externalProjects: 1 }); @@ -194,24 +212,18 @@ namespace ts.projectSystem { }; const host = createServerHost([file1]); let enqueueIsCalled = false; - let runInstallIsCalled = false; - const installer = new (class extends TestTypingsInstaller { + const installer = new (class extends Installer { constructor() { - super("", host); + super(host); } enqueueInstallTypingsRequest(project: server.Project, typingOptions: TypingOptions) { enqueueIsCalled = true; super.enqueueInstallTypingsRequest(project, typingOptions); } - execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void): void { + runCommand(requestKind: TI.RequestKind, requestId: number, command: string, cwd: string, cb: TI.RequestCompletedAction): void { const installedTypings = ["@types/jquery"]; const typingFiles = [jquery]; - if (execHelper(this, host, installedTypings, typingFiles, prefix, cb)) { - super.execAsync(prefix, command, cwd, requestId, cb); - } - if (prefix === "npm install") { - runInstallIsCalled = true; - } + executeCommand(this, host, installedTypings, typingFiles, requestKind, cb); } })(); @@ -224,12 +236,11 @@ namespace ts.projectSystem { typingOptions: { enableAutoDiscovery: true, include: ["node"] } }); - installer.runPostExecActions(); + assert.isTrue(enqueueIsCalled, "expected enqueueIsCalled to be true"); + installer.installAll([TI.NpmViewRequest], [TI.NpmInstallRequest]); // autoDiscovery is set in typing options - use it even if project contains only .ts files projectService.checkNumberOfProjects({ externalProjects: 1 }); - assert.isTrue(enqueueIsCalled, "expected 'enqueueIsCalled' to be true"); - assert.isTrue(runInstallIsCalled, "expected 'runInstallIsCalled' to be true"); }); it("external project - no typing options, with only js, jsx, d.ts files", () => { @@ -259,16 +270,14 @@ namespace ts.projectSystem { }; const host = createServerHost([file1, file2, file3]); - const installer = new (class extends TestTypingsInstaller { + const installer = new (class extends Installer { constructor() { - super("/a/data/", host); + super(host); } - execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void): void { + runCommand(requestKind: TI.RequestKind, requestId: number, command: string, cwd: string, cb: TI.RequestCompletedAction): void { const installedTypings = ["@types/lodash", "@types/react"]; const typingFiles = [lodash, react]; - if (execHelper(this, host, installedTypings, typingFiles, prefix, cb)) { - super.execAsync(prefix, command, cwd, requestId, cb); - } + executeCommand(this, host, installedTypings, typingFiles, requestKind, cb); } })(); @@ -285,7 +294,7 @@ namespace ts.projectSystem { projectService.checkNumberOfProjects({ externalProjects: 1 }); checkProjectActualFiles(p, [file1.path, file2.path, file3.path]); - installer.runPostExecActions(); + installer.installAll([TI.NpmViewRequest, TI.NpmViewRequest], [TI.NpmInstallRequest], ); checkNumberOfProjects(projectService, { externalProjects: 1 }); checkProjectActualFiles(p, [file1.path, file2.path, file3.path, lodash.path, react.path]); @@ -305,24 +314,18 @@ namespace ts.projectSystem { const host = createServerHost([file1, file2]); let enqueueIsCalled = false; - let runInstallIsCalled = false; - const installer = new (class extends TestTypingsInstaller { + const installer = new (class extends Installer { constructor() { - super("/a/data/", host); + super(host); } enqueueInstallTypingsRequest(project: server.Project, typingOptions: TypingOptions) { enqueueIsCalled = true; super.enqueueInstallTypingsRequest(project, typingOptions); } - execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void): void { + runCommand(requestKind: TI.RequestKind, requestId: number, command: string, cwd: string, cb: TI.RequestCompletedAction): void { const installedTypings: string[] = []; const typingFiles: FileOrFolder[] = []; - if (execHelper(this, host, installedTypings, typingFiles, prefix, cb)) { - super.execAsync(prefix, command, cwd, requestId, cb); - } - if (prefix === "npm install") { - runInstallIsCalled = true; - } + executeCommand(this, host, installedTypings, typingFiles, requestKind, cb); } })(); @@ -339,12 +342,10 @@ namespace ts.projectSystem { projectService.checkNumberOfProjects({ externalProjects: 1 }); checkProjectActualFiles(p, [file1.path, file2.path]); - installer.runPostExecActions(); + installer.checkPendingCommands([]); checkNumberOfProjects(projectService, { externalProjects: 1 }); checkProjectActualFiles(p, [file1.path, file2.path]); - assert.isFalse(enqueueIsCalled, "expected 'enqueueIsCalled' to be false"); - assert.isFalse(runInstallIsCalled, "expected 'runInstallIsCalled' to be false"); }); it("external project - with typing options, with only js, d.ts files", () => { @@ -392,16 +393,14 @@ namespace ts.projectSystem { }; const host = createServerHost([file1, file2, file3, packageJson]); - const installer = new (class extends TestTypingsInstaller { + const installer = new (class extends Installer { constructor() { - super("/a/data/", host); + super(host); } - execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void): void { + runCommand(requestKind: TI.RequestKind, requestId: number, command: string, cwd: string, cb: TI.RequestCompletedAction): void { const installedTypings = ["@types/commander", "@types/express", "@types/jquery", "@types/moment"]; const typingFiles = [commander, express, jquery, moment]; - if (execHelper(this, host, installedTypings, typingFiles, prefix, cb)) { - super.execAsync(prefix, command, cwd, requestId, cb); - } + executeCommand(this, host, installedTypings, typingFiles, requestKind, cb); } })(); @@ -418,18 +417,21 @@ namespace ts.projectSystem { projectService.checkNumberOfProjects({ externalProjects: 1 }); checkProjectActualFiles(p, [file1.path, file2.path, file3.path]); - installer.runPostExecActions(); + installer.installAll( + [TI.NpmViewRequest, TI.NpmViewRequest, TI.NpmViewRequest, TI.NpmViewRequest], + [TI.NpmInstallRequest] + ); checkNumberOfProjects(projectService, { externalProjects: 1 }); checkProjectActualFiles(p, [file1.path, file2.path, file3.path, commander.path, express.path, jquery.path, moment.path]); }); it("Throttle - delayed typings to install", () => { - const file1 = { + const lodashJs = { path: "/a/b/lodash.js", content: "" }; - const file2 = { + const commanderJs = { path: "/a/b/commander.js", content: "" }; @@ -442,11 +444,7 @@ namespace ts.projectSystem { content: JSON.stringify({ name: "test", dependencies: { - express: "^3.1.0", - cordova: "1.0.0", - grunt: "1.0.0", - gulp: "1.0.0", - forever: "1.0.0", + express: "^3.1.0" } }) }; @@ -471,42 +469,16 @@ namespace ts.projectSystem { path: "/a/data/node_modules/@types/lodash/index.d.ts", content: "declare const lodash: { x: number }" }; - const cordova = { - path: "/a/data/node_modules/@types/cordova/index.d.ts", - content: "declare const cordova: { x: number }" - }; - const grunt = { - path: "/a/data/node_modules/@types/grunt/index.d.ts", - content: "declare const grunt: { x: number }" - }; - const gulp = { - path: "/a/data/node_modules/@types/gulp/index.d.ts", - content: "declare const gulp: { x: number }" - }; - const forever = { - path: "/a/data/node_modules/@types/forever/index.d.ts", - content: "declare const forever: { x: number }" - }; - let npmViewCount = 0; - let npmInstallCount = 0; - const host = createServerHost([file1, file2, file3, packageJson]); - const installer = new (class extends TestTypingsInstaller { + const typingFiles = [commander, express, jquery, moment, lodash]; + const host = createServerHost([lodashJs, commanderJs, file3, packageJson]); + const installer = new (class extends Installer { constructor() { - super("/a/data/", host); + super(host, { throttleLimit: 3 }); } - execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void): void { - if (prefix === "npm view") { - npmViewCount++; - } - if (prefix === "npm install") { - npmInstallCount++; - } - const installedTypings = ["@types/commander", "@types/express", "@types/jquery", "@types/moment", "@types/lodash", "@types/cordova", "@types/grunt", "@types/gulp", "@types/forever"]; - const typingFiles = [commander, express, jquery, moment, lodash, cordova, grunt, gulp, forever]; - if (execHelper(this, host, installedTypings, typingFiles, prefix, cb)) { - super.execAsync(prefix, command, cwd, requestId, cb); - } + runCommand(requestKind: TI.RequestKind, requestId: number, command: string, cwd: string, cb: TI.RequestCompletedAction): void { + const installedTypings = ["@types/commander", "@types/express", "@types/jquery", "@types/moment", "@types/lodash"]; + executeCommand(this, host, installedTypings, typingFiles, requestKind, cb); } })(); @@ -515,33 +487,41 @@ namespace ts.projectSystem { projectService.openExternalProject({ projectFileName, options: { allowJS: true, moduleResolution: ModuleResolutionKind.NodeJs }, - rootFiles: [toExternalFile(file1.path), toExternalFile(file2.path), toExternalFile(file3.path)], + rootFiles: [toExternalFile(lodashJs.path), toExternalFile(commanderJs.path), toExternalFile(file3.path)], typingOptions: { include: ["jquery", "moment"] } }); const p = projectService.externalProjects[0]; projectService.checkNumberOfProjects({ externalProjects: 1 }); - checkProjectActualFiles(p, [file1.path, file2.path, file3.path]); - // The npm view count should be 5 even though there are 9 typings to acquire. - // The throttle limit has been reached so no more execAsync requests will be - // queued until these have been processed. - assert.isTrue(npmViewCount === 5); - assert.isTrue(npmInstallCount === 0); - - installer.runPostExecActions(); + checkProjectActualFiles(p, [lodashJs.path, commanderJs.path, file3.path]); + // expected 3 view requests in the queue + installer.checkPendingCommands([TI.NpmViewRequest, TI.NpmViewRequest, TI.NpmViewRequest]); + assert.equal(installer.pendingRunRequests.length, 2, "expected 2 pending requests"); + + // push view requests + installer.executePendingCommands(); + // expected 2 remaining view requests in the queue + installer.checkPendingCommands([TI.NpmViewRequest, TI.NpmViewRequest]); + // push view requests + installer.executePendingCommands(); + // expected one install request + installer.checkPendingCommands([TI.NpmInstallRequest]); + installer.executePendingCommands(); + // expected all typings file to exist + for (const f of typingFiles) { + assert.isTrue(host.fileExists(f.path), `expected file ${f.path} to exist`); + } - assert.isTrue(npmViewCount === 9); - assert.isTrue(npmInstallCount === 1); checkNumberOfProjects(projectService, { externalProjects: 1 }); - checkProjectActualFiles(p, [file1.path, file2.path, file3.path, commander.path, express.path, jquery.path, moment.path, lodash.path, cordova.path, grunt.path, gulp.path, forever.path]); + checkProjectActualFiles(p, [lodashJs.path, commanderJs.path, file3.path, commander.path, express.path, jquery.path, moment.path, lodash.path]); }); it("Throttle - delayed run install requests", () => { - const file1 = { + const lodashJs = { path: "/a/b/lodash.js", content: "" }; - const file2 = { + const commanderJs = { path: "/a/b/commander.js", content: "" }; @@ -552,119 +532,100 @@ namespace ts.projectSystem { const commander = { path: "/a/data/node_modules/@types/commander/index.d.ts", - content: "declare const commander: { x: number }" - }; - const express = { - path: "/a/data/node_modules/@types/express/index.d.ts", - content: "declare const express: { x: number }" + content: "declare const commander: { x: number }", + typings: "@types/commander" }; const jquery = { path: "/a/data/node_modules/@types/jquery/index.d.ts", - content: "declare const jquery: { x: number }" - }; - const moment = { - path: "/a/data/node_modules/@types/moment/index.d.ts", - content: "declare const moment: { x: number }" + content: "declare const jquery: { x: number }", + typings: "@types/jquery" }; const lodash = { path: "/a/data/node_modules/@types/lodash/index.d.ts", - content: "declare const lodash: { x: number }" + content: "declare const lodash: { x: number }", + typings: "@types/lodash" }; const cordova = { path: "/a/data/node_modules/@types/cordova/index.d.ts", - content: "declare const cordova: { x: number }" + content: "declare const cordova: { x: number }", + typings: "@types/cordova" }; const grunt = { path: "/a/data/node_modules/@types/grunt/index.d.ts", - content: "declare const grunt: { x: number }" + content: "declare const grunt: { x: number }", + typings: "@types/grunt" }; const gulp = { path: "/a/data/node_modules/@types/gulp/index.d.ts", - content: "declare const gulp: { x: number }" - }; - const forever = { - path: "/a/data/node_modules/@types/forever/index.d.ts", - content: "declare const forever: { x: number }" + content: "declare const gulp: { x: number }", + typings: "@types/gulp" }; - let npmViewCount = 0; - let npmInstallCount = 0; - let hasRunInstall2Typings = false; - const host = createServerHost([file1, file2, file3]); - const installer = new (class extends TestTypingsInstaller { + const host = createServerHost([lodashJs, commanderJs, file3]); + const installer = new (class extends Installer { constructor() { - super("/a/data/", host); + super(host, { throttleLimit: 3 }); } - execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void): void { - switch (prefix) { - case "npm view": - if (command.match("grunt|gulp|forever")) { - hasRunInstall2Typings = true; - } - npmViewCount++; - break; - case "npm install": - npmInstallCount++; - break; - } - - let installedTypings: string[]; - let typingFiles: FileOrFolder[]; - if (npmInstallCount <= 1) { - installedTypings = ["@types/commander", "@types/express", "@types/jquery", "@types/moment", "@types/lodash", "@types/cordova"]; - typingFiles = [commander, express, jquery, moment, lodash, cordova]; + runCommand(requestKind: TI.RequestKind, requestId: number, command: string, cwd: string, cb: TI.RequestCompletedAction): void { + if (requestKind === TI.NpmInstallRequest) { + let typingFiles: (FileOrFolder & { typings: string}) [] = []; + if (command.indexOf("commander") >= 0) { + typingFiles = [commander, jquery, lodash, cordova]; + } + else { + typingFiles = [grunt, gulp]; + } + executeCommand(this, host, typingFiles.map(f => f.typings), typingFiles, requestKind, cb); } else { - installedTypings = ["@types/grunt", "@types/gulp", "@types/forever"]; - typingFiles = [grunt, gulp, forever]; - } - - if (execHelper(this, host, installedTypings, typingFiles, prefix, cb)) { - super.execAsync(prefix, command, cwd, requestId, cb); + executeCommand(this, host, [], [], requestKind, cb); } } })(); - // Create project #1 with 6 typings + // Create project #1 with 4 typings const projectService = createProjectService(host, { typingsInstaller: installer }); const projectFileName1 = "/a/app/test1.csproj"; projectService.openExternalProject({ projectFileName: projectFileName1, options: { allowJS: true, moduleResolution: ModuleResolutionKind.NodeJs }, - rootFiles: [toExternalFile(file1.path), toExternalFile(file2.path), toExternalFile(file3.path)], - typingOptions: { include: ["jquery", "moment", "express", "cordova"] } + rootFiles: [toExternalFile(lodashJs.path), toExternalFile(commanderJs.path), toExternalFile(file3.path)], + typingOptions: { include: ["jquery", "cordova" ] } }); - // Create project #2 with 3 typings + installer.checkPendingCommands([TI.NpmViewRequest, TI.NpmViewRequest, TI.NpmViewRequest]); + assert.equal(installer.pendingRunRequests.length, 1, "expect one throttled request"); + + // Create project #2 with 2 typings const projectFileName2 = "/a/app/test2.csproj"; projectService.openExternalProject({ projectFileName: projectFileName2, options: { allowJS: true, moduleResolution: ModuleResolutionKind.NodeJs }, rootFiles: [toExternalFile(file3.path)], - typingOptions: { include: ["grunt", "gulp", "forever"] } + typingOptions: { include: ["grunt", "gulp"] } }); + assert.equal(installer.pendingRunRequests.length, 3, "expect three throttled request"); const p1 = projectService.externalProjects[0]; const p2 = projectService.externalProjects[1]; projectService.checkNumberOfProjects({ externalProjects: 2 }); - checkProjectActualFiles(p1, [file1.path, file2.path, file3.path]); + checkProjectActualFiles(p1, [lodashJs.path, commanderJs.path, file3.path]); checkProjectActualFiles(p2, [file3.path]); - // The npm view count should be 5 even though there are 9 typings to acquire. - // The throttle limit has been reached so no more run execAsync requests will be - // queued until these have been processed. Assert that typings from the second - // run install request have not been queued. - assert.isTrue(npmViewCount === 5); - assert.isTrue(npmInstallCount === 0); - assert.isFalse(hasRunInstall2Typings); - - installer.runPostExecActions(); - - assert.isTrue(npmViewCount === 9); - assert.isTrue(npmInstallCount === 2); - assert.isTrue(hasRunInstall2Typings); - checkProjectActualFiles(p1, [file1.path, file2.path, file3.path, commander.path, express.path, jquery.path, moment.path, lodash.path, cordova.path]); - checkProjectActualFiles(p2, [file3.path, grunt.path, gulp.path, forever.path]); + + installer.executePendingCommands(); + // expected one view request from the first project and two - from the second one + installer.checkPendingCommands([TI.NpmViewRequest, TI.NpmViewRequest, TI.NpmViewRequest]); + assert.equal(installer.pendingRunRequests.length, 0, "expected no throttled requests"); + + installer.executePendingCommands(); + + // should be two install requests from both projects + installer.checkPendingCommands([TI.NpmInstallRequest, TI.NpmInstallRequest]); + installer.executePendingCommands(); + + checkProjectActualFiles(p1, [lodashJs.path, commanderJs.path, file3.path, commander.path, jquery.path, lodash.path, cordova.path]); + checkProjectActualFiles(p2, [file3.path, grunt.path, gulp.path ]); }); }); } \ No newline at end of file diff --git a/src/server/typingsInstaller/nodeTypingsInstaller.ts b/src/server/typingsInstaller/nodeTypingsInstaller.ts index 3dc4be7e3dad4..7df1e9a78776f 100644 --- a/src/server/typingsInstaller/nodeTypingsInstaller.ts +++ b/src/server/typingsInstaller/nodeTypingsInstaller.ts @@ -6,6 +6,11 @@ namespace ts.server.typingsInstaller { appendFileSync(file: string, content: string): void } = require("fs"); + const path: { + join(...parts: string[]): string; + dirname(path: string): string; + } = require("path"); + class FileLog implements Log { constructor(private readonly logFile?: string) { } @@ -19,12 +24,17 @@ namespace ts.server.typingsInstaller { } export class NodeTypingsInstaller extends TypingsInstaller { - private exec: { (command: string, options: { cwd: string }, callback?: (error: Error, stdout: string, stderr: string) => void): any }; + private readonly exec: { (command: string, options: { cwd: string }, callback?: (error: Error, stdout: string, stderr: string) => void): any }; readonly installTypingHost: InstallTypingHost = sys; - constructor(globalTypingsCacheLocation: string, log: Log) { - super(globalTypingsCacheLocation, toPath("typingSafeList.json", __dirname, createGetCanonicalFileName(sys.useCaseSensitiveFileNames)), log); + constructor(globalTypingsCacheLocation: string, throttleLimit: number, log: Log) { + super( + globalTypingsCacheLocation, + /*npmPath*/ `"${path.join(path.dirname(process.argv[0]), "npm")}"`, + toPath("typingSafeList.json", __dirname, createGetCanonicalFileName(sys.useCaseSensitiveFileNames)), + throttleLimit, + log); if (this.log.isEnabled()) { this.log.writeLine(`Process id: ${process.pid}`); } @@ -55,16 +65,16 @@ namespace ts.server.typingsInstaller { } } - protected execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void) { + protected runCommand(requestKind: RequestKind, requestId: number, command: string, cwd: string, onRequestCompleted: RequestCompletedAction): void { if (this.log.isEnabled()) { this.log.writeLine(`#${requestId} running command '${command}'.`); } this.exec(command, { cwd }, (err, stdout, stderr) => { if (this.log.isEnabled()) { - this.log.writeLine(`${prefix} #${requestId} stdout: ${stdout}`); - this.log.writeLine(`${prefix} #${requestId} stderr: ${stderr}`); + this.log.writeLine(`${requestKind} #${requestId} stdout: ${stdout}`); + this.log.writeLine(`${requestKind} #${requestId} stderr: ${stderr}`); } - cb(err, stdout, stderr); + onRequestCompleted(err, stdout, stderr); }); } } @@ -90,6 +100,6 @@ namespace ts.server.typingsInstaller { } process.exit(0); }); - const installer = new NodeTypingsInstaller(globalTypingsCacheLocation, log); + const installer = new NodeTypingsInstaller(globalTypingsCacheLocation, /*throttleLimit*/5, log); installer.init(); } \ No newline at end of file diff --git a/src/server/typingsInstaller/typingsInstaller.ts b/src/server/typingsInstaller/typingsInstaller.ts index 24139436932fe..1113e6e2701dd 100644 --- a/src/server/typingsInstaller/typingsInstaller.ts +++ b/src/server/typingsInstaller/typingsInstaller.ts @@ -8,19 +8,11 @@ namespace ts.server.typingsInstaller { devDependencies: MapLike; } - interface RunInstallRequest { - readonly cachePath: string; - readonly typingsToInstall: string[]; - readonly postInstallAction: (installedTypings: string[]) => void; - } - export interface Log { isEnabled(): boolean; writeLine(text: string): void; } - const throttleLimit = 5; - const nullLog: Log = { isEnabled: () => false, writeLine: () => {} @@ -31,27 +23,44 @@ namespace ts.server.typingsInstaller { return result.resolvedModule && result.resolvedModule.resolvedFileName; } + export const NpmViewRequest: "npm view" = "npm view"; + export const NpmInstallRequest: "npm install" = "npm install"; + + export type RequestKind = typeof NpmViewRequest | typeof NpmInstallRequest; + + export type RequestCompletedAction = (err: Error, stdout: string, stderr: string) => void; + type PendingRequest = { + requestKind: RequestKind; + requestId: number; + command: string; + cwd: string; + onRequestCompleted: RequestCompletedAction + }; + export abstract class TypingsInstaller { + private readonly packageNameToTypingLocation: Map = createMap(); + private readonly missingTypingsSet: Map = createMap(); + private readonly knownCachesSet: Map = createMap(); + private readonly projectWatchers: Map = createMap(); + readonly pendingRunRequests: PendingRequest[] = []; + private installRunCount = 1; - private throttleCount = 0; - private packageNameToTypingLocation: Map = createMap(); - private missingTypingsSet: Map = createMap(); - private knownCachesSet: Map = createMap(); - private projectWatchers: Map = createMap(); - private delayedRunInstallRequests: RunInstallRequest[] = []; - private npmPath: string; + private inFlightRequestCount = 0; abstract readonly installTypingHost: InstallTypingHost; - constructor(readonly globalCachePath: string, readonly safeListPath: Path, protected readonly log = nullLog) { + constructor( + readonly globalCachePath: string, + readonly npmPath: string, + readonly safeListPath: Path, + readonly throttleLimit: number, + protected readonly log = nullLog) { if (this.log.isEnabled()) { this.log.writeLine(`Global cache location '${globalCachePath}', safe file path '${safeListPath}'`); } } init() { - const path = require("path"); - this.npmPath = `"${path.join(path.dirname(process.argv[0]), "npm")}"`; this.processCacheLocation(this.globalCachePath); } @@ -239,77 +248,37 @@ namespace ts.server.typingsInstaller { } private runInstall(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void): void { - if (this.throttleCount === throttleLimit) { - const request = { - cachePath: cachePath, - typingsToInstall: typingsToInstall, - postInstallAction: postInstallAction - }; - this.delayedRunInstallRequests.push(request); - return; - } - const id = this.installRunCount; + const requestId = this.installRunCount; + this.installRunCount++; let execInstallCmdCount = 0; const filteredTypings: string[] = []; - const delayedTypingsToInstall: string[] = []; for (const typing of typingsToInstall) { - if (this.throttleCount === throttleLimit) { - delayedTypingsToInstall.push(typing); - continue; - } execNpmViewTyping(this, typing); } function execNpmViewTyping(self: TypingsInstaller, typing: string) { - self.throttleCount++; const command = `${self.npmPath} view @types/${typing} --silent name`; - self.execAsync("npm view", command, cachePath, id, (err, stdout, stderr) => { + self.execAsync(NpmViewRequest, requestId, command, cachePath, (err, stdout, stderr) => { if (stdout) { filteredTypings.push(typing); } execInstallCmdCount++; - self.throttleCount--; - if (delayedTypingsToInstall.length > 0) { - return execNpmViewTyping(self, delayedTypingsToInstall.pop()); - } if (execInstallCmdCount === typingsToInstall.length) { installFilteredTypings(self, filteredTypings); - if (self.delayedRunInstallRequests.length > 0) { - const request = self.delayedRunInstallRequests.pop(); - return self.runInstall(request.cachePath, request.typingsToInstall, request.postInstallAction); - } } }); } function installFilteredTypings(self: TypingsInstaller, filteredTypings: string[]) { if (filteredTypings.length === 0) { - reportInstalledTypings(self); + postInstallAction([]); return; } - const command = `${self.npmPath} install ${filteredTypings.map(t => "@types/" + t).join(" ")} --save-dev`; - self.execAsync("npm install", command, cachePath, id, (err, stdout, stderr) => { - if (stdout) { - reportInstalledTypings(self); - } - }); - } - - function reportInstalledTypings(self: TypingsInstaller) { - const command = `${self.npmPath} ls -json`; - self.execAsync("npm ls", command, cachePath, id, (err, stdout, stderr) => { - let installedTypings: string[]; - try { - const response = JSON.parse(stdout); - if (response.dependencies) { - installedTypings = getOwnKeys(response.dependencies); - } - } - catch (e) { - self.log.writeLine(`Error parsing installed @types dependencies. Error details: ${e.message}`); - } - postInstallAction(installedTypings || []); + const scopedTypings = filteredTypings.map(t => "@types/" + t); + const command = `${self.npmPath} install ${scopedTypings.join(" ")} --save-dev`; + self.execAsync(NpmInstallRequest, requestId, command, cachePath, (err, stdout, stderr) => { + postInstallAction(stdout ? scopedTypings : []); }); } } @@ -357,7 +326,24 @@ namespace ts.server.typingsInstaller { }; } - protected abstract execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void): void; + private execAsync(requestKind: RequestKind, requestId: number, command: string, cwd: string, onRequestCompleted: RequestCompletedAction): void { + this.pendingRunRequests.unshift({ requestKind, requestId, command, cwd, onRequestCompleted }); + this.executeWithThrottling(); + } + + private executeWithThrottling() { + while (this.inFlightRequestCount < this.throttleLimit && this.pendingRunRequests.length) { + this.inFlightRequestCount++; + const request = this.pendingRunRequests.pop(); + this.runCommand(request.requestKind, request.requestId, request.command, request.cwd, (err, stdout, stderr) => { + this.inFlightRequestCount--; + request.onRequestCompleted(err, stdout, stderr); + this.executeWithThrottling(); + }); + } + } + + protected abstract runCommand(requestKind: RequestKind, requestId: number, command: string, cwd: string, onRequestCompleted: RequestCompletedAction): void; protected abstract sendResponse(response: SetTypings | InvalidateCachedTypings): void; } } \ No newline at end of file