From 5e37a310d709637617a40df024aadf681336718c Mon Sep 17 00:00:00 2001 From: Jason Ramsay Date: Tue, 23 Aug 2016 09:50:44 -0700 Subject: [PATCH 01/14] types 2.0 WIP --- src/server/editorServices.ts | 2 +- src/server/types.d.ts | 1 + src/server/typingsCache.ts | 11 +++++++++-- src/server/typingsInstaller/typingsInstaller.ts | 1 + 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 394613bebe..4add13dd18 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -213,7 +213,7 @@ namespace ts.server { } switch (response.kind) { case "set": - this.typingsCache.updateTypingsForProject(response.projectName, response.compilerOptions, response.typingOptions, response.typings); + this.typingsCache.updateTypingsForProject(response.projectName, response.compilerOptions, response.typingOptions, response.typings, response.files); project.updateGraph(); break; case "invalidate": diff --git a/src/server/types.d.ts b/src/server/types.d.ts index b4694cc401..3a9733cd0e 100644 --- a/src/server/types.d.ts +++ b/src/server/types.d.ts @@ -45,6 +45,7 @@ declare namespace ts.server { readonly typingOptions: ts.TypingOptions; readonly compilerOptions: ts.CompilerOptions; readonly typings: string[]; + readonly files: string[]; readonly kind: "set"; } diff --git a/src/server/typingsCache.ts b/src/server/typingsCache.ts index eb1d3283e0..81dd483e65 100644 --- a/src/server/typingsCache.ts +++ b/src/server/typingsCache.ts @@ -17,6 +17,7 @@ namespace ts.server { readonly typingOptions: TypingOptions; readonly compilerOptions: CompilerOptions; readonly typings: TypingsArray; + readonly files: string[]; poisoned: boolean; } @@ -73,6 +74,10 @@ namespace ts.server { return opt1.allowJs != opt2.allowJs; } + function filesChanged(before: string[], after: string[]): boolean { + return !setIsEqualTo(before, after); + } + export interface TypingsArray extends ReadonlyArray { " __typingsArrayBrand": any; } @@ -97,7 +102,7 @@ namespace ts.server { const entry = this.perProjectCache[project.getProjectName()]; const result: TypingsArray = entry ? entry.typings : emptyArray; - if (!entry || typingOptionsChanged(typingOptions, entry.typingOptions) || compilerOptionsChanged(project.getCompilerOptions(), entry.compilerOptions)) { + if (!entry || typingOptionsChanged(typingOptions, entry.typingOptions) || compilerOptionsChanged(project.getCompilerOptions(), entry.compilerOptions) || filesChanged(project.getFileNames(), entry.files)) { // something has been changed, issue a request to update typings this.installer.enqueueInstallTypingsRequest(project, typingOptions); // Note: entry is now poisoned since it does not really contain typings for a given combination of compiler options\typings options. @@ -106,6 +111,7 @@ namespace ts.server { compilerOptions: project.getCompilerOptions(), typingOptions, typings: result, + files: project.getFileNames(), poisoned: true }; } @@ -120,11 +126,12 @@ namespace ts.server { this.installer.enqueueInstallTypingsRequest(project, typingOptions); } - updateTypingsForProject(projectName: string, compilerOptions: CompilerOptions, typingOptions: TypingOptions, newTypings: string[]) { + updateTypingsForProject(projectName: string, compilerOptions: CompilerOptions, typingOptions: TypingOptions, newTypings: string[], files: string[]) { this.perProjectCache[projectName] = { compilerOptions, typingOptions, typings: toTypingsArray(newTypings), + files: files, poisoned: false }; } diff --git a/src/server/typingsInstaller/typingsInstaller.ts b/src/server/typingsInstaller/typingsInstaller.ts index f4a899068f..701763b956 100644 --- a/src/server/typingsInstaller/typingsInstaller.ts +++ b/src/server/typingsInstaller/typingsInstaller.ts @@ -287,6 +287,7 @@ namespace ts.server.typingsInstaller { typingOptions: request.typingOptions, compilerOptions: request.compilerOptions, typings, + files: request.fileNames, kind: "set" }; } From 1f9f20f04351bd048daa00f1908181d061771bdc Mon Sep 17 00:00:00 2001 From: Jason Ramsay Date: Thu, 25 Aug 2016 16:25:34 -0700 Subject: [PATCH 02/14] Swap out TSD for Types 2.0 --- src/server/editorServices.ts | 2 +- src/server/project.ts | 3 + src/server/types.d.ts | 1 - src/server/typingsCache.ts | 17 ++-- .../typingsInstaller/nodeTypingsInstaller.ts | 44 +++------ .../typingsInstaller/typingsInstaller.ts | 90 +++++++------------ 6 files changed, 52 insertions(+), 105 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 4add13dd18..394613bebe 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -213,7 +213,7 @@ namespace ts.server { } switch (response.kind) { case "set": - this.typingsCache.updateTypingsForProject(response.projectName, response.compilerOptions, response.typingOptions, response.typings, response.files); + this.typingsCache.updateTypingsForProject(response.projectName, response.compilerOptions, response.typingOptions, response.typings); project.updateGraph(); break; case "invalidate": diff --git a/src/server/project.ts b/src/server/project.ts index e8293decdd..6d48c5d87b 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -355,6 +355,9 @@ namespace ts.server { removed.push(id); } } + if (added.length > 0 || removed.length > 0) { + this.projectService.typingsCache.invalidateCachedTypingsForProject(this); + } this.lastReportedFileNames = currentFiles; this.lastReportedFileNames = currentFiles; diff --git a/src/server/types.d.ts b/src/server/types.d.ts index 3a9733cd0e..b4694cc401 100644 --- a/src/server/types.d.ts +++ b/src/server/types.d.ts @@ -45,7 +45,6 @@ declare namespace ts.server { readonly typingOptions: ts.TypingOptions; readonly compilerOptions: ts.CompilerOptions; readonly typings: string[]; - readonly files: string[]; readonly kind: "set"; } diff --git a/src/server/typingsCache.ts b/src/server/typingsCache.ts index 81dd483e65..1ef93fbbb7 100644 --- a/src/server/typingsCache.ts +++ b/src/server/typingsCache.ts @@ -17,12 +17,11 @@ namespace ts.server { readonly typingOptions: TypingOptions; readonly compilerOptions: CompilerOptions; readonly typings: TypingsArray; - readonly files: string[]; poisoned: boolean; } const emptyArray: any[] = []; - const jsOrDts = [".js", ".d.ts"]; + const jsOrDts = [".js", ".jsx", ".d.ts"]; function getTypingOptionsForProjects(proj: Project): TypingOptions { if (proj.projectKind === ProjectKind.Configured) { @@ -74,10 +73,6 @@ namespace ts.server { return opt1.allowJs != opt2.allowJs; } - function filesChanged(before: string[], after: string[]): boolean { - return !setIsEqualTo(before, after); - } - export interface TypingsArray extends ReadonlyArray { " __typingsArrayBrand": any; } @@ -102,18 +97,17 @@ namespace ts.server { const entry = this.perProjectCache[project.getProjectName()]; const result: TypingsArray = entry ? entry.typings : emptyArray; - if (!entry || typingOptionsChanged(typingOptions, entry.typingOptions) || compilerOptionsChanged(project.getCompilerOptions(), entry.compilerOptions) || filesChanged(project.getFileNames(), entry.files)) { - // something has been changed, issue a request to update typings - this.installer.enqueueInstallTypingsRequest(project, typingOptions); + if (!entry || typingOptionsChanged(typingOptions, entry.typingOptions) || compilerOptionsChanged(project.getCompilerOptions(), entry.compilerOptions)) { // Note: entry is now poisoned since it does not really contain typings for a given combination of compiler options\typings options. // instead it acts as a placeholder to prevent issuing multiple requests this.perProjectCache[project.getProjectName()] = { compilerOptions: project.getCompilerOptions(), typingOptions, typings: result, - files: project.getFileNames(), poisoned: true }; + // something has been changed, issue a request to update typings + this.installer.enqueueInstallTypingsRequest(project, typingOptions); } return result; } @@ -126,12 +120,11 @@ namespace ts.server { this.installer.enqueueInstallTypingsRequest(project, typingOptions); } - updateTypingsForProject(projectName: string, compilerOptions: CompilerOptions, typingOptions: TypingOptions, newTypings: string[], files: string[]) { + updateTypingsForProject(projectName: string, compilerOptions: CompilerOptions, typingOptions: TypingOptions, newTypings: string[]) { this.perProjectCache[projectName] = { compilerOptions, typingOptions, typings: toTypingsArray(newTypings), - files: files, poisoned: false }; } diff --git a/src/server/typingsInstaller/nodeTypingsInstaller.ts b/src/server/typingsInstaller/nodeTypingsInstaller.ts index e0a361f941..13c70da9f8 100644 --- a/src/server/typingsInstaller/nodeTypingsInstaller.ts +++ b/src/server/typingsInstaller/nodeTypingsInstaller.ts @@ -46,7 +46,7 @@ namespace ts.server.typingsInstaller { private exec: { (command: string, options: { cwd: string }, callback?: (error: Error, stdout: string, stderr: string) => void): any }; private npmBinPath: string; - private tsdRunCount = 1; + private installRunCount = 1; readonly installTypingHost: InstallTypingHost = sys; constructor(log?: Log) { @@ -101,23 +101,6 @@ namespace ts.server.typingsInstaller { } } - protected installPackage(packageName: string) { - try { - const output = this.execSync(`npm install --silent --global ${packageName}`, { stdio: "pipe" }).toString(); - if (this.log.isEnabled()) { - this.log.writeLine(`installPackage::stdout '${output}'`); - } - return true; - } - catch (e) { - if (this.log.isEnabled()) { - this.log.writeLine(`installPackage::err::stdout '${e.stdout && e.stdout.toString()}'`); - this.log.writeLine(`installPackage::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)}`); @@ -128,30 +111,23 @@ namespace ts.server.typingsInstaller { } } - protected runTsd(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void): void { - const id = this.tsdRunCount; - this.tsdRunCount++; - const tsdPath = combinePaths(this.npmBinPath, "tsd"); - const command = `${tsdPath} install ${typingsToInstall.join(" ")} -ros`; + protected runInstall(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void): void { + const id = this.installRunCount; + this.installRunCount++; + const command = `npm install @types/${typingsToInstall.join(" @types/")} --save-dev`; if (this.log.isEnabled()) { - this.log.writeLine(`Running tsd ${id}, command '${command}'. cache path '${cachePath}'`); + this.log.writeLine(`Running npm install @types ${id}, command '${command}'. cache path '${cachePath}'`); } this.exec(command, { cwd: cachePath }, (err, stdout, stderr) => { if (this.log.isEnabled()) { - this.log.writeLine(`TSD ${id} stdout: ${stdout}`); - this.log.writeLine(`TSD ${id} stderr: ${stderr}`); - } - const i = stdout.indexOf("running install"); - if (i < 0) { - return; + this.log.writeLine(`npm install @types ${id} stdout: ${stdout}`); + this.log.writeLine(`npm install @types ${id} stderr: ${stderr}`); } const installedTypings: string[] = []; - - const expr = /^\s*-\s*(\S+)\s*$/gm; - expr.lastIndex = i; + const expr = /^.*(node_modules)\\(@types)\\(\S+)\s*$/gm; let match: RegExpExecArray; while (match = expr.exec(stdout)) { - installedTypings.push(match[1]); + installedTypings.push(`${match[1]}/${match[2]}/${match[3]}`); } postInstallAction(installedTypings); }); diff --git a/src/server/typingsInstaller/typingsInstaller.ts b/src/server/typingsInstaller/typingsInstaller.ts index 701763b956..ac72782eb5 100644 --- a/src/server/typingsInstaller/typingsInstaller.ts +++ b/src/server/typingsInstaller/typingsInstaller.ts @@ -1,16 +1,10 @@ +/// /// /// namespace ts.server.typingsInstaller { - const DefaultTsdSettings = JSON.stringify({ - version: "v4", - repo: "DefinitelyTyped/DefinitelyTyped", - ref: "master", - path: "typings" - }, /*replacer*/undefined, /*space*/4); - - interface TsdConfig { - installed: MapLike; + interface NpmConfig { + devDependencies: MapLike; } export interface Log { @@ -23,18 +17,17 @@ namespace ts.server.typingsInstaller { writeLine: () => {} }; - function tsdTypingToFileName(cachePath: string, tsdTypingFile: string) { - return combinePaths(cachePath, `typings/${tsdTypingFile}`); + function typingToFileName(cachePath: string, packageName: string, installTypingHost: InstallTypingHost): string { + const result = resolveModuleName(packageName, combinePaths(cachePath, "index.d.ts"), { moduleResolution: ModuleResolutionKind.NodeJs }, installTypingHost); + return result.resolvedModule ? result.resolvedModule.resolvedFileName : null; } - function getPackageName(tsdTypingFile: string) { - const idx = tsdTypingFile.indexOf("/"); - return idx > 0 ? tsdTypingFile.substr(0, idx) : undefined; + function getPackageName(typingFile: string) { + const idx = typingFile.lastIndexOf("/"); + return idx > 0 ? typingFile.substr(idx + 1) : undefined; } export abstract class TypingsInstaller { - private isTsdInstalled: boolean; - private packageNameToTypingLocation: Map = createMap(); private missingTypingsSet: Map = createMap(); private knownCachesSet: Map = createMap(); @@ -50,20 +43,6 @@ namespace ts.server.typingsInstaller { } init() { - this.isTsdInstalled = this.isPackageInstalled("tsd"); - if (this.log.isEnabled()) { - this.log.writeLine(`isTsdInstalled: ${this.isTsdInstalled}`); - } - - if (!this.isTsdInstalled) { - if (this.log.isEnabled()) { - this.log.writeLine(`tsd is not installed, installing tsd...`); - } - this.isTsdInstalled = this.installPackage("tsd"); - if (this.log.isEnabled()) { - this.log.writeLine(`isTsdInstalled: ${this.isTsdInstalled}`); - } - } this.processCacheLocation(this.globalCachePath); } @@ -94,13 +73,6 @@ namespace ts.server.typingsInstaller { } install(req: DiscoverTypings) { - if (!this.isTsdInstalled) { - if (this.log.isEnabled()) { - this.log.writeLine(`tsd is not installed, ignoring request...`); - } - return; - } - if (this.log.isEnabled()) { this.log.writeLine(`Got install request ${JSON.stringify(req)}`); } @@ -153,23 +125,26 @@ namespace ts.server.typingsInstaller { } return; } - const tsdJson = combinePaths(cacheLocation, "tsd.json"); + const packageJson = combinePaths(cacheLocation, "package.json"); if (this.log.isEnabled()) { - this.log.writeLine(`Trying to find '${tsdJson}'...`); + this.log.writeLine(`Trying to find '${packageJson}'...`); } - if (this.installTypingHost.fileExists(tsdJson)) { - const tsdConfig = JSON.parse(this.installTypingHost.readFile(tsdJson)); + if (this.installTypingHost.fileExists(packageJson)) { + const npmConfig = JSON.parse(this.installTypingHost.readFile(packageJson)); if (this.log.isEnabled()) { - this.log.writeLine(`Loaded content of '${tsdJson}': ${JSON.stringify(tsdConfig)}`); + this.log.writeLine(`Loaded content of '${npmConfig}': ${JSON.stringify(npmConfig)}`); } - if (tsdConfig.installed) { - for (const key in tsdConfig.installed) { - // key is / + if (npmConfig.devDependencies) { + for (const key in npmConfig.devDependencies) { + // key is @types/ const packageName = getPackageName(key); if (!packageName) { continue; } - const typingFile = tsdTypingToFileName(cacheLocation, key); + var typingFile = typingToFileName(cacheLocation, packageName, this.installTypingHost); + if (!typingFile) { + continue; + } const existingTypingFile = this.packageNameToTypingLocation[packageName]; if (existingTypingFile === typingFile) { continue; @@ -204,20 +179,19 @@ namespace ts.server.typingsInstaller { return; } - // TODO: install typings and send response when they are ready - const tsdPath = combinePaths(cachePath, "tsd.json"); + const npmConfigPath = combinePaths(cachePath, "package.json"); if (this.log.isEnabled()) { - this.log.writeLine(`Tsd config file: ${tsdPath}`); + this.log.writeLine(`Npm config file: ${npmConfigPath}`); } - if (!this.installTypingHost.fileExists(tsdPath)) { + if (!this.installTypingHost.fileExists(npmConfigPath)) { if (this.log.isEnabled()) { - this.log.writeLine(`Tsd config file '${tsdPath}' is missing, creating new one...`); + this.log.writeLine(`Npm config file: '${npmConfigPath}' is missing, creating new one...`); } this.ensureDirectoryExists(cachePath, this.installTypingHost); - this.installTypingHost.writeFile(tsdPath, DefaultTsdSettings); + this.installTypingHost.writeFile(npmConfigPath, "{}"); } - this.runTsd(cachePath, typingsToInstall, installedTypings => { + this.runInstall(cachePath, typingsToInstall, installedTypings => { // TODO: watch project directory if (this.log.isEnabled()) { this.log.writeLine(`Requested to install typings ${JSON.stringify(typingsToInstall)}, installed typings ${JSON.stringify(installedTypings)}`); @@ -230,7 +204,11 @@ namespace ts.server.typingsInstaller { continue; } installedPackages[packageName] = true; - installedTypingFiles.push(tsdTypingToFileName(cachePath, t)); + var typingFile = typingToFileName(cachePath, packageName, this.installTypingHost); + if (!typingFile) { + continue; + } + installedTypingFiles.push(typingFile); } if (this.log.isEnabled()) { this.log.writeLine(`Installed typing files ${JSON.stringify(installedTypingFiles)}`); @@ -287,14 +265,12 @@ namespace ts.server.typingsInstaller { typingOptions: request.typingOptions, compilerOptions: request.compilerOptions, typings, - files: request.fileNames, kind: "set" }; } protected abstract isPackageInstalled(packageName: string): boolean; - protected abstract installPackage(packageName: string): boolean; protected abstract sendResponse(response: SetTypings | InvalidateCachedTypings): void; - protected abstract runTsd(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void): void; + protected abstract runInstall(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void): void; } } \ No newline at end of file From 7d3f22fc66494f6e85dceb00b6f882daba70691a Mon Sep 17 00:00:00 2001 From: Jason Ramsay Date: Fri, 26 Aug 2016 16:37:31 -0700 Subject: [PATCH 03/14] Changes from CR feedback --- src/server/editorServices.ts | 1 + src/server/project.ts | 3 --- src/server/typingsInstaller/nodeTypingsInstaller.ts | 2 +- src/server/typingsInstaller/typingsInstaller.ts | 11 +++-------- 4 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 394613bebe..ad7ad90cb6 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -294,6 +294,7 @@ namespace ts.server { let shouldRefreshInferredProjects = false; for (const p of projects) { if (!p.updateGraph()) { + this.typingsCache.invalidateCachedTypingsForProject(p); shouldRefreshInferredProjects = true; } } diff --git a/src/server/project.ts b/src/server/project.ts index 6d48c5d87b..e8293decdd 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -355,9 +355,6 @@ namespace ts.server { removed.push(id); } } - if (added.length > 0 || removed.length > 0) { - this.projectService.typingsCache.invalidateCachedTypingsForProject(this); - } this.lastReportedFileNames = currentFiles; this.lastReportedFileNames = currentFiles; diff --git a/src/server/typingsInstaller/nodeTypingsInstaller.ts b/src/server/typingsInstaller/nodeTypingsInstaller.ts index 13c70da9f8..1de0a2dc9c 100644 --- a/src/server/typingsInstaller/nodeTypingsInstaller.ts +++ b/src/server/typingsInstaller/nodeTypingsInstaller.ts @@ -114,7 +114,7 @@ namespace ts.server.typingsInstaller { protected runInstall(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void): void { const id = this.installRunCount; this.installRunCount++; - const command = `npm install @types/${typingsToInstall.join(" @types/")} --save-dev`; + const command = `npm install ${typingsToInstall.map(t => "@types/" + t)} --save-dev`; if (this.log.isEnabled()) { this.log.writeLine(`Running npm install @types ${id}, command '${command}'. cache path '${cachePath}'`); } diff --git a/src/server/typingsInstaller/typingsInstaller.ts b/src/server/typingsInstaller/typingsInstaller.ts index ac72782eb5..619b307316 100644 --- a/src/server/typingsInstaller/typingsInstaller.ts +++ b/src/server/typingsInstaller/typingsInstaller.ts @@ -19,12 +19,7 @@ namespace ts.server.typingsInstaller { function typingToFileName(cachePath: string, packageName: string, installTypingHost: InstallTypingHost): string { const result = resolveModuleName(packageName, combinePaths(cachePath, "index.d.ts"), { moduleResolution: ModuleResolutionKind.NodeJs }, installTypingHost); - return result.resolvedModule ? result.resolvedModule.resolvedFileName : null; - } - - function getPackageName(typingFile: string) { - const idx = typingFile.lastIndexOf("/"); - return idx > 0 ? typingFile.substr(idx + 1) : undefined; + return result.resolvedModule && result.resolvedModule.resolvedFileName; } export abstract class TypingsInstaller { @@ -137,7 +132,7 @@ namespace ts.server.typingsInstaller { if (npmConfig.devDependencies) { for (const key in npmConfig.devDependencies) { // key is @types/ - const packageName = getPackageName(key); + const packageName = getBaseFileName(key); if (!packageName) { continue; } @@ -199,7 +194,7 @@ namespace ts.server.typingsInstaller { const installedPackages: Map = createMap(); const installedTypingFiles: string[] = []; for (const t of installedTypings) { - const packageName = getPackageName(t); + const packageName = getBaseFileName(t); if (!packageName) { continue; } From a2f92aa583b505dd800149533f871bd0960daf1d Mon Sep 17 00:00:00 2001 From: Jason Ramsay Date: Mon, 29 Aug 2016 15:44:33 -0700 Subject: [PATCH 04/14] invalidate cached typings on add or remove file --- src/server/project.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/server/project.ts b/src/server/project.ts index 4adbc7ca05..267175f569 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -414,6 +414,9 @@ namespace ts.server { removed.push(id); } } + if (added.length > 0 || removed.length > 0) { + this.projectService.typingsCache.invalidateCachedTypingsForProject(this); + } this.lastReportedFileNames = currentFiles; this.lastReportedFileNames = currentFiles; From 456227bbeeab35720f636dc6681a7ff721ba50f8 Mon Sep 17 00:00:00 2001 From: Jason Ramsay Date: Mon, 29 Aug 2016 16:43:34 -0700 Subject: [PATCH 05/14] Move invalidate typings cache to UpdateGraphWorker --- src/server/editorServices.ts | 1 - src/server/project.ts | 4 +--- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 5d1865c96a..dbaeb4b45c 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -301,7 +301,6 @@ namespace ts.server { let shouldRefreshInferredProjects = false; for (const p of projects) { if (!p.updateGraph()) { - this.typingsCache.invalidateCachedTypingsForProject(p); shouldRefreshInferredProjects = true; } } diff --git a/src/server/project.ts b/src/server/project.ts index a0792082f1..31546e2933 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -312,6 +312,7 @@ namespace ts.server { // - newProgram is different from the old program and structure of the old program was not reused. if (!oldProgram || (this.program !== oldProgram && !oldProgram.structureIsReused)) { hasChanges = true; + this.projectService.typingsCache.invalidateCachedTypingsForProject(this); if (oldProgram) { for (const f of oldProgram.getSourceFiles()) { if (this.program.getSourceFileByPath(f.path)) { @@ -414,9 +415,6 @@ namespace ts.server { removed.push(id); } } - if (added.length > 0 || removed.length > 0) { - this.projectService.typingsCache.invalidateCachedTypingsForProject(this); - } this.lastReportedFileNames = currentFiles; this.lastReportedFileNames = currentFiles; From 199e533059bca61c1c30004ffea64dc6f9cbea53 Mon Sep 17 00:00:00 2001 From: Jason Ramsay Date: Mon, 29 Aug 2016 19:01:55 -0700 Subject: [PATCH 06/14] - Command line should use spaces between types instead of comma - if entry.typings is empty use entry.typings for default typings to install --- src/server/typingsCache.ts | 2 +- src/server/typingsInstaller/nodeTypingsInstaller.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/server/typingsCache.ts b/src/server/typingsCache.ts index a58855118f..9be3583e9c 100644 --- a/src/server/typingsCache.ts +++ b/src/server/typingsCache.ts @@ -82,7 +82,7 @@ namespace ts.server { } const entry = this.perProjectCache[project.getProjectName()]; - const result: TypingsArray = entry ? entry.typings : emptyArray; + const result: TypingsArray = entry && entry.typings.length > 0 ? entry.typings : toTypingsArray(typingOptions.include); if (!entry || typingOptionsChanged(typingOptions, entry.typingOptions) || compilerOptionsChanged(project.getCompilerOptions(), entry.compilerOptions)) { // Note: entry is now poisoned since it does not really contain typings for a given combination of compiler options\typings options. // instead it acts as a placeholder to prevent issuing multiple requests diff --git a/src/server/typingsInstaller/nodeTypingsInstaller.ts b/src/server/typingsInstaller/nodeTypingsInstaller.ts index 1de0a2dc9c..50a3626700 100644 --- a/src/server/typingsInstaller/nodeTypingsInstaller.ts +++ b/src/server/typingsInstaller/nodeTypingsInstaller.ts @@ -114,7 +114,7 @@ namespace ts.server.typingsInstaller { protected runInstall(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void): void { const id = this.installRunCount; this.installRunCount++; - const command = `npm install ${typingsToInstall.map(t => "@types/" + t)} --save-dev`; + const command = `npm install ${typingsToInstall.map(t => "@types/" + t).join(" ")} --save-dev`; if (this.log.isEnabled()) { this.log.writeLine(`Running npm install @types ${id}, command '${command}'. cache path '${cachePath}'`); } From 24ef426fbb0f5c84c8d2fd76e63c6dabc3bf2fa0 Mon Sep 17 00:00:00 2001 From: Jason Ramsay Date: Tue, 30 Aug 2016 13:41:24 -0700 Subject: [PATCH 07/14] Install packages separately --- src/server/project.ts | 11 +++++- .../typingsInstaller/nodeTypingsInstaller.ts | 36 +++++++++++-------- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/server/project.ts b/src/server/project.ts index 31546e2933..90d1e9c64e 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -312,7 +312,7 @@ namespace ts.server { // - newProgram is different from the old program and structure of the old program was not reused. if (!oldProgram || (this.program !== oldProgram && !oldProgram.structureIsReused)) { hasChanges = true; - this.projectService.typingsCache.invalidateCachedTypingsForProject(this); + //this.projectService.typingsCache.invalidateCachedTypingsForProject(this); if (oldProgram) { for (const f of oldProgram.getSourceFiles()) { if (this.program.getSourceFileByPath(f.path)) { @@ -405,16 +405,25 @@ namespace ts.server { const added: string[] = []; const removed: string[] = []; + let invalidateTypings = false; for (const id in currentFiles) { if (hasProperty(currentFiles, id) && !hasProperty(lastReportedFileNames, id)) { added.push(id); + if (this.typingFiles.indexOf(id) < 0) { + invalidateTypings = true; + break; + } } } for (const id in lastReportedFileNames) { if (hasProperty(lastReportedFileNames, id) && !hasProperty(currentFiles, id)) { removed.push(id); + invalidateTypings = true; } } + if (invalidateTypings) { + this.projectService.typingsCache.invalidateCachedTypingsForProject(this); + } this.lastReportedFileNames = currentFiles; this.lastReportedFileNames = currentFiles; diff --git a/src/server/typingsInstaller/nodeTypingsInstaller.ts b/src/server/typingsInstaller/nodeTypingsInstaller.ts index 50a3626700..a5334c4d65 100644 --- a/src/server/typingsInstaller/nodeTypingsInstaller.ts +++ b/src/server/typingsInstaller/nodeTypingsInstaller.ts @@ -114,23 +114,29 @@ namespace ts.server.typingsInstaller { protected runInstall(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void): void { const id = this.installRunCount; this.installRunCount++; - const command = `npm install ${typingsToInstall.map(t => "@types/" + t).join(" ")} --save-dev`; - if (this.log.isEnabled()) { - this.log.writeLine(`Running npm install @types ${id}, command '${command}'. cache path '${cachePath}'`); - } - this.exec(command, { cwd: cachePath }, (err, stdout, stderr) => { + let installCount = 0; + const installedTypings: string[] = []; + const expr = /^.*(@types\/\w+)\S*\s*$/gm; + let match: RegExpExecArray; + for (const typing of typingsToInstall) { + const command = `npm install @types/${typing} --save-dev`; if (this.log.isEnabled()) { - this.log.writeLine(`npm install @types ${id} stdout: ${stdout}`); - this.log.writeLine(`npm install @types ${id} stderr: ${stderr}`); + this.log.writeLine(`Running npm install @types ${id}, command '${command}'. cache path '${cachePath}'`); } - const installedTypings: string[] = []; - const expr = /^.*(node_modules)\\(@types)\\(\S+)\s*$/gm; - let match: RegExpExecArray; - while (match = expr.exec(stdout)) { - installedTypings.push(`${match[1]}/${match[2]}/${match[3]}`); - } - postInstallAction(installedTypings); - }); + this.exec(command, { cwd: cachePath }, (err, stdout, stderr) => { + installCount++; + if (this.log.isEnabled()) { + this.log.writeLine(`npm install @types ${id} stdout: ${stdout}`); + this.log.writeLine(`npm install @types ${id} stderr: ${stderr}`); + } + while (match = expr.exec(stdout)) { + installedTypings.push(`node_modules/${match[1]}`); + } + if (installCount >= typingsToInstall.length) { + postInstallAction(installedTypings); + } + }); + } } } From d15381682b40ec9f9f6aaddb3d159e9eed629ed9 Mon Sep 17 00:00:00 2001 From: Jason Ramsay Date: Tue, 30 Aug 2016 15:51:43 -0700 Subject: [PATCH 08/14] - invalidate typings fix - update gc timer --- src/server/project.ts | 18 +++--------------- src/server/session.ts | 2 +- src/server/typingsCache.ts | 6 +++--- .../typingsInstaller/nodeTypingsInstaller.ts | 8 ++++---- 4 files changed, 11 insertions(+), 23 deletions(-) diff --git a/src/server/project.ts b/src/server/project.ts index baf759c75c..448b956e44 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -283,7 +283,7 @@ namespace ts.server { return true; } let hasChanges = this.updateGraphWorker(); - const cachedTypings = this.projectService.typingsCache.getTypingsForProject(this); + const cachedTypings = this.projectService.typingsCache.getTypingsForProject(this, hasChanges); if (this.setTypings(cachedTypings)) { hasChanges = this.updateGraphWorker() || hasChanges; } @@ -312,7 +312,6 @@ namespace ts.server { // - newProgram is different from the old program and structure of the old program was not reused. if (!oldProgram || (this.program !== oldProgram && !oldProgram.structureIsReused)) { hasChanges = true; - //this.projectService.typingsCache.invalidateCachedTypingsForProject(this); if (oldProgram) { for (const f of oldProgram.getSourceFiles()) { if (this.program.getSourceFileByPath(f.path)) { @@ -405,27 +404,16 @@ namespace ts.server { const added: string[] = []; const removed: string[] = []; - let invalidateTypings = false; for (const id in currentFiles) { - if (hasProperty(currentFiles, id) && !hasProperty(lastReportedFileNames, id)) { + if (!hasProperty(lastReportedFileNames, id)) { added.push(id); - if (this.typingFiles.indexOf(id) < 0) { - invalidateTypings = true; - break; - } } } for (const id in lastReportedFileNames) { - if (hasProperty(lastReportedFileNames, id) && !hasProperty(currentFiles, id)) { + if (!hasProperty(currentFiles, id)) { removed.push(id); - invalidateTypings = true; } } - if (invalidateTypings) { - this.projectService.typingsCache.invalidateCachedTypingsForProject(this); - } - this.lastReportedFileNames = currentFiles; - this.lastReportedFileNames = currentFiles; this.lastReportedVersion = this.projectStructureVersion; return { info, changes: { added, removed }, projectErrors: this.projectErrors }; diff --git a/src/server/session.ts b/src/server/session.ts index 3da87f4063..2ab51b2198 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -168,7 +168,7 @@ namespace ts.server { : undefined; this.projectService = new ProjectService(host, logger, cancellationToken, useSingleInferredProject, typingsInstaller, eventHandler); - this.gcTimer = new GcTimer(host, /*delay*/ 15000, logger); + this.gcTimer = new GcTimer(host, /*delay*/ 7000, logger); } private handleEvent(event: ProjectServiceEvent) { diff --git a/src/server/typingsCache.ts b/src/server/typingsCache.ts index fd5b314951..889cfe1fb8 100644 --- a/src/server/typingsCache.ts +++ b/src/server/typingsCache.ts @@ -76,7 +76,7 @@ namespace ts.server { constructor(private readonly installer: ITypingsInstaller) { } - getTypingsForProject(project: Project): TypingsArray { + getTypingsForProject(project: Project, forceRefresh: boolean): TypingsArray { const typingOptions = project.getTypingOptions(); if (!typingOptions || !typingOptions.enableAutoDiscovery) { @@ -84,8 +84,8 @@ namespace ts.server { } const entry = this.perProjectCache[project.getProjectName()]; - const result: TypingsArray = entry && entry.typings.length > 0 ? entry.typings : toTypingsArray(typingOptions.include); - if (!entry || typingOptionsChanged(typingOptions, entry.typingOptions) || compilerOptionsChanged(project.getCompilerOptions(), entry.compilerOptions)) { + const result: TypingsArray = entry ? entry.typings : emptyArray; + if (forceRefresh || !entry || typingOptionsChanged(typingOptions, entry.typingOptions) || compilerOptionsChanged(project.getCompilerOptions(), entry.compilerOptions)) { // Note: entry is now poisoned since it does not really contain typings for a given combination of compiler options\typings options. // instead it acts as a placeholder to prevent issuing multiple requests this.perProjectCache[project.getProjectName()] = { diff --git a/src/server/typingsInstaller/nodeTypingsInstaller.ts b/src/server/typingsInstaller/nodeTypingsInstaller.ts index 6931b647a8..278d8d6302 100644 --- a/src/server/typingsInstaller/nodeTypingsInstaller.ts +++ b/src/server/typingsInstaller/nodeTypingsInstaller.ts @@ -92,7 +92,7 @@ namespace ts.server.typingsInstaller { protected runInstall(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void): void { const id = this.installRunCount; this.installRunCount++; - let installCount = 0; + let execInstallCmdCount = 0; const installedTypings: string[] = []; const expr = /^.*(@types\/\w+)\S*\s*$/gm; let match: RegExpExecArray; @@ -102,7 +102,7 @@ namespace ts.server.typingsInstaller { this.log.writeLine(`Running npm install @types ${id}, command '${command}'. cache path '${cachePath}'`); } this.exec(command, { cwd: cachePath }, (err, stdout, stderr) => { - installCount++; + execInstallCmdCount++; if (this.log.isEnabled()) { this.log.writeLine(`npm install @types ${id} stdout: ${stdout}`); this.log.writeLine(`npm install @types ${id} stderr: ${stderr}`); @@ -110,7 +110,7 @@ namespace ts.server.typingsInstaller { while (match = expr.exec(stdout)) { installedTypings.push(`node_modules/${match[1]}`); } - if (installCount >= typingsToInstall.length) { + if (execInstallCmdCount >= typingsToInstall.length) { postInstallAction(installedTypings); } }); @@ -121,7 +121,7 @@ namespace ts.server.typingsInstaller { function findArgument(argumentName: string) { const index = sys.args.indexOf(argumentName); return index >= 0 && index < sys.args.length - 1 - ? sys.args[index] + ? sys.args[index + 1] : undefined; } From 48736dea20796beb51391d1583a097e934dfa381 Mon Sep 17 00:00:00 2001 From: Jason Ramsay Date: Tue, 30 Aug 2016 16:02:01 -0700 Subject: [PATCH 09/14] onSourceFileChanged return immediately when info is undefined --- src/server/editorServices.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index fe8c11d262..bf08d893e3 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -312,6 +312,7 @@ namespace ts.server { const info = this.getScriptInfoForNormalizedPath(fileName); if (!info) { this.logger.info(`Error: got watch notification for unknown file: ${fileName}`); + return; } if (!this.host.fileExists(fileName)) { From 3a993c89f30f0b4b8ded5220d2e2a72973acc9e3 Mon Sep 17 00:00:00 2001 From: Jason Ramsay Date: Wed, 31 Aug 2016 21:14:24 -0700 Subject: [PATCH 10/14] update runInstall --- src/compiler/checker.ts | 3 ++ .../typingsInstaller/nodeTypingsInstaller.ts | 36 ++++++++++++++----- .../typingsInstaller/typingsInstaller.ts | 4 ++- 3 files changed, 34 insertions(+), 9 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index d38ce359e8..a209db4e73 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -18627,6 +18627,9 @@ namespace ts { continue; } const file = host.getSourceFile(resolvedDirective.resolvedFileName); + if (!file) { + continue; + } fileToDirective.set(file.path, key); } } diff --git a/src/server/typingsInstaller/nodeTypingsInstaller.ts b/src/server/typingsInstaller/nodeTypingsInstaller.ts index 278d8d6302..3745b32029 100644 --- a/src/server/typingsInstaller/nodeTypingsInstaller.ts +++ b/src/server/typingsInstaller/nodeTypingsInstaller.ts @@ -93,13 +93,11 @@ namespace ts.server.typingsInstaller { const id = this.installRunCount; this.installRunCount++; let execInstallCmdCount = 0; - const installedTypings: string[] = []; - const expr = /^.*(@types\/\w+)\S*\s*$/gm; - let match: RegExpExecArray; + let filteredTypings: string[] = []; for (const typing of typingsToInstall) { - const command = `npm install @types/${typing} --save-dev`; + const command = `npm view @types/${typing} --silent name`; if (this.log.isEnabled()) { - this.log.writeLine(`Running npm install @types ${id}, command '${command}'. cache path '${cachePath}'`); + this.log.writeLine(`Running npm view @types ${id}, command '${command}'.`); } this.exec(command, { cwd: cachePath }, (err, stdout, stderr) => { execInstallCmdCount++; @@ -107,11 +105,33 @@ namespace ts.server.typingsInstaller { this.log.writeLine(`npm install @types ${id} stdout: ${stdout}`); this.log.writeLine(`npm install @types ${id} stderr: ${stderr}`); } - while (match = expr.exec(stdout)) { - installedTypings.push(`node_modules/${match[1]}`); + if (stdout !== "") { + filteredTypings.push(typing); } if (execInstallCmdCount >= typingsToInstall.length) { - postInstallAction(installedTypings); + const command = `npm install ${filteredTypings.map(t => "@types/" + t).join(" ")} --save-dev -json`; + if (this.log.isEnabled()) { + this.log.writeLine(`Running npm install @types ${id}, command '${command}'. cache path '${cachePath}'`); + } + this.exec(command, { cwd: cachePath }, (err, stdout, stderr) => { + if (this.log.isEnabled()) { + this.log.writeLine(`npm install @types ${id} stdout: ${stdout}`); + this.log.writeLine(`npm install @types ${id} stderr: ${stderr}`); + } + const installedTypings: string[] = []; + try { + const response = JSON.parse(stdout); + if (response.dependencies) { + for (const typing in response.dependencies) { + installedTypings.push(typing); + } + } + } + catch (e) { + this.log.writeLine(`Error parsing installed @types dependencies. Error details: ${e.message}`); + } + postInstallAction(installedTypings); + }); } }); } diff --git a/src/server/typingsInstaller/typingsInstaller.ts b/src/server/typingsInstaller/typingsInstaller.ts index 619b307316..2905fb9aff 100644 --- a/src/server/typingsInstaller/typingsInstaller.ts +++ b/src/server/typingsInstaller/typingsInstaller.ts @@ -26,7 +26,6 @@ namespace ts.server.typingsInstaller { private packageNameToTypingLocation: Map = createMap(); private missingTypingsSet: Map = createMap(); private knownCachesSet: Map = createMap(); - private projectWatchers: Map = createMap(); abstract readonly installTypingHost: InstallTypingHost; @@ -203,6 +202,9 @@ namespace ts.server.typingsInstaller { if (!typingFile) { continue; } + if (!this.packageNameToTypingLocation[packageName]) { + this.packageNameToTypingLocation[packageName] = typingFile; + } installedTypingFiles.push(typingFile); } if (this.log.isEnabled()) { From 4f7a5185f8304cf136d176075ba0c62433b3c3c9 Mon Sep 17 00:00:00 2001 From: Jason Ramsay Date: Thu, 1 Sep 2016 12:30:32 -0700 Subject: [PATCH 11/14] switch to using npm ls -json instead npm install -json --- .../typingsInstaller/nodeTypingsInstaller.ts | 34 ++++++++++++------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/src/server/typingsInstaller/nodeTypingsInstaller.ts b/src/server/typingsInstaller/nodeTypingsInstaller.ts index 3745b32029..d9dddfabc9 100644 --- a/src/server/typingsInstaller/nodeTypingsInstaller.ts +++ b/src/server/typingsInstaller/nodeTypingsInstaller.ts @@ -109,7 +109,7 @@ namespace ts.server.typingsInstaller { filteredTypings.push(typing); } if (execInstallCmdCount >= typingsToInstall.length) { - const command = `npm install ${filteredTypings.map(t => "@types/" + t).join(" ")} --save-dev -json`; + const command = `npm install ${filteredTypings.map(t => "@types/" + t).join(" ")} --save-dev`; if (this.log.isEnabled()) { this.log.writeLine(`Running npm install @types ${id}, command '${command}'. cache path '${cachePath}'`); } @@ -118,19 +118,29 @@ namespace ts.server.typingsInstaller { this.log.writeLine(`npm install @types ${id} stdout: ${stdout}`); this.log.writeLine(`npm install @types ${id} stderr: ${stderr}`); } - const installedTypings: string[] = []; - try { - const response = JSON.parse(stdout); - if (response.dependencies) { - for (const typing in response.dependencies) { - installedTypings.push(typing); + if (stdout === "") { + return; + } + const command = "npm ls -json"; + this.exec(command, { cwd: cachePath }, (err, stdout, stderr) => { + if (this.log.isEnabled()) { + this.log.writeLine(`npm ls -json ${id} stdout: ${stdout}`); + this.log.writeLine(`npm ls -json ${id} stderr: ${stderr}`); + } + const installedTypings: string[] = []; + try { + const response = JSON.parse(stdout); + if (response.dependencies) { + for (const typing in response.dependencies) { + installedTypings.push(typing); + } } } - } - catch (e) { - this.log.writeLine(`Error parsing installed @types dependencies. Error details: ${e.message}`); - } - postInstallAction(installedTypings); + catch (e) { + this.log.writeLine(`Error parsing installed @types dependencies. Error details: ${e.message}`); + } + postInstallAction(installedTypings); + }); }); } }); From 511b3cd69b2d32910ce26ef2d9799d2c29ce680e Mon Sep 17 00:00:00 2001 From: Jason Ramsay Date: Thu, 1 Sep 2016 14:12:31 -0700 Subject: [PATCH 12/14] Fix existing TypingsInstaller tests --- src/harness/unittests/tsserverProjectSystem.ts | 6 +----- src/harness/unittests/typingsInstaller.ts | 8 ++++---- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 941215fc3a..9bf1523572 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -56,15 +56,11 @@ namespace ts.projectSystem { return this.installTypingHost; } - installPackage(packageName: string) { - return true; - } - isPackageInstalled(packageName: string) { return true; } - runTsd(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void) { + runInstall(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void) { this.postInstallActions.push(map => { postInstallAction(map(typingsToInstall)); }); diff --git a/src/harness/unittests/typingsInstaller.ts b/src/harness/unittests/typingsInstaller.ts index 18c1e89ec7..408d11a720 100644 --- a/src/harness/unittests/typingsInstaller.ts +++ b/src/harness/unittests/typingsInstaller.ts @@ -157,7 +157,7 @@ namespace ts.projectSystem { }; const host = createServerHost([file1]); let enqueueIsCalled = false; - let runTsdIsCalled = false; + let runInstallIsCalled = false; const installer = new (class extends TestTypingsInstaller { constructor() { super("", host); @@ -168,8 +168,8 @@ namespace ts.projectSystem { } runTsd(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void): void { assert.deepEqual(typingsToInstall, ["node"]); - runTsdIsCalled = true; - super.runTsd(cachePath, typingsToInstall, postInstallAction); + runInstallIsCalled = true; + super.runInstall(cachePath, typingsToInstall, postInstallAction); } })(); @@ -184,7 +184,7 @@ namespace ts.projectSystem { // 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(runTsdIsCalled, "expected 'runTsdIsCalled' to be true"); + assert.isTrue(runInstallIsCalled, "expected 'runInstallIsCalled' to be true"); }); }); } \ No newline at end of file From 798232c9752dba67380bd315c79eea0686651741 Mon Sep 17 00:00:00 2001 From: Jason Ramsay Date: Thu, 1 Sep 2016 18:28:37 -0700 Subject: [PATCH 13/14] Fixing unit tests, lint errors & addressing CR feedback --- src/harness/unittests/typingsInstaller.ts | 19 ++-- .../typingsInstaller/nodeTypingsInstaller.ts | 90 +++++++++---------- .../typingsInstaller/typingsInstaller.ts | 4 +- 3 files changed, 57 insertions(+), 56 deletions(-) diff --git a/src/harness/unittests/typingsInstaller.ts b/src/harness/unittests/typingsInstaller.ts index 408d11a720..a8b4039e00 100644 --- a/src/harness/unittests/typingsInstaller.ts +++ b/src/harness/unittests/typingsInstaller.ts @@ -4,7 +4,8 @@ namespace ts.projectSystem { describe("typings installer", () => { - it("configured projects (tsd installed) 1", () => { + it("configured projects (typings installed) 1", () => { + debugger; const file1 = { path: "/a/b/app.js", content: "" @@ -31,7 +32,7 @@ namespace ts.projectSystem { }; const jquery = { - path: "/a/data/typings/jquery/jquery.d.ts", + path: "/a/data/node_modules/@types/jquery/index.d.ts", content: "declare const $: { x: number }" }; @@ -44,18 +45,18 @@ namespace ts.projectSystem { const p = projectService.configuredProjects[0]; checkProjectActualFiles(p, [file1.path]); - assert(host.fileExists(combinePaths(installer.globalTypingsCacheLocation, "tsd.json"))); + assert(host.fileExists(combinePaths(installer.globalTypingsCacheLocation, "package.json"))); installer.runPostInstallActions(t => { assert.deepEqual(t, ["jquery"]); host.createFileOrFolder(jquery, /*createParentDirectory*/ true); - return ["jquery/jquery.d.ts"]; + return ["@types/jquery"]; }); checkNumberOfProjects(projectService, { configuredProjects: 1 }); checkProjectActualFiles(p, [file1.path, jquery.path]); }); - it("inferred project (tsd installed)", () => { + it("inferred project (typings installed)", () => { const file1 = { path: "/a/b/app.js", content: "" @@ -71,7 +72,7 @@ namespace ts.projectSystem { }; const jquery = { - path: "/a/data/typings/jquery/jquery.d.ts", + path: "/a/data/node_modules/@types/jquery/index.d.ts", content: "declare const $: { x: number }" }; const host = createServerHost([file1, packageJson]); @@ -84,12 +85,12 @@ namespace ts.projectSystem { const p = projectService.inferredProjects[0]; checkProjectActualFiles(p, [file1.path]); - assert(host.fileExists(combinePaths(installer.globalTypingsCacheLocation, "tsd.json"))); + assert(host.fileExists(combinePaths(installer.globalTypingsCacheLocation, "package.json"))); installer.runPostInstallActions(t => { assert.deepEqual(t, ["jquery"]); host.createFileOrFolder(jquery, /*createParentDirectory*/ true); - return ["jquery/jquery.d.ts"]; + return ["@types/jquery"]; }); checkNumberOfProjects(projectService, { inferredProjects: 1 }); checkProjectActualFiles(p, [file1.path, jquery.path]); @@ -166,7 +167,7 @@ namespace ts.projectSystem { enqueueIsCalled = true; super.enqueueInstallTypingsRequest(project, typingOptions); } - runTsd(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void): void { + runInstall(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void): void { assert.deepEqual(typingsToInstall, ["node"]); runInstallIsCalled = true; super.runInstall(cachePath, typingsToInstall, postInstallAction); diff --git a/src/server/typingsInstaller/nodeTypingsInstaller.ts b/src/server/typingsInstaller/nodeTypingsInstaller.ts index d9dddfabc9..0d22b1b364 100644 --- a/src/server/typingsInstaller/nodeTypingsInstaller.ts +++ b/src/server/typingsInstaller/nodeTypingsInstaller.ts @@ -93,58 +93,58 @@ namespace ts.server.typingsInstaller { const id = this.installRunCount; this.installRunCount++; let execInstallCmdCount = 0; - let filteredTypings: string[] = []; + const filteredTypings: string[] = []; for (const typing of typingsToInstall) { const command = `npm view @types/${typing} --silent name`; - if (this.log.isEnabled()) { - this.log.writeLine(`Running npm view @types ${id}, command '${command}'.`); - } - this.exec(command, { cwd: cachePath }, (err, stdout, stderr) => { - execInstallCmdCount++; - if (this.log.isEnabled()) { - this.log.writeLine(`npm install @types ${id} stdout: ${stdout}`); - this.log.writeLine(`npm install @types ${id} stderr: ${stderr}`); - } - if (stdout !== "") { + this.execAsync("npm view", command, cachePath, id, (err, stdout, stderr) => { + if (stdout) { filteredTypings.push(typing); } - if (execInstallCmdCount >= typingsToInstall.length) { - const command = `npm install ${filteredTypings.map(t => "@types/" + t).join(" ")} --save-dev`; - if (this.log.isEnabled()) { - this.log.writeLine(`Running npm install @types ${id}, command '${command}'. cache path '${cachePath}'`); - } - this.exec(command, { cwd: cachePath }, (err, stdout, stderr) => { - if (this.log.isEnabled()) { - this.log.writeLine(`npm install @types ${id} stdout: ${stdout}`); - this.log.writeLine(`npm install @types ${id} stderr: ${stderr}`); - } - if (stdout === "") { - return; - } - const command = "npm ls -json"; - this.exec(command, { cwd: cachePath }, (err, stdout, stderr) => { - if (this.log.isEnabled()) { - this.log.writeLine(`npm ls -json ${id} stdout: ${stdout}`); - this.log.writeLine(`npm ls -json ${id} stderr: ${stderr}`); - } - const installedTypings: string[] = []; - try { - const response = JSON.parse(stdout); - if (response.dependencies) { - for (const typing in response.dependencies) { - installedTypings.push(typing); - } - } - } - catch (e) { - this.log.writeLine(`Error parsing installed @types dependencies. Error details: ${e.message}`); - } - postInstallAction(installedTypings); - }); - }); + execInstallCmdCount++; + if (execInstallCmdCount === typingsToInstall.length) { + installFilteredTypings(this, filteredTypings); } }); } + + function installFilteredTypings(self: NodeTypingsInstaller, filteredTypings: string[]) { + 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) { + 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}`); + } + cb(err, stdout, stderr); + }); } } diff --git a/src/server/typingsInstaller/typingsInstaller.ts b/src/server/typingsInstaller/typingsInstaller.ts index 2905fb9aff..c96fe3c6c4 100644 --- a/src/server/typingsInstaller/typingsInstaller.ts +++ b/src/server/typingsInstaller/typingsInstaller.ts @@ -135,7 +135,7 @@ namespace ts.server.typingsInstaller { if (!packageName) { continue; } - var typingFile = typingToFileName(cacheLocation, packageName, this.installTypingHost); + const typingFile = typingToFileName(cacheLocation, packageName, this.installTypingHost); if (!typingFile) { continue; } @@ -198,7 +198,7 @@ namespace ts.server.typingsInstaller { continue; } installedPackages[packageName] = true; - var typingFile = typingToFileName(cachePath, packageName, this.installTypingHost); + const typingFile = typingToFileName(cachePath, packageName, this.installTypingHost); if (!typingFile) { continue; } From 7d0fc648d8840a26a5aefa6bb07fab1fe1a10067 Mon Sep 17 00:00:00 2001 From: Jason Ramsay Date: Fri, 2 Sep 2016 15:37:38 -0700 Subject: [PATCH 14/14] Adding typingsInstaller unit tests --- .../unittests/tsserverProjectSystem.ts | 19 +- src/harness/unittests/typingsInstaller.ts | 182 +++++++++++++++++- 2 files changed, 197 insertions(+), 4 deletions(-) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 9bf1523572..71a835b0ef 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -2,6 +2,17 @@ /// namespace ts.projectSystem { + const safeList = { + path: "/safeList.json", + content: JSON.stringify({ + commander: "commander", + express: "express", + jquery: "jquery", + lodash: "lodash", + moment: "moment" + }) + }; + export function notImplemented(): any { throw new Error("Not yet implemented"); } @@ -31,11 +42,11 @@ namespace ts.projectSystem { export class TestTypingsInstaller extends server.typingsInstaller.TypingsInstaller implements server.ITypingsInstaller { protected projectService: server.ProjectService; constructor(readonly globalTypingsCacheLocation: string, readonly installTypingHost: server.ServerHost) { - super(globalTypingsCacheLocation, ""); + super(globalTypingsCacheLocation, safeList.path); this.init(); } - safeFileList = ""; + safeFileList = safeList.path; postInstallActions: ((map: (t: string[]) => string[]) => void)[] = []; runPostInstallActions(map: (t: string[]) => string[]) { @@ -102,11 +113,13 @@ namespace ts.projectSystem { if (!params) { params = {}; } - return new TestServerHost( + const host = new TestServerHost( params.useCaseSensitiveFileNames !== undefined ? params.useCaseSensitiveFileNames : false, params.executingFilePath || getExecutingFilePathFromLibFile(libFilePath), params.currentDirectory || "/", fileOrFolderList); + host.createFileOrFolder(safeList, /*createParentDirectory*/ true); + return host; } export interface CreateProjectServiceParameters { diff --git a/src/harness/unittests/typingsInstaller.ts b/src/harness/unittests/typingsInstaller.ts index a8b4039e00..47373bfd26 100644 --- a/src/harness/unittests/typingsInstaller.ts +++ b/src/harness/unittests/typingsInstaller.ts @@ -5,7 +5,6 @@ namespace ts.projectSystem { describe("typings installer", () => { it("configured projects (typings installed) 1", () => { - debugger; const file1 = { path: "/a/b/app.js", content: "" @@ -187,5 +186,186 @@ namespace ts.projectSystem { 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", () => { + // Tests: + // 1. react typings are installed for .jsx + // 2. loose files names are matched against safe list for typings if + // this is a JS project (only js, jsx, d.ts files are present) + const file1 = { + path: "/a/b/lodash.js", + content: "" + }; + const file2 = { + path: "/a/b/file2.jsx", + content: "" + }; + const file3 = { + path: "/a/b/file3.d.ts", + content: "" + }; + const react = { + path: "/a/data/node_modules/@types/react/index.d.ts", + content: "declare const react: { x: number }" + }; + const lodash = { + path: "/a/data/node_modules/@types/lodash/index.d.ts", + content: "declare const lodash: { x: number }" + }; + + const host = createServerHost([file1, file2, file3]); + const installer = new TestTypingsInstaller("/a/data/", host); + + 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: {} + }); + + const p = projectService.externalProjects[0]; + 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"]; + }); + + checkNumberOfProjects(projectService, { externalProjects: 1 }); + checkProjectActualFiles(p, [file1.path, file2.path, file3.path, lodash.path, react.path]); + }); + + it("external project - no typing options, with js & ts files", () => { + // Tests: + // 1. No typings are included for JS projects when the project contains ts files + const file1 = { + path: "/a/b/jquery.js", + content: "" + }; + const file2 = { + path: "/a/b/file2.ts", + content: "" + }; + + 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); + } + enqueueInstallTypingsRequest(project: server.Project, typingOptions: TypingOptions) { + enqueueIsCalled = true; + super.enqueueInstallTypingsRequest(project, typingOptions); + } + runInstall(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void): void { + runInstallIsCalled = true; + super.runInstall(cachePath, typingsToInstall, postInstallAction); + } + })(); + + 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)], + typingOptions: {} + }); + + const p = projectService.externalProjects[0]; + projectService.checkNumberOfProjects({ externalProjects: 1 }); + checkProjectActualFiles(p, [file1.path, file2.path]); + + installer.runPostInstallActions(t => { + runPostInstallIsCalled = true; + return []; + }); + + 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", () => { + // Tests: + // 1. Safelist matching, typing options includes/excludes and package.json typings are all acquired + // 2. Types for safelist matches are not included when they also appear in the typing option exclude list + // 3. Multiple includes and excludes are respected in typing options + 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" + } + }) + }; + + 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 host = createServerHost([file1, file2, file3, packageJson]); + const installer = new TestTypingsInstaller("/a/data/", host); + + 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"], exclude: ["lodash"] } + }); + + const p = projectService.externalProjects[0]; + 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"]; + }); + + checkNumberOfProjects(projectService, { externalProjects: 1 }); + checkProjectActualFiles(p, [file1.path, file2.path, file3.path, commander.path, express.path, jquery.path, moment.path]); + }); }); } \ No newline at end of file