From b714f5a5a1127991cf5a7e27fc73ffd87b3a5a8a 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 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 --- .../src/ngtsc/annotations/src/component.ts | 6 ++++-- .../compiler-cli/src/ngtsc/incremental/BUILD.bazel | 1 + .../compiler-cli/src/ngtsc/incremental/src/state.ts | 13 +++++++------ .../src/ngtsc/util/src/resource_recorder.ts | 3 ++- packages/compiler-cli/src/perform_watch.ts | 10 ++++++---- packages/compiler-cli/test/perform_watch_spec.ts | 2 +- 6 files changed, 21 insertions(+), 14 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index 7706e298743cd1..e011d36fc39e72 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -251,7 +251,8 @@ export class ComponentDecoratorHandler implements const resourceUrl = this.resourceLoader.resolve(styleUrl, containingFile); const resourceStr = this.resourceLoader.load(resourceUrl); styles.push(resourceStr); - this.resourceDependencies.recordResourceDependency(node.getSourceFile(), resourceUrl); + this.resourceDependencies.recordResourceDependency( + node.getSourceFile(), absoluteFrom(resourceUrl)); } } if (component.has('styles')) { @@ -608,7 +609,8 @@ export class ComponentDecoratorHandler implements templateSourceMapping: TemplateSourceMapping } { const templateStr = this.resourceLoader.load(resourceUrl); - this.resourceDependencies.recordResourceDependency(node.getSourceFile(), resourceUrl); + this.resourceDependencies.recordResourceDependency( + node.getSourceFile(), absoluteFrom(resourceUrl)); const parseTemplate = (options?: ParseTemplateOptions) => this._parseTemplate( component, templateStr, sourceMapUrl(resourceUrl), /* templateRange */ undefined, diff --git a/packages/compiler-cli/src/ngtsc/incremental/BUILD.bazel b/packages/compiler-cli/src/ngtsc/incremental/BUILD.bazel index 9f3bc7655fbdf1..d319cf240fdf83 100644 --- a/packages/compiler-cli/src/ngtsc/incremental/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/incremental/BUILD.bazel @@ -8,6 +8,7 @@ ts_library( "src/**/*.ts", ]), deps = [ + "//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", diff --git a/packages/compiler-cli/src/ngtsc/incremental/src/state.ts b/packages/compiler-cli/src/ngtsc/incremental/src/state.ts index e8ed5ec1ffdeb0..95b40516510216 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 {DependencyTracker} from '../../partial_evaluator'; import {ResourceDependencyRecorder} from '../../util/src/resource_recorder'; @@ -53,7 +54,7 @@ export class IncrementalDriver implements DependencyTracker, ResourceDependencyR state = { kind: BuildStateKind.Pending, pendingEmit: oldDriver.state.pendingEmit, - changedResourcePaths: new Set(), + changedResourcePaths: new Set(), changedTsPaths: new Set(), }; } @@ -61,7 +62,7 @@ export class IncrementalDriver implements DependencyTracker, ResourceDependencyR // 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)); } } @@ -122,7 +123,7 @@ export class IncrementalDriver implements DependencyTracker, ResourceDependencyR const state: PendingBuildState = { kind: BuildStateKind.Pending, pendingEmit: new Set(tsFiles.map(sf => sf.fileName)), - changedResourcePaths: new Set(), + changedResourcePaths: new Set(), changedTsPaths: new Set(), }; @@ -182,7 +183,7 @@ export class IncrementalDriver implements DependencyTracker, ResourceDependencyR return Array.from(meta.fileDependencies); } - recordResourceDependency(file: ts.SourceFile, resourcePath: string): void { + recordResourceDependency(file: ts.SourceFile, resourcePath: AbsoluteFsPath): void { const metadata = this.ensureMetadata(file); metadata.resourcePaths.add(resourcePath); } @@ -211,7 +212,7 @@ export class IncrementalDriver implements DependencyTracker, ResourceDependencyR class FileMetadata { /** A set of source files that this file depends upon. */ fileDependencies = new Set(); - resourcePaths = new Set(); + resourcePaths = new Set(); } @@ -271,7 +272,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/ngtsc/util/src/resource_recorder.ts b/packages/compiler-cli/src/ngtsc/util/src/resource_recorder.ts index e0cccff28eb374..e2d46d95a0a427 100644 --- a/packages/compiler-cli/src/ngtsc/util/src/resource_recorder.ts +++ b/packages/compiler-cli/src/ngtsc/util/src/resource_recorder.ts @@ -7,12 +7,13 @@ * found in the LICENSE file at https://angular.io/license */ import * as ts from 'typescript'; +import {AbsoluteFsPath} from '../../file_system'; /** * Implement this interface to record what resources a source file depends upon. */ export interface ResourceDependencyRecorder { - recordResourceDependency(file: ts.SourceFile, resourcePath: string): void; + recordResourceDependency(file: ts.SourceFile, resourcePath: AbsoluteFsPath): void; } export class NoopResourceDependencyRecorder implements ResourceDependencyRecorder { diff --git a/packages/compiler-cli/src/perform_watch.ts b/packages/compiler-cli/src/perform_watch.ts index c9e802dc370007..2d406686ace5d9 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 479a3e388b5642..1ce31d7ac6d848 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');