Merge pull request #11776 from Microsoft/vladima/port-11764

Ports PR #11764 into master
microsoft · Oct 21, 2016 · 3263fde · 3263fde
2 parents 46f7a0f + a477d1f
commit 3263fde
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 16 deletions.
diff --git 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,34 +720,77 @@ 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);
         });
     });
 
     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
@@ -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
@@ -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));
                 }