Skip to content

Commit

Permalink
fix(ivy): recompile on template change in ngc watch mode on Windows
Browse files Browse the repository at this point in the history
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
  • Loading branch information
JoostK committed Nov 24, 2019
1 parent 8c76e78 commit b714f5a
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 14 deletions.
6 changes: 4 additions & 2 deletions packages/compiler-cli/src/ngtsc/annotations/src/component.ts
Expand Up @@ -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')) {
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions packages/compiler-cli/src/ngtsc/incremental/BUILD.bazel
Expand Up @@ -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",
Expand Down
13 changes: 7 additions & 6 deletions packages/compiler-cli/src/ngtsc/incremental/src/state.ts
Expand Up @@ -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';

Expand Down Expand Up @@ -53,15 +54,15 @@ export class IncrementalDriver implements DependencyTracker, ResourceDependencyR
state = {
kind: BuildStateKind.Pending,
pendingEmit: oldDriver.state.pendingEmit,
changedResourcePaths: new Set<string>(),
changedResourcePaths: new Set<AbsoluteFsPath>(),
changedTsPaths: new Set<string>(),
};
}

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

Expand Down Expand Up @@ -122,7 +123,7 @@ export class IncrementalDriver implements DependencyTracker, ResourceDependencyR
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>(),
};

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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<ts.SourceFile>();
resourcePaths = new Set<string>();
resourcePaths = new Set<AbsoluteFsPath>();
}


Expand Down Expand Up @@ -271,7 +272,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 {
Expand Down
Expand Up @@ -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 {
Expand Down
10 changes: 6 additions & 4 deletions packages/compiler-cli/src/perform_watch.ts
Expand Up @@ -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 (
Expand All @@ -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);
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/compiler-cli/test/perform_watch_spec.ts
Expand Up @@ -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');

Expand Down

0 comments on commit b714f5a

Please sign in to comment.