From 5cada5cce106ab03bdd22f285cdddcf373a4523b Mon Sep 17 00:00:00 2001 From: JoostK Date: Sat, 23 Nov 2019 21:06:37 +0100 Subject: [PATCH] fix(ivy): recompile on template change in ngc watch mode on Windows (#34015) In #33551, a bug in `ngc --watch` mode was fixed so that a component is recompiled when its template file is changed. Due to insufficient normalization of files paths, this fix did not have the desired effect on Windows. Fixes #32869 PR Close #34015 --- .../src/ngtsc/annotations/src/component.ts | 4 ++-- .../src/ngtsc/incremental/BUILD.bazel | 2 ++ .../compiler-cli/src/ngtsc/incremental/api.ts | 3 ++- .../ngtsc/incremental/src/dependency_tracking.ts | 15 ++++++++------- .../src/ngtsc/incremental/src/state.ts | 9 +++++---- packages/compiler-cli/src/perform_watch.ts | 10 ++++++---- packages/compiler-cli/test/perform_watch_spec.ts | 2 +- 7 files changed, 26 insertions(+), 19 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index f8d6c2c757..246b1ab031 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -273,7 +273,7 @@ export class ComponentDecoratorHandler implements const resourceStr = this.resourceLoader.load(resourceUrl); styles.push(resourceStr); if (this.depTracker !== null) { - this.depTracker.addResourceDependency(node.getSourceFile(), resourceUrl); + this.depTracker.addResourceDependency(node.getSourceFile(), absoluteFrom(resourceUrl)); } } } @@ -675,7 +675,7 @@ export class ComponentDecoratorHandler implements resourceUrl: string): ParsedTemplateWithSource { const templateStr = this.resourceLoader.load(resourceUrl); if (this.depTracker !== null) { - this.depTracker.addResourceDependency(node.getSourceFile(), resourceUrl); + this.depTracker.addResourceDependency(node.getSourceFile(), absoluteFrom(resourceUrl)); } const template = this._parseTemplate( diff --git a/packages/compiler-cli/src/ngtsc/incremental/BUILD.bazel b/packages/compiler-cli/src/ngtsc/incremental/BUILD.bazel index 2c162e8c23..9894e3e96e 100644 --- a/packages/compiler-cli/src/ngtsc/incremental/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/incremental/BUILD.bazel @@ -9,6 +9,7 @@ ts_library( ]), deps = [ ":api", + "//packages/compiler-cli/src/ngtsc/file_system", "//packages/compiler-cli/src/ngtsc/imports", "//packages/compiler-cli/src/ngtsc/metadata", "//packages/compiler-cli/src/ngtsc/partial_evaluator", @@ -24,6 +25,7 @@ ts_library( name = "api", srcs = ["api.ts"], deps = [ + "//packages/compiler-cli/src/ngtsc/file_system", "@npm//typescript", ], ) diff --git a/packages/compiler-cli/src/ngtsc/incremental/api.ts b/packages/compiler-cli/src/ngtsc/incremental/api.ts index ec105ef17d..e987cd59ec 100644 --- a/packages/compiler-cli/src/ngtsc/incremental/api.ts +++ b/packages/compiler-cli/src/ngtsc/incremental/api.ts @@ -7,6 +7,7 @@ */ import * as ts from 'typescript'; +import {AbsoluteFsPath} from '../file_system'; /** * Interface of the incremental build engine. @@ -33,7 +34,7 @@ export interface DependencyTracker /** * Record that the file `from` depends on the resource file `on`. */ - addResourceDependency(from: T, on: string): void; + addResourceDependency(from: T, on: AbsoluteFsPath): void; /** * Record that the file `from` depends on the file `on` as well as `on`'s direct dependencies. diff --git a/packages/compiler-cli/src/ngtsc/incremental/src/dependency_tracking.ts b/packages/compiler-cli/src/ngtsc/incremental/src/dependency_tracking.ts index 7fc810447c..fdd2f8ce1c 100644 --- a/packages/compiler-cli/src/ngtsc/incremental/src/dependency_tracking.ts +++ b/packages/compiler-cli/src/ngtsc/incremental/src/dependency_tracking.ts @@ -8,6 +8,7 @@ import * as ts from 'typescript'; +import {AbsoluteFsPath} from '../../file_system'; import {DependencyTracker} from '../api'; /** @@ -28,7 +29,7 @@ export class FileDependencyGraph im addDependency(from: T, on: T): void { this.nodeFor(from).dependsOn.add(on.fileName); } - addResourceDependency(from: T, resource: string): void { + addResourceDependency(from: T, resource: AbsoluteFsPath): void { this.nodeFor(from).usesResources.add(resource); } @@ -50,7 +51,7 @@ export class FileDependencyGraph im } } - isStale(sf: T, changedTsPaths: Set, changedResources: Set): boolean { + isStale(sf: T, changedTsPaths: Set, changedResources: Set): boolean { return isLogicallyChanged(sf, this.nodeFor(sf), changedTsPaths, EMPTY_SET, changedResources); } @@ -77,7 +78,7 @@ export class FileDependencyGraph im */ updateWithPhysicalChanges( previous: FileDependencyGraph, changedTsPaths: Set, deletedTsPaths: Set, - changedResources: Set): Set { + changedResources: Set): Set { const logicallyChanged = new Set(); for (const sf of previous.nodes.keys()) { @@ -99,7 +100,7 @@ export class FileDependencyGraph im if (!this.nodes.has(sf)) { this.nodes.set(sf, { dependsOn: new Set(), - usesResources: new Set(), + usesResources: new Set(), }); } return this.nodes.get(sf) !; @@ -112,13 +113,13 @@ export class FileDependencyGraph im */ function isLogicallyChanged( sf: T, node: FileNode, changedTsPaths: ReadonlySet, deletedTsPaths: ReadonlySet, - changedResources: ReadonlySet): boolean { + changedResources: ReadonlySet): boolean { // A file is logically changed if it has physically changed itself (including being deleted). if (changedTsPaths.has(sf.fileName) || deletedTsPaths.has(sf.fileName)) { return true; } - // A file is logically changed if one of its dependencies has physically cxhanged. + // A file is logically changed if one of its dependencies has physically changed. for (const dep of node.dependsOn) { if (changedTsPaths.has(dep) || deletedTsPaths.has(dep)) { return true; @@ -136,7 +137,7 @@ function isLogicallyChanged( interface FileNode { dependsOn: Set; - usesResources: Set; + usesResources: Set; } const EMPTY_SET: ReadonlySet = new Set(); diff --git a/packages/compiler-cli/src/ngtsc/incremental/src/state.ts b/packages/compiler-cli/src/ngtsc/incremental/src/state.ts index f83405c1be..21f3ef680b 100644 --- a/packages/compiler-cli/src/ngtsc/incremental/src/state.ts +++ b/packages/compiler-cli/src/ngtsc/incremental/src/state.ts @@ -8,6 +8,7 @@ import * as ts from 'typescript'; +import {AbsoluteFsPath, absoluteFrom} from '../../file_system'; import {ClassRecord, TraitCompiler} from '../../transform'; import {IncrementalBuild} from '../api'; @@ -52,7 +53,7 @@ export class IncrementalDriver implements IncrementalBuild { state = { kind: BuildStateKind.Pending, pendingEmit: oldDriver.state.pendingEmit, - changedResourcePaths: new Set(), + changedResourcePaths: new Set(), changedTsPaths: new Set(), lastGood: oldDriver.state.lastGood, }; @@ -61,7 +62,7 @@ export class IncrementalDriver implements IncrementalBuild { // Merge the freshly modified resource files with any prior ones. if (modifiedResourceFiles !== null) { for (const resFile of modifiedResourceFiles) { - state.changedResourcePaths.add(resFile); + state.changedResourcePaths.add(absoluteFrom(resFile)); } } @@ -153,7 +154,7 @@ export class IncrementalDriver implements IncrementalBuild { const state: PendingBuildState = { kind: BuildStateKind.Pending, pendingEmit: new Set(tsFiles.map(sf => sf.fileName)), - changedResourcePaths: new Set(), + changedResourcePaths: new Set(), changedTsPaths: new Set(), lastGood: null, }; @@ -287,7 +288,7 @@ interface PendingBuildState extends BaseBuildState { /** * Set of resource file paths which have changed since the last successfully analyzed build. */ - changedResourcePaths: Set; + changedResourcePaths: Set; } interface AnalyzedBuildState extends BaseBuildState { diff --git a/packages/compiler-cli/src/perform_watch.ts b/packages/compiler-cli/src/perform_watch.ts index c9e802dc37..2d406686ac 100644 --- a/packages/compiler-cli/src/perform_watch.ts +++ b/packages/compiler-cli/src/perform_watch.ts @@ -246,11 +246,13 @@ export function performWatchCompilation(host: PerformWatchHost): } function watchedFileChanged(event: FileChangeEvent, fileName: string) { + const normalizedPath = path.normalize(fileName); + if (cachedOptions && event === FileChangeEvent.Change && // TODO(chuckj): validate that this is sufficient to skip files that were written. // This assumes that the file path we write is the same file path we will receive in the // change notification. - path.normalize(fileName) === path.normalize(cachedOptions.project)) { + normalizedPath === path.normalize(cachedOptions.project)) { // If the configuration file changes, forget everything and start the recompilation timer resetOptions(); } else if ( @@ -263,12 +265,12 @@ export function performWatchCompilation(host: PerformWatchHost): if (event === FileChangeEvent.CreateDeleteDir) { fileCache.clear(); } else { - fileCache.delete(path.normalize(fileName)); + fileCache.delete(normalizedPath); } - if (!ignoreFilesForWatch.has(path.normalize(fileName))) { + if (!ignoreFilesForWatch.has(normalizedPath)) { // Ignore the file if the file is one that was written by the compiler. - startTimerForRecompilation(fileName); + startTimerForRecompilation(normalizedPath); } } diff --git a/packages/compiler-cli/test/perform_watch_spec.ts b/packages/compiler-cli/test/perform_watch_spec.ts index 479a3e388b..1ce31d7ac6 100644 --- a/packages/compiler-cli/test/perform_watch_spec.ts +++ b/packages/compiler-cli/test/perform_watch_spec.ts @@ -64,7 +64,7 @@ describe('perform watch', () => { const watchResult = performWatchCompilation(host); expectNoDiagnostics(config.options, watchResult.firstCompileResult); - const htmlPath = path.posix.join(testSupport.basePath, 'src', 'main.html'); + const htmlPath = path.join(testSupport.basePath, 'src', 'main.html'); const genPath = ivyEnabled ? path.posix.join(outDir, 'src', 'main.js') : path.posix.join(outDir, 'src', 'main.ngfactory.js');