From b4b29ab1e1365673c4db7c5cc96ece3f773ed39f Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 9 Oct 2018 13:21:43 -0700 Subject: [PATCH 1/2] Converted legacySafelist to map to avoid using array prototype methods accidently to match with filename Eg. constructor.js was adding constructor function to aquisition.include which resulted in the mismatch between typing installer's typeAquisition (which was passed as JSON string and parsed back to null) and one in project That meant we kept on queuing the new typing installation request. Fixes #27435 --- src/server/editorServices.ts | 10 ++-- .../unittests/tsserverProjectSystem.ts | 57 +++++++++++++++++++ 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 1fc71711ea..30a9844eaf 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -486,7 +486,7 @@ namespace ts.server { private readonly hostConfiguration: HostConfiguration; private safelist: SafeList = defaultTypeSafeList; - private legacySafelist: { [key: string]: string } = {}; + private readonly legacySafelist = createMap(); private pendingProjectUpdates = createMap(); /* @internal */ @@ -638,14 +638,14 @@ namespace ts.server { this.safelist = raw.typesMap; for (const key in raw.simpleMap) { if (raw.simpleMap.hasOwnProperty(key)) { - this.legacySafelist[key] = raw.simpleMap[key].toLowerCase(); + this.legacySafelist.set(key, raw.simpleMap[key].toLowerCase()); } } } catch (e) { this.logger.info(`Error loading types map: ${e}`); this.safelist = defaultTypeSafeList; - this.legacySafelist = {}; + this.legacySafelist.clear(); } } @@ -2721,13 +2721,13 @@ namespace ts.server { if (fileExtensionIs(baseName, "js")) { const inferredTypingName = removeFileExtension(baseName); const cleanedTypingName = removeMinAndVersionNumbers(inferredTypingName); - if (this.legacySafelist[cleanedTypingName]) { + const typeName = this.legacySafelist.get(cleanedTypingName); + if (typeName !== undefined) { this.logger.info(`Excluded '${normalizedNames[i]}' because it matched ${cleanedTypingName} from the legacy safelist`); excludedFiles.push(normalizedNames[i]); // *exclude* it from the project... exclude = true; // ... but *include* it in the list of types to acquire - const typeName = this.legacySafelist[cleanedTypingName]; // Same best-effort dedupe as above if (typeAcqInclude.indexOf(typeName) < 0) { typeAcqInclude.push(typeName); diff --git a/src/testRunner/unittests/tsserverProjectSystem.ts b/src/testRunner/unittests/tsserverProjectSystem.ts index 2105ce6ef4..9c4d776d54 100644 --- a/src/testRunner/unittests/tsserverProjectSystem.ts +++ b/src/testRunner/unittests/tsserverProjectSystem.ts @@ -1826,6 +1826,63 @@ namespace ts.projectSystem { } }); + it("file with name constructor.js doesnt cause issue with typeAcquisition when safe type list", () => { + const file1 = { + path: "/a/b/f1.js", + content: `export let x = 5; import { s } from "s"` + }; + const constructorFile = { + path: "/a/b/constructor.js", + content: "const x = 10;" + }; + const bliss = { + path: "/a/b/bliss.js", + content: "export function is() { return true; }" + }; + const host = createServerHost([file1, libFile, constructorFile, bliss, customTypesMap]); + let request: string | undefined; + const cachePath = "/a/data"; + const typingsInstaller: server.ITypingsInstaller = { + isKnownTypesPackageName: returnFalse, + installPackage: notImplemented, + inspectValue: notImplemented, + enqueueInstallTypingsRequest: (proj, typeAcquisition, unresolvedImports) => { + assert.isUndefined(request); + request = JSON.stringify(server.createInstallTypingsRequest(proj, typeAcquisition, unresolvedImports || server.emptyArray, cachePath)); + }, + attach: noop, + onProjectClosed: noop, + globalTypingsCacheLocation: cachePath + }; + + const projectName = "project"; + const projectService = createProjectService(host, { typingsInstaller }); + projectService.openExternalProject({ projectFileName: projectName, options: {}, rootFiles: toExternalFiles([file1.path, constructorFile.path, bliss.path]) }); + assert.equal(request, JSON.stringify({ + projectName, + fileNames: [libFile.path, file1.path, constructorFile.path, bliss.path], + compilerOptions: { allowNonTsExtensions: true, noEmitForJsFiles: true }, + typeAcquisition: { include: ["blissfuljs"], exclude: [], enable: true }, + unresolvedImports: ["s"], + projectRootPath: "/", + cachePath, + kind: "discover" + })); + const response = JSON.parse(request!); + request = undefined; + projectService.updateTypingsForProject({ + kind: "action::set", + projectName: response.projectName, + typeAcquisition: response.typeAcquisition, + compilerOptions: response.compilerOptions, + typings: emptyArray, + unresolvedImports: response.unresolvedImports, + }); + + host.checkTimeoutQueueLengthAndRun(2); + assert.isUndefined(request); + }); + it("ignores files excluded by the default type list", () => { const file1 = { path: "/a/b/f1.js", From 23ec0c44846036005035317f5e5f27421d443cb6 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 9 Oct 2018 13:45:17 -0700 Subject: [PATCH 2/2] Accept public api change --- tests/baselines/reference/api/tsserverlibrary.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 0120e49f0a..d830655998 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -8497,7 +8497,7 @@ declare namespace ts.server { private readonly throttledOperations; private readonly hostConfiguration; private safelist; - private legacySafelist; + private readonly legacySafelist; private pendingProjectUpdates; readonly currentDirectory: NormalizedPath; readonly toCanonicalFileName: (f: string) => string;