Improvements to tsc --watch #17669

sheetalkamat · 2017-08-07T23:34:14Z

This PR is based off of #17269
This uses builder to emit only changed/affected files because of the change. It also caches the semantic diagnostics and updating the errors when they are part of changed/affected list

…issing host files

Also dont update filesByName array just to update missing file paths

…st it out

…leverage it

…i is accessible all the time

…neration now that api has tests

…aths

…e file as that would involve more work) so the files are picked up

…ed module resolution

…es the errors of unchanged files

ghost · 2017-08-10T17:21:48Z

src/compiler/builder.ts

+        function writeFile(fileName: string, text: string, writeByteOrderMark: boolean, _onError: (message: string) => void, sourceFiles: SourceFile[]) {
+            outputFiles.push({ name: fileName, writeByteOrderMark, text });
+            if (isDetailed) {
+                emittedSourceFiles = concatenate(emittedSourceFiles, sourceFiles);


Nit: use addRange

ghost · 2017-08-10T17:25:42Z

src/compiler/commandLineParser.ts

@@ -1979,7 +1979,7 @@ namespace ts {
     * @param host The host used to resolve files and directories.
     * @param errors An array for diagnostic reporting.
     */
-    export function getFileNamesFromConfigSpecs(spec: ConfigFileSpecs, basePath: string, options: CompilerOptions, host: ParseConfigHost, extraFileExtensions: ReadonlyArray<JsFileExtensionInfo>): ExpandResult {
+    export function getFileNamesFromConfigSpecs(spec: ConfigFileSpecs, basePath: string, options: CompilerOptions, host: ParseConfigHost, extraFileExtensions: ReadonlyArray<JsFileExtensionInfo> = []): ExpandResult {


It looks like this last argument is always provided?

ghost · 2017-08-10T17:26:58Z

src/compiler/core.ts

@@ -473,12 +473,14 @@ namespace ts {
        return result;
    }

-    export function flatMapIter<T, U>(iter: Iterator<T>, mapfn: (x: T) => U | U[] | undefined): U[] {
-        const result: U[] = [];
+    export function flatMapIter<T>(iter: Iterator<T>): T[];


No need to modify this function, arrayFrom already handles converting an iterator to an array.

ghost · 2017-08-10T17:28:45Z

src/compiler/core.ts

@@ -1211,7 +1220,7 @@ namespace ts {
    }

    /** Does nothing. */
-    export function noop(): void {}
+    export function noop(): any {}


Avoid using any. Use () => false if you need a function that always returns a falsy value.

ghost · 2017-08-10T17:29:28Z

src/compiler/core.ts

@@ -2552,4 +2561,168 @@ namespace ts {
    export function isCheckJsEnabledForFile(sourceFile: SourceFile, compilerOptions: CompilerOptions) {
        return sourceFile.checkJsDirective ? sourceFile.checkJsDirective.enabled : compilerOptions.checkJs;
    }
+
+    export interface HostForCaching {


Could this inherit from something? These signatures look familar.

ghost · 2017-08-10T17:29:43Z

src/compiler/core.ts

+        readDirectory(path: string, extensions?: ReadonlyArray<string>, exclude?: ReadonlyArray<string>, include?: ReadonlyArray<string>, depth?: number): string[];
+    }
+
+    export interface CachedHost {


Same comment as above.

ghost · 2017-08-10T18:06:43Z

src/compiler/program.ts

+    export function updateWatchingWildcardDirectories(existingWatchedForWildcards: Map<WildCardDirectoryWatchers>, wildcardDirectories: Map<WatchDirectoryFlags>,
+        watchDirectory: (directory: string, recursive: boolean) => FileWatcher,
+        closeDirectoryWatcher: (directory: string, watcher: FileWatcher, recursive: boolean, recursiveChanged: boolean) => void) {
+        return mutateExistingMap(


This would be more readable if mutateExistingMap took an object, rather than many positional arguments.

ghost · 2017-08-10T18:08:31Z

src/compiler/program.ts

@@ -473,10 +592,12 @@ namespace ts {
        }

        const filesByName = createMap<SourceFile | undefined>();
+        let missingFilePaths: Path[];


getMissingFilePaths promises to return a defined result -- I would make this a ReadonlyArray<Path> and make it emptyArray by default.

Made it readonlyArray and added assert that it is always defined

ghost · 2017-08-10T18:12:03Z

src/compiler/types.ts

@@ -3952,8 +3953,8 @@ namespace ts {
    }

    export interface CompilerHost extends ModuleResolutionHost {
-        getSourceFile(fileName: string, languageVersion: ScriptTarget, onError?: (message: string) => void): SourceFile;
-        getSourceFileByPath?(fileName: string, path: Path, languageVersion: ScriptTarget, onError?: (message: string) => void): SourceFile;
+        getSourceFile(fileName: string, languageVersion: ScriptTarget, onError?: (message: string) => void, shouldCreateNewSourceFile?: boolean): SourceFile;


Instead of having an onError callback, why not return SourceFile | Error?

This is existing public API

ghost · 2017-08-10T18:13:27Z

src/compiler/utilities.ts

@@ -1394,6 +1394,10 @@ namespace ts {
        return SpecialPropertyAssignmentKind.None;
    }

+    export function isModuleWithStringLiteralName(node: Node): node is ModuleDeclaration {


Put this near isAmbientModule since it's similar.

ghost · 2017-08-10T18:16:30Z

src/compiler/utilities.ts

+        createNewValue: (key: string, valueInNewMap: U) => T,
+        onDeleteExistingValue: (key: string, existingValue: T) => void,
+        isSameValue?: (existingValue: T, valueInNewMap: U) => boolean,
+        OnDeleteExistingMismatchValue?: (key: string, existingValue: T) => void,


Prefer lower-case variable name

ghost · 2017-08-10T18:26:30Z