From a477d1f7bbdea99ebf022e0b99f558d4c6add24f Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Thu, 20 Oct 2016 21:15:47 -0700 Subject: [PATCH] Merge pull request #11764 from Microsoft/vladima/11744 watch configuration files if they exist even if they cannot be parsed --- src/harness/unittests/typingsInstaller.ts | 69 +++++++++++++++---- .../typingsInstaller/typingsInstaller.ts | 6 +- src/services/jsTyping.ts | 4 +- 3 files changed, 63 insertions(+), 16 deletions(-) diff --git a/src/harness/unittests/typingsInstaller.ts b/src/harness/unittests/typingsInstaller.ts index 323d8c0ed9e37..4a0021e6ad43d 100644 --- a/src/harness/unittests/typingsInstaller.ts +++ b/src/harness/unittests/typingsInstaller.ts @@ -569,7 +569,7 @@ namespace ts.projectSystem { } executeRequest(requestKind: TI.RequestKind, _requestId: number, args: string[], _cwd: string, cb: TI.RequestCompletedAction): void { if (requestKind === TI.NpmInstallRequest) { - let typingFiles: (FileOrFolder & { typings: string}) [] = []; + let typingFiles: (FileOrFolder & { typings: string })[] = []; if (args.indexOf("@types/commander") >= 0) { typingFiles = [commander, jquery, lodash, cordova]; } @@ -591,7 +591,7 @@ namespace ts.projectSystem { projectFileName: projectFileName1, options: { allowJS: true, moduleResolution: ModuleResolutionKind.NodeJs }, rootFiles: [toExternalFile(lodashJs.path), toExternalFile(commanderJs.path), toExternalFile(file3.path)], - typingOptions: { include: ["jquery", "cordova" ] } + typingOptions: { include: ["jquery", "cordova"] } }); installer.checkPendingCommands([TI.NpmViewRequest, TI.NpmViewRequest, TI.NpmViewRequest]); @@ -626,7 +626,7 @@ namespace ts.projectSystem { 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 ]); + checkProjectActualFiles(p2, [file3.path, grunt.path, gulp.path]); }); it("configured projects discover from node_modules", () => { @@ -687,10 +687,10 @@ namespace ts.projectSystem { const bowerJson = { path: "/bower.json", content: JSON.stringify({ - "dependencies": { - "jquery": "^3.1.0" - } - }) + "dependencies": { + "jquery": "^3.1.0" + } + }) }; const jqueryDTS = { path: "/tmp/node_modules/@types/jquery/index.d.ts", @@ -720,26 +720,69 @@ namespace ts.projectSystem { checkNumberOfProjects(projectService, { configuredProjects: 1 }); checkProjectActualFiles(p, [app.path, jqueryDTS.path]); }); + + it("Malformed package.json should be watched", () => { + const f = { + path: "/a/b/app.js", + content: "var x = require('commander')" + }; + const brokenPackageJson = { + path: "/a/b/package.json", + content: `{ "dependencies": { "co } }` + }; + const fixedPackageJson = { + path: brokenPackageJson.path, + content: `{ "dependencies": { "commander": "0.0.2" } }` + }; + const cachePath = "/a/cache/"; + const commander = { + path: cachePath + "node_modules/@types/commander/index.d.ts", + content: "export let x: number" + }; + const host = createServerHost([f, brokenPackageJson]); + const installer = new (class extends Installer { + constructor() { + super(host, { globalTypingsCacheLocation: cachePath }); + } + executeRequest(requestKind: TI.RequestKind, _requestId: number, _args: string[], _cwd: string, cb: server.typingsInstaller.RequestCompletedAction) { + const installedTypings = ["@types/commander"]; + const typingFiles = [commander]; + executeCommand(this, host, installedTypings, typingFiles, requestKind, cb); + } + })(); + const service = createProjectService(host, { typingsInstaller: installer }); + service.openClientFile(f.path); + + installer.checkPendingCommands([]); + + host.reloadFS([f, fixedPackageJson]); + host.triggerFileWatcherCallback(fixedPackageJson.path, /*removed*/ false); + // expected one view and one install request + installer.installAll([TI.NpmViewRequest], [TI.NpmInstallRequest]); + + service.checkNumberOfProjects({ inferredProjects: 1 }); + checkProjectActualFiles(service.inferredProjects[0], [f.path, commander.path]); + }); }); describe("Validate package name:", () => { - it ("name cannot be too long", () => { + it("name cannot be too long", () => { let packageName = "a"; for (let i = 0; i < 8; i++) { packageName += packageName; } assert.equal(TI.validatePackageName(packageName), TI.PackageNameValidationResult.NameTooLong); }); - it ("name cannot start with dot", () => { + it("name cannot start with dot", () => { assert.equal(TI.validatePackageName(".foo"), TI.PackageNameValidationResult.NameStartsWithDot); }); - it ("name cannot start with underscore", () => { + it("name cannot start with underscore", () => { assert.equal(TI.validatePackageName("_foo"), TI.PackageNameValidationResult.NameStartsWithUnderscore); }); - it ("scoped packages not supported", () => { + it("scoped packages not supported", () => { assert.equal(TI.validatePackageName("@scope/bar"), TI.PackageNameValidationResult.ScopedPackagesNotSupported); }); - it ("non URI safe characters are not supported", () => { + it("non URI safe characters are not supported", () => { assert.equal(TI.validatePackageName(" scope "), TI.PackageNameValidationResult.NameContainsNonURISafeCharacters); assert.equal(TI.validatePackageName("; say ‘Hello from TypeScript!’ #"), TI.PackageNameValidationResult.NameContainsNonURISafeCharacters); assert.equal(TI.validatePackageName("a/b/c"), TI.PackageNameValidationResult.NameContainsNonURISafeCharacters); @@ -747,7 +790,7 @@ namespace ts.projectSystem { }); describe("Invalid package names", () => { - it ("should not be installed", () => { + it("should not be installed", () => { const f1 = { path: "/a/b/app.js", content: "let x = 1" diff --git a/src/server/typingsInstaller/typingsInstaller.ts b/src/server/typingsInstaller/typingsInstaller.ts index 2851da3a64d8a..21f85c241a6e9 100644 --- a/src/server/typingsInstaller/typingsInstaller.ts +++ b/src/server/typingsInstaller/typingsInstaller.ts @@ -381,8 +381,10 @@ namespace ts.server.typingsInstaller { if (this.log.isEnabled()) { this.log.writeLine(`Got FS notification for ${f}, handler is already invoked '${isInvoked}'`); } - this.sendResponse({ projectName: projectName, kind: "invalidate" }); - isInvoked = true; + if (!isInvoked) { + this.sendResponse({ projectName: projectName, kind: "invalidate" }); + isInvoked = true; + } }); watchers.push(w); } diff --git a/src/services/jsTyping.ts b/src/services/jsTyping.ts index 96654ec670287..561e0110cdaa5 100644 --- a/src/services/jsTyping.ts +++ b/src/services/jsTyping.ts @@ -135,10 +135,12 @@ namespace ts.JsTyping { * Get the typing info from common package manager json files like package.json or bower.json */ function getTypingNamesFromJson(jsonPath: string, filesToWatch: string[]) { + if (host.fileExists(jsonPath)) { + filesToWatch.push(jsonPath); + } const result = readConfigFile(jsonPath, (path: string) => host.readFile(path)); if (result.config) { const jsonConfig: PackageJson = result.config; - filesToWatch.push(jsonPath); if (jsonConfig.dependencies) { mergeTypings(getOwnKeys(jsonConfig.dependencies)); }