FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(compiler-cli): correct incremental behavior even with broken impo… · angular/angular@adeeb84 · GitHub

  1. angular
  2. angular

Repository navigation

Commit adeeb84

Browse files
alxhub
authored and
mhevery
committed
fix(compiler-cli): correct incremental behavior even with broken imports (#39967)
When the compiler is invoked via ngc or the Angular CLI, its APIs are used under the assumption that Angular analysis/diagnostics are only requested if the program has no TypeScript-level errors. A result of this assumption is that the incremental engine has not needed to resolve changes via its dependency graph when the program contained broken imports, since broken imports are a TypeScript error. The Angular Language Service for Ivy is using the compiler as a backend, and exercising its incremental compilation APIs without enforcing this assumption. As a result, the Language Service has run into issues where broken imports cause incremental compilation to fail and produce incorrect results. This commit introduces a mechanism within the compiler to keep track of files for which dependency analysis has failed, and to always treat such files as potentially affected by future incremental steps. PR Close #39967
1 parent 178cc51 commit adeeb84

5 files changed

Lines changed: 32 additions & 0 deletions

File tree

‎packages/compiler-cli/ngcc/src/analysis/util.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ class NoopDependencyTracker implements DependencyTracker {
1818
addResourceDependency(): void {}
1919
addTransitiveDependency(): void {}
2020
addTransitiveResources(): void {}
21+
recordDependencyAnalysisFailure(): void {}
2122
}
2223

2324
export const NOOP_DEPENDENCY_TRACKER: DependencyTracker = new NoopDependencyTracker();

‎packages/compiler-cli/src/ngtsc/incremental/api.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,12 @@ export interface DependencyTracker<T extends {fileName: string} = ts.SourceFile>
6464
* `resourcesOf` they will not automatically be added to `from`.
6565
*/
6666
addTransitiveResources(from: T, resourcesOf: T): void;
67+
68+
/**
69+
* Record that the given file contains unresolvable dependencies.
70+
*
71+
* In practice, this means that the dependency graph cannot provide insight into the effects of
72+
* future changes on that file.
73+
*/
74+
recordDependencyAnalysisFailure(file: T): void;
6775
}

‎packages/compiler-cli/src/ngtsc/incremental/src/dependency_tracking.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ export class FileDependencyGraph<T extends {fileName: string} = ts.SourceFile> i
5353
}
5454
}
5555

56+
recordDependencyAnalysisFailure(file: T): void {
57+
this.nodeFor(file).failedAnalysis = true;
58+
}
59+
5660
getResourceDependencies(from: T): AbsoluteFsPath[] {
5761
const node = this.nodes.get(from);
5862

@@ -97,6 +101,7 @@ export class FileDependencyGraph<T extends {fileName: string} = ts.SourceFile> i
97101
this.nodes.set(sf, {
98102
dependsOn: new Set(node.dependsOn),
99103
usesResources: new Set(node.usesResources),
104+
failedAnalysis: false,
100105
});
101106
}
102107
}
@@ -109,6 +114,7 @@ export class FileDependencyGraph<T extends {fileName: string} = ts.SourceFile> i
109114
this.nodes.set(sf, {
110115
dependsOn: new Set<string>(),
111116
usesResources: new Set<AbsoluteFsPath>(),
117+
failedAnalysis: false,
112118
});
113119
}
114120
return this.nodes.get(sf)!;
@@ -122,6 +128,12 @@ export class FileDependencyGraph<T extends {fileName: string} = ts.SourceFile> i
122128
function isLogicallyChanged<T extends {fileName: string}>(
123129
sf: T, node: FileNode, changedTsPaths: ReadonlySet<string>, deletedTsPaths: ReadonlySet<string>,
124130
changedResources: ReadonlySet<AbsoluteFsPath>): boolean {
131+
// A file is assumed to have logically changed if its dependencies could not be determined
132+
// accurately.
133+
if (node.failedAnalysis) {
134+
return true;
135+
}
136+
125137
// A file is logically changed if it has physically changed itself (including being deleted).
126138
if (changedTsPaths.has(sf.fileName) || deletedTsPaths.has(sf.fileName)) {
127139
return true;
@@ -146,6 +158,7 @@ function isLogicallyChanged<T extends {fileName: string}>(
146158
interface FileNode {
147159
dependsOn: Set<string>;
148160
usesResources: Set<AbsoluteFsPath>;
161+
failedAnalysis: boolean;
149162
}
150163

151164
const EMPTY_SET: ReadonlySet<any> = new Set<any>();

‎packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,15 @@ export class StaticInterpreter {
220220
if (node.originalKeywordKind === ts.SyntaxKind.UndefinedKeyword) {
221221
return undefined;
222222
} else {
223+
// Check if the symbol here is imported.
224+
if (this.dependencyTracker !== null && this.host.getImportOfIdentifier(node) !== null) {
225+
// It was, but no declaration for the node could be found. This means that the dependency
226+
// graph for the current file cannot be properly updated to account for this (broken)
227+
// import. Instead, the originating file is reported as failing dependency analysis,
228+
// ensuring that future compilations will always attempt to re-resolve the previously
229+
// broken identifier.
230+
this.dependencyTracker.recordDependencyAnalysisFailure(context.originatingFile);
231+
}
223232
return DynamicValue.fromUnknownIdentifier(node);
224233
}
225234
}

‎packages/compiler-cli/src/ngtsc/partial_evaluator/test/evaluator_spec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -991,4 +991,5 @@ const fakeDepTracker: DependencyTracker = {
991991
addResourceDependency: () => undefined,
992992
addTransitiveDependency: () => undefined,
993993
addTransitiveResources: () => undefined,
994+
recordDependencyAnalysisFailure: () => undefined,
994995
};

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL