From 3dc0d5a77c950e4757c17fd4a17959bb5f36b495 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 5 Dec 2018 12:18:14 -0800 Subject: [PATCH] Watch missing map file and update the source mapping accordingly --- src/server/editorServices.ts | 150 +++++++++++++----- src/server/scriptInfo.ts | 18 ++- src/services/sourcemaps.ts | 5 +- .../unittests/tsserverProjectSystem.ts | 117 +++++++++++--- 4 files changed, 224 insertions(+), 66 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index a3ce0989b0c..b970a8fc716 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -342,6 +342,7 @@ namespace ts.server { FailedLookupLocation = "Directory of Failed lookup locations in module resolution", TypeRoots = "Type root directory", NodeModulesForClosedScriptInfo = "node_modules for closed script infos in them", + MissingSourceMapFile = "Missing source map file" } const enum ConfigFileWatcherStatus { @@ -953,22 +954,25 @@ namespace ts.server { private handleSourceMapProjects(info: ScriptInfo) { // Change in d.ts, update source projects as well if (info.sourceMapFilePath) { - const sourceMapFileInfo = this.getScriptInfoForPath(info.sourceMapFilePath); - if (sourceMapFileInfo && sourceMapFileInfo.sourceInfos) { - this.delayUpdateSourceInfoProjects(sourceMapFileInfo.sourceInfos); + if (isString(info.sourceMapFilePath)) { + const sourceMapFileInfo = this.getScriptInfoForPath(info.sourceMapFilePath); + this.delayUpdateSourceInfoProjects(sourceMapFileInfo && sourceMapFileInfo.sourceInfos); + } + else { + this.delayUpdateSourceInfoProjects(info.sourceMapFilePath.sourceInfos); } } // Change in mapInfo, update declarationProjects and source projects - if (info.sourceInfos) { - this.delayUpdateSourceInfoProjects(info.sourceInfos); - } + this.delayUpdateSourceInfoProjects(info.sourceInfos); if (info.declarationInfoPath) { this.delayUpdateProjectsOfScriptInfoPath(info.declarationInfoPath); } } - private delayUpdateSourceInfoProjects(sourceInfos: Map) { - sourceInfos.forEach((_value, path) => this.delayUpdateProjectsOfScriptInfoPath(path as Path)); + private delayUpdateSourceInfoProjects(sourceInfos: Map | undefined) { + if (sourceInfos) { + sourceInfos.forEach((_value, path) => this.delayUpdateProjectsOfScriptInfoPath(path as Path)); + } } private delayUpdateProjectsOfScriptInfoPath(path: Path) { @@ -992,6 +996,14 @@ namespace ts.server { // update projects to make sure that set of referenced files is correct this.delayUpdateProjectGraphs(containingProjects); this.handleSourceMapProjects(info); + info.closeSourceMapFileWatcher(); + // need to recalculate source map from declaration file + if (info.declarationInfoPath) { + const declarationInfo = this.getScriptInfoForPath(info.declarationInfoPath); + if (declarationInfo) { + declarationInfo.sourceMapFilePath = undefined; + } + } } } @@ -2228,32 +2240,43 @@ namespace ts.server { /*@internal*/ getDocumentPositionMapper(project: Project, generatedFileName: string, sourceFileName?: string): DocumentPositionMapper | undefined { - const declarationInfo = this.getOrCreateScriptInfoNotOpenedByClient(generatedFileName, project.currentDirectory, project.directoryStructureHost); + // Since declaration info and map file watches arent updating project's directory structure host (which can cache file structure) use host + const declarationInfo = this.getOrCreateScriptInfoNotOpenedByClient(generatedFileName, project.currentDirectory, this.host); if (!declarationInfo) return undefined; // Try to get from cache declarationInfo.getSnapshot(); // Ensure synchronized - if (declarationInfo.sourceMapFilePath !== undefined) { - // Doesnt have sourceMap - if (!declarationInfo.sourceMapFilePath) return undefined; - + if (isString(declarationInfo.sourceMapFilePath)) { // Ensure mapper is synchronized - const mapFileInfo = this.getScriptInfoForPath(declarationInfo.sourceMapFilePath); - if (mapFileInfo) { - mapFileInfo.getSnapshot(); - if (mapFileInfo.documentPositionMapper !== undefined) { - return mapFileInfo.documentPositionMapper ? mapFileInfo.documentPositionMapper : undefined; + const sourceMapFileInfo = this.getScriptInfoForPath(declarationInfo.sourceMapFilePath); + if (sourceMapFileInfo) { + sourceMapFileInfo.getSnapshot(); + if (sourceMapFileInfo.documentPositionMapper !== undefined) { + sourceMapFileInfo.sourceInfos = this.addSourceInfoToSourceMap(sourceFileName, project, sourceMapFileInfo.sourceInfos); + return sourceMapFileInfo.documentPositionMapper ? sourceMapFileInfo.documentPositionMapper : undefined; } } + declarationInfo.sourceMapFilePath = undefined; + } + else if (declarationInfo.sourceMapFilePath) { + declarationInfo.sourceMapFilePath.sourceInfos = this.addSourceInfoToSourceMap(sourceFileName, project, declarationInfo.sourceMapFilePath.sourceInfos); + return undefined; + } + else if (declarationInfo.sourceMapFilePath !== undefined) { + // Doesnt have sourceMap + return undefined; } // Create the mapper - declarationInfo.sourceMapFilePath = undefined; let sourceMapFileInfo: ScriptInfo | undefined; + let mapFileNameFromDeclarationInfo: string | undefined; - let readMapFile: ReadMapFile | undefined = mapFileName => { - const mapInfo = this.getOrCreateScriptInfoNotOpenedByClient(mapFileName, project.currentDirectory, project.directoryStructureHost); - if (!mapInfo) return undefined; + let readMapFile: ReadMapFile | undefined = (mapFileName, mapFileNameFromDts) => { + const mapInfo = this.getOrCreateScriptInfoNotOpenedByClient(mapFileName, project.currentDirectory, this.host); + if (!mapInfo) { + mapFileNameFromDeclarationInfo = mapFileNameFromDts; + return undefined; + } sourceMapFileInfo = mapInfo; const snap = mapInfo.getSnapshot(); if (mapInfo.documentPositionMapper !== undefined) return mapInfo.documentPositionMapper; @@ -2271,11 +2294,19 @@ namespace ts.server { declarationInfo.sourceMapFilePath = sourceMapFileInfo.path; sourceMapFileInfo.declarationInfoPath = declarationInfo.path; sourceMapFileInfo.documentPositionMapper = documentPositionMapper || false; - if (sourceFileName && documentPositionMapper) { - // Attach as source - const sourceInfo = this.getOrCreateScriptInfoNotOpenedByClient(sourceFileName, project.currentDirectory, project.directoryStructureHost)!; - (sourceMapFileInfo.sourceInfos || (sourceMapFileInfo.sourceInfos = createMap())).set(sourceInfo.path, true); - } + sourceMapFileInfo.sourceInfos = this.addSourceInfoToSourceMap(sourceFileName, project, sourceMapFileInfo.sourceInfos); + } + else if (mapFileNameFromDeclarationInfo) { + declarationInfo.sourceMapFilePath = { + declarationInfoPath: declarationInfo.path, + watcher: this.addMissingSourceMapFile( + project.currentDirectory === this.currentDirectory ? + mapFileNameFromDeclarationInfo : + getNormalizedAbsolutePath(mapFileNameFromDeclarationInfo, project.currentDirectory), + declarationInfo.path + ), + sourceInfos: this.addSourceInfoToSourceMap(sourceFileName, project) + }; } else { declarationInfo.sourceMapFilePath = false; @@ -2283,6 +2314,34 @@ namespace ts.server { return documentPositionMapper; } + private addSourceInfoToSourceMap(sourceFileName: string | undefined, project: Project, sourceInfos?: Map) { + if (sourceFileName) { + // Attach as source + const sourceInfo = this.getOrCreateScriptInfoNotOpenedByClient(sourceFileName, project.currentDirectory, project.directoryStructureHost)!; + (sourceInfos || (sourceInfos = createMap())).set(sourceInfo.path, true); + } + return sourceInfos; + } + + private addMissingSourceMapFile(mapFileName: string, declarationInfoPath: Path) { + const fileWatcher = this.watchFactory.watchFile( + this.host, + mapFileName, + () => { + const declarationInfo = this.getScriptInfoForPath(declarationInfoPath); + if (declarationInfo && declarationInfo.sourceMapFilePath && !isString(declarationInfo.sourceMapFilePath)) { + // Update declaration and source projects + this.delayUpdateProjectGraphs(declarationInfo.containingProjects); + this.delayUpdateSourceInfoProjects(declarationInfo.sourceMapFilePath.sourceInfos); + declarationInfo.closeSourceMapFileWatcher(); + } + }, + PollingInterval.High, + WatchType.MissingSourceMapFile, + ); + return fileWatcher; + } + /*@internal*/ getSourceFileLike(fileName: string, projectNameOrProject: string | Project, declarationInfo?: ScriptInfo) { const project = (projectNameOrProject as Project).projectName ? projectNameOrProject as Project : this.findProject(projectNameOrProject as string); @@ -2297,7 +2356,7 @@ namespace ts.server { if (!info) return undefined; // Attach as source - if (declarationInfo && declarationInfo.sourceMapFilePath && info !== declarationInfo) { + if (declarationInfo && isString(declarationInfo.sourceMapFilePath) && info !== declarationInfo) { const sourceMapInfo = this.getScriptInfoForPath(declarationInfo.sourceMapFilePath); if (sourceMapInfo) { (sourceMapInfo.sourceInfos || (sourceMapInfo.sourceInfos = createMap())).set(info.path, true); @@ -2669,11 +2728,18 @@ namespace ts.server { if (!info.isScriptOpen() && info.isOrphan()) { // Otherwise if there is any source info that is alive, this alive too if (!info.sourceMapFilePath) return; - const sourceMapInfo = this.getScriptInfoForPath(info.sourceMapFilePath); - if (!sourceMapInfo || !sourceMapInfo.sourceInfos) return; - if (!forEachKey(sourceMapInfo.sourceInfos, path => { - const info = this.getScriptInfoForPath(path as Path)!; - return info.isScriptOpen() || !info.isOrphan(); + let sourceInfos: Map | undefined; + if (isString(info.sourceMapFilePath)) { + const sourceMapInfo = this.getScriptInfoForPath(info.sourceMapFilePath); + sourceInfos = sourceMapInfo && sourceMapInfo.sourceInfos; + } + else { + sourceInfos = info.sourceMapFilePath.sourceInfos; + } + if (!sourceInfos) return; + if (!forEachKey(sourceInfos, path => { + const info = this.getScriptInfoForPath(path as Path); + return !!info && (info.isScriptOpen() || !info.isOrphan()); })) { return; } @@ -2682,11 +2748,18 @@ namespace ts.server { // Retain this script info toRemoveScriptInfos.delete(info.path); if (info.sourceMapFilePath) { - // And map file info and source infos - toRemoveScriptInfos.delete(info.sourceMapFilePath); - const sourceMapInfo = this.getScriptInfoForPath(info.sourceMapFilePath); - if (sourceMapInfo && sourceMapInfo.sourceInfos) { - sourceMapInfo.sourceInfos.forEach((_value, path) => toRemoveScriptInfos.delete(path)); + let sourceInfos: Map | undefined; + if (isString(info.sourceMapFilePath)) { + // And map file info and source infos + toRemoveScriptInfos.delete(info.sourceMapFilePath); + const sourceMapInfo = this.getScriptInfoForPath(info.sourceMapFilePath); + sourceInfos = sourceMapInfo && sourceMapInfo.sourceInfos; + } + else { + sourceInfos = info.sourceMapFilePath.sourceInfos; + } + if (sourceInfos) { + sourceInfos.forEach((_value, path) => toRemoveScriptInfos.delete(path)); } } }); @@ -2695,6 +2768,7 @@ namespace ts.server { // if there are not projects that include this script info - delete it this.stopWatchingScriptInfo(info); this.deleteScriptInfo(info); + info.closeSourceMapFileWatcher(); }); } diff --git a/src/server/scriptInfo.ts b/src/server/scriptInfo.ts index 1cd00cdca45..21c011d7261 100644 --- a/src/server/scriptInfo.ts +++ b/src/server/scriptInfo.ts @@ -66,6 +66,7 @@ namespace ts.server { private resetSourceMapInfo() { this.info.sourceFileLike = undefined; + this.info.closeSourceMapFileWatcher(); this.info.sourceMapFilePath = undefined; this.info.declarationInfoPath = undefined; this.info.sourceInfos = undefined; @@ -280,6 +281,13 @@ namespace ts.server { sourceFile: SourceFile; } + /*@internal*/ + export interface SourceMapFileWatcher { + declarationInfoPath: Path; + watcher: FileWatcher; + sourceInfos?: Map; + } + export class ScriptInfo { /** * All projects that include this file @@ -309,7 +317,7 @@ namespace ts.server { sourceFileLike?: SourceFileLike; /*@internal*/ - sourceMapFilePath?: Path | false; + sourceMapFilePath?: Path | SourceMapFileWatcher | false; // Present on sourceMapFile info /*@internal*/ @@ -606,5 +614,13 @@ namespace ts.server { getLineInfo(): LineInfo { return this.textStorage.getLineInfo(); } + + /*@internal*/ + closeSourceMapFileWatcher() { + if (this.sourceMapFilePath && !isString(this.sourceMapFilePath)) { + closeFileWatcherOf(this.sourceMapFilePath); + this.sourceMapFilePath = undefined; + } + } } } diff --git a/src/services/sourcemaps.ts b/src/services/sourcemaps.ts index 42b00b8a8d7..d07c21a9f43 100644 --- a/src/services/sourcemaps.ts +++ b/src/services/sourcemaps.ts @@ -131,7 +131,7 @@ namespace ts { * string | undefined to contents of map file to create DocumentPositionMapper from it * DocumentPositionMapper | false to give back cached DocumentPositionMapper */ - export type ReadMapFile = (mapFileName: string) => string | undefined | DocumentPositionMapper | false; + export type ReadMapFile = (mapFileName: string, mapFileNameFromDts: string | undefined) => string | undefined | DocumentPositionMapper | false; export function getDocumentPositionMapper( host: DocumentPositionMapperHost, @@ -155,9 +155,10 @@ namespace ts { possibleMapLocations.push(mapFileName); } possibleMapLocations.push(generatedFileName + ".map"); + const originalMapFileName = mapFileName && getNormalizedAbsolutePath(mapFileName, getDirectoryPath(generatedFileName)); for (const location of possibleMapLocations) { const mapFileName = getNormalizedAbsolutePath(location, getDirectoryPath(generatedFileName)); - const mapFileContents = readMapFile(mapFileName); + const mapFileContents = readMapFile(mapFileName, originalMapFileName); if (isString(mapFileContents)) { return convertDocumentToSourceMapper(host, mapFileContents, mapFileName); } diff --git a/src/testRunner/unittests/tsserverProjectSystem.ts b/src/testRunner/unittests/tsserverProjectSystem.ts index 077f0128adc..b5909d3b3d5 100644 --- a/src/testRunner/unittests/tsserverProjectSystem.ts +++ b/src/testRunner/unittests/tsserverProjectSystem.ts @@ -10806,8 +10806,11 @@ fn5(); const configFiles = openFiles.map(openFile => `${getDirectoryPath(openFile.path)}/tsconfig.json`); const openInfos = openFiles.map(f => f.path); const otherWatchedFiles = configFiles; - function openTsFile() { + function openTsFile(onHostCreate?: (host: TestServerHost) => void) { const host = createHost(files, [mainConfig.path]); + if (onHostCreate) { + onHostCreate(host); + } const session = createSession(host); openFilesForSession([...openFiles, randomFile], session); return { host, session }; @@ -10821,8 +10824,19 @@ fn5(); }); } - function verifyInfos(session: TestSession, host: TestServerHost, minusDtsMapAndSource?: true) { - verifyInfosWithRandom(session, host, openInfos, minusDtsMapAndSource ? closedInfos.filter(f => f !== dependencyTs.path && f.toLowerCase() !== dtsMapPath) : closedInfos, otherWatchedFiles); + function verifyInfos(session: TestSession, host: TestServerHost) { + verifyInfosWithRandom(session, host, openInfos, closedInfos, otherWatchedFiles); + } + + function verifyInfosWhenNoMapFile(session: TestSession, host: TestServerHost, dependencyTsOK?: true) { + const dtsMapClosedInfo = firstDefined(closedInfos, f => f.toLowerCase() === dtsMapPath ? f : undefined); + verifyInfosWithRandom( + session, + host, + openInfos, + closedInfos.filter(f => f !== dtsMapClosedInfo && (dependencyTsOK || f !== dependencyTs.path)), + dtsMapClosedInfo ? otherWatchedFiles.concat(dtsMapClosedInfo) : otherWatchedFiles + ); } function verifyDocumentPositionMapper(session: TestSession, dependencyMap: server.ScriptInfo, documentPositionMapper: server.ScriptInfo["documentPositionMapper"], notEqual?: true) { @@ -10841,14 +10855,20 @@ fn5(); return { response, expectedResponse, expectedNoMapResponse }; } - function verifyAllFnActionWorker(session: TestSession, verifyAction: (result: ReturnType, dtsInfo: server.ScriptInfo) => void) { + function firstAction(session: TestSession) { + actionGetters.forEach(actionGetter => action(actionGetter, 1, session)); + } + + function verifyAllFnActionWorker(session: TestSession, verifyAction: (result: ReturnType, dtsInfo: server.ScriptInfo, isFirst: boolean) => void) { // action + let isFirst = true; for (const actionGetter of actionGetters) { for (let fn = 1; fn <= 5; fn++) { const result = action(actionGetter, fn, session); const dtsInfo = session.getProjectService().filenameToScriptInfo.get(dtsPath); assert.isDefined(dtsInfo); - verifyAction(result, dtsInfo!); + verifyAction(result, dtsInfo!, isFirst); + isFirst = false; } } } @@ -10861,13 +10881,11 @@ fn5(); documentPositionMapper?: server.ScriptInfo["documentPositionMapper"] ) { // action - let isFirst = true; - verifyAllFnActionWorker(session, ({ response, expectedResponse }, dtsInfo) => { + verifyAllFnActionWorker(session, ({ response, expectedResponse }, dtsInfo, isFirst) => { assert.deepEqual(response, expectedResponse); verifyInfos(session, host); assert.equal(dtsInfo.sourceMapFilePath, dtsMapPath); if (isFirst) { - isFirst = false; if (dependencyMap) { verifyDocumentPositionMapper(session, dependencyMap, documentPositionMapper, firstDocumentPositionMapperNotEquals); documentPositionMapper = dependencyMap.documentPositionMapper; @@ -10886,15 +10904,26 @@ fn5(); function verifyAllFnActionWithNoMap( session: TestSession, - host: TestServerHost + host: TestServerHost, + dependencyTsOK?: true ) { + let sourceMapFilePath: server.ScriptInfo["sourceMapFilePath"]; // action - verifyAllFnActionWorker(session, ({ response, expectedResponse, expectedNoMapResponse }, dtsInfo) => { + verifyAllFnActionWorker(session, ({ response, expectedResponse, expectedNoMapResponse }, dtsInfo, isFirst) => { assert.deepEqual(response, expectedNoMapResponse && verifier.length ? expectedNoMapResponse : expectedResponse); - verifyInfos(session, host, /*minusDtsMapAndSource*/ true); - assert.isFalse(dtsInfo.sourceMapFilePath); + verifyInfosWhenNoMapFile(session, host, dependencyTsOK); assert.isUndefined(session.getProjectService().filenameToScriptInfo.get(dtsMapPath)); + if (isFirst) { + assert.isNotString(dtsInfo.sourceMapFilePath); + assert.isNotFalse(dtsInfo.sourceMapFilePath); + assert.isDefined(dtsInfo.sourceMapFilePath); + sourceMapFilePath = dtsInfo.sourceMapFilePath; + } + else { + assert.equal(dtsInfo.sourceMapFilePath, sourceMapFilePath); + } }); + return sourceMapFilePath; } function verifyScenarioWithChangesWorker( @@ -10905,7 +10934,7 @@ fn5(); const { host, session } = openTsFile(); // Create DocumentPositionMapper - actionGetters.forEach(actionGetter => action(actionGetter, 1, session)); + firstAction(session); const dependencyMap = session.getProjectService().filenameToScriptInfo.get(dtsMapPath)!; const documentPositionMapper = dependencyMap.documentPositionMapper; @@ -10934,10 +10963,7 @@ fn5(); }); } - it(mainScenario, () => { - const { host, session } = openTsFile(); - checkProject(session); - + function verifyMainScenarioAndScriptInfoCollection(session: TestSession, host: TestServerHost) { // Main scenario action const { dependencyMap, documentPositionMapper } = verifyAllFnAction(session, host); checkProject(session); @@ -10953,6 +10979,28 @@ fn5(); closeFilesForSession([...openFiles, randomFile], session); openFilesForSession([randomFile], session); verifyOnlyRandomInfos(session, host); + } + + function verifyMainScenarioAndScriptInfoCollectionWithNoMap(session: TestSession, host: TestServerHost, dependencyTsOKInScenario?: true) { + // Main scenario action + verifyAllFnActionWithNoMap(session, host, dependencyTsOKInScenario); + + // Collecting at this point retains dependency.d.ts and map watcher + closeFilesForSession([randomFile], session); + openFilesForSession([randomFile], session); + verifyInfosWhenNoMapFile(session, host); + + // Closing open file, removes dependencies too + closeFilesForSession([...openFiles, randomFile], session); + openFilesForSession([randomFile], session); + verifyOnlyRandomInfos(session, host); + } + + it(mainScenario, () => { + const { host, session } = openTsFile(); + checkProject(session); + + verifyMainScenarioAndScriptInfoCollection(session, host); }); describe("when usage file changes, document position mapper doesnt change", () => { @@ -10963,7 +11011,7 @@ fn5(); }))); }); - describe("when dependency file changes, document position mapper doesnt change", () => { + describe("when dependency .d.ts changes, document position mapper doesnt change", () => { // Edit dts to add new fn verifyScenarioWithChanges(host => host.writeFile( dtsLocation, @@ -10983,15 +11031,34 @@ fn5(); ), /*afterActionDocumentPositionMapperNotEquals*/ true); }); - it("when map file is not present", () => { - const host = createHost(files, [mainConfig.path]); - host.deleteFile(dtsMapLocation); + describe("when map file is not present", () => { + it(mainScenario, () => { + const { host, session } = openTsFile(host => host.deleteFile(dtsMapLocation)); + checkProject(session); - const session = createSession(host); - openFilesForSession([...openFiles, randomFile], session); - checkProject(session); + verifyMainScenarioAndScriptInfoCollectionWithNoMap(session, host); + }); - verifyAllFnActionWithNoMap(session, host); + it("when map file is created", () => { + let dtsMapContents: string | undefined; + const { host, session } = openTsFile(host => { + dtsMapContents = host.readFile(dtsMapLocation)!; + host.deleteFile(dtsMapLocation); + }); + firstAction(session); + + host.writeFile(dtsMapLocation, dtsMapContents!); + verifyMainScenarioAndScriptInfoCollection(session, host); + }); + + it("when map file is deleted", () => { + const { host, session } = openTsFile(); + firstAction(session); + + // The dependency file is deleted when orphan files are collected + host.deleteFile(dtsMapLocation); + verifyMainScenarioAndScriptInfoCollectionWithNoMap(session, host, /*dependencyTsOKInScenario*/ true); + }); }); }