Skip to content

Commit 1d31716

Browse files
authored
Merge pull request #11764 from Microsoft/vladima/11744
watch configuration files if they exist even if they cannot be parsed
2 parents 3ab3d95 + 8f55333 commit 1d31716

File tree

3 files changed

+64
-17
lines changed

3 files changed

+64
-17
lines changed

src/harness/unittests/typingsInstaller.ts

Lines changed: 56 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -569,7 +569,7 @@ namespace ts.projectSystem {
569569
}
570570
executeRequest(requestKind: TI.RequestKind, requestId: number, args: string[], cwd: string, cb: TI.RequestCompletedAction): void {
571571
if (requestKind === TI.NpmInstallRequest) {
572-
let typingFiles: (FileOrFolder & { typings: string}) [] = [];
572+
let typingFiles: (FileOrFolder & { typings: string })[] = [];
573573
if (args.indexOf("@types/commander") >= 0) {
574574
typingFiles = [commander, jquery, lodash, cordova];
575575
}
@@ -591,7 +591,7 @@ namespace ts.projectSystem {
591591
projectFileName: projectFileName1,
592592
options: { allowJS: true, moduleResolution: ModuleResolutionKind.NodeJs },
593593
rootFiles: [toExternalFile(lodashJs.path), toExternalFile(commanderJs.path), toExternalFile(file3.path)],
594-
typingOptions: { include: ["jquery", "cordova" ] }
594+
typingOptions: { include: ["jquery", "cordova"] }
595595
});
596596

597597
installer.checkPendingCommands([TI.NpmViewRequest, TI.NpmViewRequest, TI.NpmViewRequest]);
@@ -626,7 +626,7 @@ namespace ts.projectSystem {
626626
installer.executePendingCommands();
627627

628628
checkProjectActualFiles(p1, [lodashJs.path, commanderJs.path, file3.path, commander.path, jquery.path, lodash.path, cordova.path]);
629-
checkProjectActualFiles(p2, [file3.path, grunt.path, gulp.path ]);
629+
checkProjectActualFiles(p2, [file3.path, grunt.path, gulp.path]);
630630
});
631631

632632
it("configured projects discover from node_modules", () => {
@@ -687,10 +687,10 @@ namespace ts.projectSystem {
687687
const bowerJson = {
688688
path: "/bower.json",
689689
content: JSON.stringify({
690-
"dependencies": {
691-
"jquery": "^3.1.0"
692-
}
693-
})
690+
"dependencies": {
691+
"jquery": "^3.1.0"
692+
}
693+
})
694694
};
695695
const jqueryDTS = {
696696
path: "/tmp/node_modules/@types/jquery/index.d.ts",
@@ -720,34 +720,77 @@ namespace ts.projectSystem {
720720
checkNumberOfProjects(projectService, { configuredProjects: 1 });
721721
checkProjectActualFiles(p, [app.path, jqueryDTS.path]);
722722
});
723+
724+
it("Malformed package.json should be watched", () => {
725+
const f = {
726+
path: "/a/b/app.js",
727+
content: "var x = require('commander')"
728+
};
729+
const brokenPackageJson = {
730+
path: "/a/b/package.json",
731+
content: `{ "dependencies": { "co } }`
732+
};
733+
const fixedPackageJson = {
734+
path: brokenPackageJson.path,
735+
content: `{ "dependencies": { "commander": "0.0.2" } }`
736+
};
737+
const cachePath = "/a/cache/";
738+
const commander = {
739+
path: cachePath + "node_modules/@types/commander/index.d.ts",
740+
content: "export let x: number"
741+
};
742+
const host = createServerHost([f, brokenPackageJson]);
743+
const installer = new (class extends Installer {
744+
constructor() {
745+
super(host, { globalTypingsCacheLocation: cachePath });
746+
}
747+
executeRequest(requestKind: TI.RequestKind, requestId: number, args: string[], cwd: string, cb: server.typingsInstaller.RequestCompletedAction) {
748+
const installedTypings = ["@types/commander"];
749+
const typingFiles = [commander];
750+
executeCommand(this, host, installedTypings, typingFiles, requestKind, cb);
751+
}
752+
})();
753+
const service = createProjectService(host, { typingsInstaller: installer });
754+
service.openClientFile(f.path);
755+
756+
installer.checkPendingCommands([]);
757+
758+
host.reloadFS([f, fixedPackageJson]);
759+
host.triggerFileWatcherCallback(fixedPackageJson.path, /*removed*/ false);
760+
// expected one view and one install request
761+
installer.installAll([TI.NpmViewRequest], [TI.NpmInstallRequest]);
762+
763+
service.checkNumberOfProjects({ inferredProjects: 1 });
764+
checkProjectActualFiles(service.inferredProjects[0], [f.path, commander.path]);
765+
});
723766
});
724767

725768
describe("Validate package name:", () => {
726-
it ("name cannot be too long", () => {
769+
it("name cannot be too long", () => {
727770
let packageName = "a";
728771
for (let i = 0; i < 8; i++) {
729772
packageName += packageName;
730773
}
731774
assert.equal(TI.validatePackageName(packageName), TI.PackageNameValidationResult.NameTooLong);
732775
});
733-
it ("name cannot start with dot", () => {
776+
it("name cannot start with dot", () => {
734777
assert.equal(TI.validatePackageName(".foo"), TI.PackageNameValidationResult.NameStartsWithDot);
735778
});
736-
it ("name cannot start with underscore", () => {
779+
it("name cannot start with underscore", () => {
737780
assert.equal(TI.validatePackageName("_foo"), TI.PackageNameValidationResult.NameStartsWithUnderscore);
738781
});
739-
it ("scoped packages not supported", () => {
782+
it("scoped packages not supported", () => {
740783
assert.equal(TI.validatePackageName("@scope/bar"), TI.PackageNameValidationResult.ScopedPackagesNotSupported);
741784
});
742-
it ("non URI safe characters are not supported", () => {
785+
it("non URI safe characters are not supported", () => {
743786
assert.equal(TI.validatePackageName(" scope "), TI.PackageNameValidationResult.NameContainsNonURISafeCharacters);
744787
assert.equal(TI.validatePackageName("; say ‘Hello from TypeScript!’ #"), TI.PackageNameValidationResult.NameContainsNonURISafeCharacters);
745788
assert.equal(TI.validatePackageName("a/b/c"), TI.PackageNameValidationResult.NameContainsNonURISafeCharacters);
746789
});
747790
});
748791

749792
describe("Invalid package names", () => {
750-
it ("should not be installed", () => {
793+
it("should not be installed", () => {
751794
const f1 = {
752795
path: "/a/b/app.js",
753796
content: "let x = 1"

src/server/typingsInstaller/typingsInstaller.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ namespace ts.server.typingsInstaller {
1515

1616
const nullLog: Log = {
1717
isEnabled: () => false,
18-
writeLine: () => {}
18+
writeLine: () => { }
1919
};
2020

2121
function typingToFileName(cachePath: string, packageName: string, installTypingHost: InstallTypingHost): string {
@@ -382,8 +382,10 @@ namespace ts.server.typingsInstaller {
382382
if (this.log.isEnabled()) {
383383
this.log.writeLine(`Got FS notification for ${f}, handler is already invoked '${isInvoked}'`);
384384
}
385-
this.sendResponse({ projectName: projectName, kind: "invalidate" });
386-
isInvoked = true;
385+
if (!isInvoked) {
386+
this.sendResponse({ projectName: projectName, kind: "invalidate" });
387+
isInvoked = true;
388+
}
387389
});
388390
watchers.push(w);
389391
}

src/services/jsTyping.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,10 +136,12 @@ namespace ts.JsTyping {
136136
* Get the typing info from common package manager json files like package.json or bower.json
137137
*/
138138
function getTypingNamesFromJson(jsonPath: string, filesToWatch: string[]) {
139+
if (host.fileExists(jsonPath)) {
140+
filesToWatch.push(jsonPath);
141+
}
139142
const result = readConfigFile(jsonPath, (path: string) => host.readFile(path));
140143
if (result.config) {
141144
const jsonConfig: PackageJson = result.config;
142-
filesToWatch.push(jsonPath);
143145
if (jsonConfig.dependencies) {
144146
mergeTypings(getOwnKeys(jsonConfig.dependencies));
145147
}

0 commit comments

Comments
 (0)