From 384aad6906d0abf6fb689ca09d082259f9b6a082 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 6 Feb 2018 10:50:32 -0800 Subject: [PATCH 1/4] Add test case for file change happening as part of file create and delete --- src/harness/unittests/tscWatchMode.ts | 32 +++++++++++++++++++++++ src/harness/virtualFileSystemWithWatch.ts | 18 ++++++++++--- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/src/harness/unittests/tscWatchMode.ts b/src/harness/unittests/tscWatchMode.ts index b6d3099dc7768..45cabc2be0e15 100644 --- a/src/harness/unittests/tscWatchMode.ts +++ b/src/harness/unittests/tscWatchMode.ts @@ -1719,6 +1719,38 @@ namespace ts.tscWatch { return [files[0]]; } }); + + it("file is deleted and created as part of change", () => { + const projectLocation = "/home/username/project"; + const file: FileOrFolder = { + path: `${projectLocation}/app/file.ts`, + content: "var a = 10;" + }; + const fileJs = `${projectLocation}/app/file.js`; + const configFile: FileOrFolder = { + path: `${projectLocation}/tsconfig.json`, + content: JSON.stringify({ + "include": [ + "app/**/*.ts" + ] + }) + }; + const files = [file, configFile, libFile]; + const host = createWatchedSystem(files, { currentDirectory: projectLocation, useCaseSensitiveFileNames: true }); + createWatchOfConfigFile("tsconfig.json", host); + verifyProgram(); + + file.content += "var b = 10;"; + + host.reloadFS(files, { invokeFileDeleteCreateAsPartInsteadOfChange: true }); + host.runQueuedTimeoutCallbacks(); + verifyProgram(); + + function verifyProgram() { + assert.isTrue(host.fileExists(fileJs)); + assert.equal(host.readFile(fileJs), file.content + "\n"); + } + }); }); describe("tsc-watch module resolution caching", () => { diff --git a/src/harness/virtualFileSystemWithWatch.ts b/src/harness/virtualFileSystemWithWatch.ts index 381917d71c7f0..8545e193ae4c1 100644 --- a/src/harness/virtualFileSystemWithWatch.ts +++ b/src/harness/virtualFileSystemWithWatch.ts @@ -246,8 +246,12 @@ interface Array {}` } export interface ReloadWatchInvokeOptions { + /** Invokes the directory watcher for the parent instead of the file changed */ invokeDirectoryWatcherInsteadOfFileChanged: boolean; + /** When new file is created, do not invoke watches for it */ ignoreWatchInvokedWithTriggerAsFileCreate: boolean; + /** Invoke the file delete, followed by create instead of file changed */ + invokeFileDeleteCreateAsPartInsteadOfChange: boolean; } export class TestServerHost implements server.ServerHost, FormatDiagnosticsHost, ModuleResolutionHost { @@ -315,12 +319,18 @@ interface Array {}` if (isString(fileOrDirectory.content)) { // Update file if (currentEntry.content !== fileOrDirectory.content) { - currentEntry.content = fileOrDirectory.content; - if (options && options.invokeDirectoryWatcherInsteadOfFileChanged) { - this.invokeDirectoryWatcher(getDirectoryPath(currentEntry.fullPath), currentEntry.fullPath); + if (options && options.invokeFileDeleteCreateAsPartInsteadOfChange) { + this.removeFileOrFolder(currentEntry, returnFalse); + this.ensureFileOrFolder(fileOrDirectory); } else { - this.invokeFileWatcher(currentEntry.fullPath, FileWatcherEventKind.Changed); + currentEntry.content = fileOrDirectory.content; + if (options && options.invokeDirectoryWatcherInsteadOfFileChanged) { + this.invokeDirectoryWatcher(getDirectoryPath(currentEntry.fullPath), currentEntry.fullPath); + } + else { + this.invokeFileWatcher(currentEntry.fullPath, FileWatcherEventKind.Changed); + } } } } From 26847845452241ec7a3365d2bec5abce4cd6052d Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 6 Feb 2018 11:47:37 -0800 Subject: [PATCH 2/4] Handle source file versioning better since the emit depends on it This ensures that when file is deleted and re-created, the file version isnt same to ensure emits correctly Fixes #21444 --- src/compiler/watch.ts | 111 +++++++++++++------------- src/harness/unittests/tscWatchMode.ts | 4 +- 2 files changed, 58 insertions(+), 57 deletions(-) diff --git a/src/compiler/watch.ts b/src/compiler/watch.ts index 08a3081056808..2c386023e8812 100644 --- a/src/compiler/watch.ts +++ b/src/compiler/watch.ts @@ -420,6 +420,9 @@ namespace ts { } } + const intialVersion = 1; + const intialVersionString = "1"; + /** * Creates the watch from the host for root files and compiler options */ @@ -429,11 +432,17 @@ namespace ts { */ export function createWatchProgram(host: WatchCompilerHostOfConfigFile): WatchOfConfigFile; export function createWatchProgram(host: WatchCompilerHostOfFilesAndCompilerOptions & WatchCompilerHostOfConfigFile): WatchOfFilesAndCompilerOptions | WatchOfConfigFile { - interface HostFileInfo { + interface FilePresentOnHost { version: number; sourceFile: SourceFile; fileWatcher: FileWatcher; } + type FileMissingOnHost = string; + interface FilePresenceUnknownOnHost { + version: number; + } + type FileMayBePresentOnHost = FilePresentOnHost | FilePresenceUnknownOnHost; + type HostFileInfo = FilePresentOnHost | FileMissingOnHost | FilePresenceUnknownOnHost; let builderProgram: T; let reloadLevel: ConfigFileProgramReloadLevel; // level to indicate if the program needs to be reloaded from config file/just filenames etc @@ -441,7 +450,7 @@ namespace ts { let watchedWildcardDirectories: Map; // map of watchers for the wild card directories in the config file let timerToUpdateProgram: any; // timer callback to recompile the program - const sourceFilesCache = createMap(); // Cache that stores the source file and version info + const sourceFilesCache = createMap(); // Cache that stores the source file and version info let missingFilePathsRequestedForRelease: Path[]; // These paths are held temparirly so that we can remove the entry from source file cache if the file is not tracked by missing files let hasChangedCompilerOptions = false; // True if the compiler options have changed between compilations let hasChangedAutomaticTypeDirectiveNames = false; // True if the automatic type directives have changed @@ -627,11 +636,20 @@ namespace ts { return ts.toPath(fileName, currentDirectory, getCanonicalFileName); } + function isFileMissingOnHost(hostSourceFile: HostFileInfo): hostSourceFile is FileMissingOnHost { + return isString(hostSourceFile); + } + + function isFilePresentOnHost(hostSourceFile: FileMayBePresentOnHost): hostSourceFile is FilePresentOnHost { + return !!(hostSourceFile as FilePresentOnHost).sourceFile; + } + function fileExists(fileName: string) { const path = toPath(fileName); - const hostSourceFileInfo = sourceFilesCache.get(path); - if (hostSourceFileInfo !== undefined) { - return !isString(hostSourceFileInfo); + // If file is missing on host from cache, we can definitely say file doesnt exist + // otherwise we need to ensure from the disk + if (isFileMissingOnHost(sourceFilesCache.get(path))) { + return true; } return directoryStructureHost.fileExists(fileName); @@ -640,39 +658,42 @@ namespace ts { function getVersionedSourceFileByPath(fileName: string, path: Path, languageVersion: ScriptTarget, onError?: (message: string) => void, shouldCreateNewSourceFile?: boolean): SourceFile { const hostSourceFile = sourceFilesCache.get(path); // No source file on the host - if (isString(hostSourceFile)) { + if (isFileMissingOnHost(hostSourceFile)) { return undefined; } // Create new source file if requested or the versions dont match - if (!hostSourceFile || shouldCreateNewSourceFile || hostSourceFile.version.toString() !== hostSourceFile.sourceFile.version) { + if (!hostSourceFile || shouldCreateNewSourceFile || !isFilePresentOnHost(hostSourceFile) || hostSourceFile.version.toString() !== hostSourceFile.sourceFile.version) { const sourceFile = getNewSourceFile(); if (hostSourceFile) { if (shouldCreateNewSourceFile) { hostSourceFile.version++; } + if (sourceFile) { - hostSourceFile.sourceFile = sourceFile; + // Set the source file and create file watcher now that file was present on the disk + (hostSourceFile as FilePresentOnHost).sourceFile = sourceFile; sourceFile.version = hostSourceFile.version.toString(); - if (!hostSourceFile.fileWatcher) { - hostSourceFile.fileWatcher = watchFilePath(host, fileName, onSourceFileChange, path, writeLog); + if (!(hostSourceFile as FilePresentOnHost).fileWatcher) { + (hostSourceFile as FilePresentOnHost).fileWatcher = watchFilePath(host, fileName, onSourceFileChange, path, writeLog); } } else { // There is no source file on host any more, close the watch, missing file paths will track it - hostSourceFile.fileWatcher.close(); + if (isFilePresentOnHost(hostSourceFile)) { + hostSourceFile.fileWatcher.close(); + } sourceFilesCache.set(path, hostSourceFile.version.toString()); } } else { - let fileWatcher: FileWatcher; if (sourceFile) { - sourceFile.version = "1"; - fileWatcher = watchFilePath(host, fileName, onSourceFileChange, path, writeLog); - sourceFilesCache.set(path, { sourceFile, version: 1, fileWatcher }); + sourceFile.version = intialVersionString; + const fileWatcher = watchFilePath(host, fileName, onSourceFileChange, path, writeLog); + sourceFilesCache.set(path, { sourceFile, version: intialVersion, fileWatcher }); } else { - sourceFilesCache.set(path, "0"); + sourceFilesCache.set(path, intialVersionString); } } return sourceFile; @@ -697,20 +718,22 @@ namespace ts { } } - function removeSourceFile(path: Path) { + function nextSourceFileVersion(path: Path) { const hostSourceFile = sourceFilesCache.get(path); if (hostSourceFile !== undefined) { - if (!isString(hostSourceFile)) { - hostSourceFile.fileWatcher.close(); - resolutionCache.invalidateResolutionOfFile(path); + if (isFileMissingOnHost(hostSourceFile)) { + // The next version, lets set it as presence unknown file + sourceFilesCache.set(path, { version: Number(hostSourceFile) + 1 }); + } + else { + hostSourceFile.version++; } - sourceFilesCache.delete(path); } } function getSourceVersion(path: Path): string { const hostSourceFile = sourceFilesCache.get(path); - return !hostSourceFile || isString(hostSourceFile) ? undefined : hostSourceFile.version.toString(); + return !hostSourceFile || isFileMissingOnHost(hostSourceFile) ? undefined : hostSourceFile.version.toString(); } function onReleaseOldSourceFile(oldSourceFile: SourceFile, _oldOptions: CompilerOptions) { @@ -721,10 +744,10 @@ namespace ts { // there was version update and new source file was created. if (hostSourceFileInfo) { // record the missing file paths so they can be removed later if watchers arent tracking them - if (isString(hostSourceFileInfo)) { + if (isFileMissingOnHost(hostSourceFileInfo)) { (missingFilePathsRequestedForRelease || (missingFilePathsRequestedForRelease = [])).push(oldSourceFile.path); } - else if (hostSourceFileInfo.sourceFile === oldSourceFile) { + else if ((hostSourceFileInfo as FilePresentOnHost).sourceFile === oldSourceFile) { sourceFilesCache.delete(oldSourceFile.path); resolutionCache.removeResolutionsOfFile(oldSourceFile.path); } @@ -808,27 +831,12 @@ namespace ts { function onSourceFileChange(fileName: string, eventKind: FileWatcherEventKind, path: Path) { updateCachedSystemWithFile(fileName, path, eventKind); - const hostSourceFile = sourceFilesCache.get(path); - if (hostSourceFile) { - // Update the cache - if (eventKind === FileWatcherEventKind.Deleted) { - resolutionCache.invalidateResolutionOfFile(path); - if (!isString(hostSourceFile)) { - hostSourceFile.fileWatcher.close(); - sourceFilesCache.set(path, (++hostSourceFile.version).toString()); - } - } - else { - // Deleted file created - if (isString(hostSourceFile)) { - sourceFilesCache.delete(path); - } - else { - // file changed - just update the version - hostSourceFile.version++; - } - } + + // Update the source file cache + if (eventKind === FileWatcherEventKind.Deleted && sourceFilesCache.get(path)) { + resolutionCache.invalidateResolutionOfFile(path); } + nextSourceFileVersion(path); // Update the program scheduleProgramUpdate(); @@ -856,7 +864,7 @@ namespace ts { missingFilesMap.delete(missingFilePath); // Delete the entry in the source files cache so that new source file is created - removeSourceFile(missingFilePath); + nextSourceFileVersion(missingFilePath); // When a missing file is created, we should update the graph. scheduleProgramUpdate(); @@ -885,17 +893,10 @@ namespace ts { const fileOrDirectoryPath = toPath(fileOrDirectory); // Since the file existance changed, update the sourceFiles cache - const result = cachedDirectoryStructureHost && cachedDirectoryStructureHost.addOrDeleteFileOrDirectory(fileOrDirectory, fileOrDirectoryPath); - - // Instead of deleting the file, mark it as changed instead - // Many times node calls add/remove/file when watching directories recursively - const hostSourceFile = sourceFilesCache.get(fileOrDirectoryPath); - if (hostSourceFile && !isString(hostSourceFile) && (result ? result.fileExists : directoryStructureHost.fileExists(fileOrDirectory))) { - hostSourceFile.version++; - } - else { - removeSourceFile(fileOrDirectoryPath); + if (cachedDirectoryStructureHost) { + cachedDirectoryStructureHost.addOrDeleteFileOrDirectory(fileOrDirectory, fileOrDirectoryPath); } + nextSourceFileVersion(fileOrDirectoryPath); // If the the added or created file or directory is not supported file name, ignore the file // But when watched directory is added/removed, we need to reload the file list diff --git a/src/harness/unittests/tscWatchMode.ts b/src/harness/unittests/tscWatchMode.ts index 45cabc2be0e15..b7a477b04bf1f 100644 --- a/src/harness/unittests/tscWatchMode.ts +++ b/src/harness/unittests/tscWatchMode.ts @@ -1730,7 +1730,7 @@ namespace ts.tscWatch { const configFile: FileOrFolder = { path: `${projectLocation}/tsconfig.json`, content: JSON.stringify({ - "include": [ + include: [ "app/**/*.ts" ] }) @@ -1740,7 +1740,7 @@ namespace ts.tscWatch { createWatchOfConfigFile("tsconfig.json", host); verifyProgram(); - file.content += "var b = 10;"; + file.content += "\nvar b = 10;"; host.reloadFS(files, { invokeFileDeleteCreateAsPartInsteadOfChange: true }); host.runQueuedTimeoutCallbacks(); From 22c0444814f7a4547e0cdd41b3a0b51d37364310 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 6 Feb 2018 13:48:09 -0800 Subject: [PATCH 3/4] Missing files are versions are stored as number --- src/compiler/watch.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/compiler/watch.ts b/src/compiler/watch.ts index 2c386023e8812..2f03e5bc5d4bd 100644 --- a/src/compiler/watch.ts +++ b/src/compiler/watch.ts @@ -420,8 +420,7 @@ namespace ts { } } - const intialVersion = 1; - const intialVersionString = "1"; + const initialVersion = 1; /** * Creates the watch from the host for root files and compiler options @@ -437,7 +436,7 @@ namespace ts { sourceFile: SourceFile; fileWatcher: FileWatcher; } - type FileMissingOnHost = string; + type FileMissingOnHost = number; interface FilePresenceUnknownOnHost { version: number; } @@ -637,7 +636,7 @@ namespace ts { } function isFileMissingOnHost(hostSourceFile: HostFileInfo): hostSourceFile is FileMissingOnHost { - return isString(hostSourceFile); + return typeof hostSourceFile === "number"; } function isFilePresentOnHost(hostSourceFile: FileMayBePresentOnHost): hostSourceFile is FilePresentOnHost { @@ -683,17 +682,17 @@ namespace ts { if (isFilePresentOnHost(hostSourceFile)) { hostSourceFile.fileWatcher.close(); } - sourceFilesCache.set(path, hostSourceFile.version.toString()); + sourceFilesCache.set(path, hostSourceFile.version); } } else { if (sourceFile) { - sourceFile.version = intialVersionString; + sourceFile.version = initialVersion.toString(); const fileWatcher = watchFilePath(host, fileName, onSourceFileChange, path, writeLog); - sourceFilesCache.set(path, { sourceFile, version: intialVersion, fileWatcher }); + sourceFilesCache.set(path, { sourceFile, version: initialVersion, fileWatcher }); } else { - sourceFilesCache.set(path, intialVersionString); + sourceFilesCache.set(path, initialVersion); } } return sourceFile; From 82aa1fbbe4e38896f4d4f430b3326544537dbf90 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 7 Feb 2018 09:21:38 -0800 Subject: [PATCH 4/4] Get new line before writing log to log correct output --- src/compiler/watch.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/compiler/watch.ts b/src/compiler/watch.ts index 2f03e5bc5d4bd..7a98c43dd04a7 100644 --- a/src/compiler/watch.ts +++ b/src/compiler/watch.ts @@ -488,14 +488,14 @@ namespace ts { const watchFilePath = compilerOptions.extendedDiagnostics ? ts.addFilePathWatcherWithLogging : ts.addFilePathWatcher; const watchDirectoryWorker = compilerOptions.extendedDiagnostics ? ts.addDirectoryWatcherWithLogging : ts.addDirectoryWatcher; + const getCanonicalFileName = createGetCanonicalFileName(useCaseSensitiveFileNames); + let newLine = updateNewLine(); + writeLog(`Current directory: ${currentDirectory} CaseSensitiveFileNames: ${useCaseSensitiveFileNames}`); if (configFileName) { watchFile(host, configFileName, scheduleProgramReload, writeLog); } - const getCanonicalFileName = createGetCanonicalFileName(useCaseSensitiveFileNames); - let newLine = updateNewLine(); - const compilerHost: CompilerHost & ResolutionCacheHost = { // Members for CompilerHost getSourceFile: (fileName, languageVersion, onError?, shouldCreateNewSourceFile?) => getVersionedSourceFileByPath(fileName, toPath(fileName), languageVersion, onError, shouldCreateNewSourceFile), @@ -583,7 +583,9 @@ namespace ts { // Compile the program if (loggingEnabled) { - writeLog(`CreatingProgramWith::\n roots: ${JSON.stringify(rootFileNames)}\n options: ${JSON.stringify(compilerOptions)}`); + writeLog(`CreatingProgramWith::`); + writeLog(` roots: ${JSON.stringify(rootFileNames)}`); + writeLog(` options: ${JSON.stringify(compilerOptions)}`); } const needsUpdateInTypeRootWatch = hasChangedCompilerOptions || !program;