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
This commit is contained in:
JoostK 2019-11-23 21:06:37 +01:00 committed by Miško Hevery
parent f7d4cc9cab
commit 5cada5cce1
7 changed files with 26 additions and 19 deletions

View File

@ -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(

View File

@ -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",
],
)

View File

@ -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<T extends{fileName: string} = ts.SourceFile>
/**
* 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.

View File

@ -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<T extends{fileName: string} = ts.SourceFile> 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<T extends{fileName: string} = ts.SourceFile> im
}
}
isStale(sf: T, changedTsPaths: Set<string>, changedResources: Set<string>): boolean {
isStale(sf: T, changedTsPaths: Set<string>, changedResources: Set<AbsoluteFsPath>): boolean {
return isLogicallyChanged(sf, this.nodeFor(sf), changedTsPaths, EMPTY_SET, changedResources);
}
@ -77,7 +78,7 @@ export class FileDependencyGraph<T extends{fileName: string} = ts.SourceFile> im
*/
updateWithPhysicalChanges(
previous: FileDependencyGraph<T>, changedTsPaths: Set<string>, deletedTsPaths: Set<string>,
changedResources: Set<string>): Set<string> {
changedResources: Set<AbsoluteFsPath>): Set<string> {
const logicallyChanged = new Set<string>();
for (const sf of previous.nodes.keys()) {
@ -99,7 +100,7 @@ export class FileDependencyGraph<T extends{fileName: string} = ts.SourceFile> im
if (!this.nodes.has(sf)) {
this.nodes.set(sf, {
dependsOn: new Set<string>(),
usesResources: new Set<string>(),
usesResources: new Set<AbsoluteFsPath>(),
});
}
return this.nodes.get(sf) !;
@ -112,13 +113,13 @@ export class FileDependencyGraph<T extends{fileName: string} = ts.SourceFile> im
*/
function isLogicallyChanged<T extends{fileName: string}>(
sf: T, node: FileNode, changedTsPaths: ReadonlySet<string>, deletedTsPaths: ReadonlySet<string>,
changedResources: ReadonlySet<string>): boolean {
changedResources: ReadonlySet<AbsoluteFsPath>): 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<T extends{fileName: string}>(
interface FileNode {
dependsOn: Set<string>;
usesResources: Set<string>;
usesResources: Set<AbsoluteFsPath>;
}
const EMPTY_SET: ReadonlySet<any> = new Set<any>();

View File

@ -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<ClassRecord> {
state = {
kind: BuildStateKind.Pending,
pendingEmit: oldDriver.state.pendingEmit,
changedResourcePaths: new Set<string>(),
changedResourcePaths: new Set<AbsoluteFsPath>(),
changedTsPaths: new Set<string>(),
lastGood: oldDriver.state.lastGood,
};
@ -61,7 +62,7 @@ export class IncrementalDriver implements IncrementalBuild<ClassRecord> {
// 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<ClassRecord> {
const state: PendingBuildState = {
kind: BuildStateKind.Pending,
pendingEmit: new Set<string>(tsFiles.map(sf => sf.fileName)),
changedResourcePaths: new Set<string>(),
changedResourcePaths: new Set<AbsoluteFsPath>(),
changedTsPaths: new Set<string>(),
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<string>;
changedResourcePaths: Set<AbsoluteFsPath>;
}
interface AnalyzedBuildState extends BaseBuildState {

View File

@ -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);
}
}

View File

@ -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');