GeezerVison is showing github.com — fix(compiler-cli): correct incremental behavior even with broken impo… · angular/angular@adeeb84
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: packages/compiler-cli/ngcc/src/analysis/util.ts +1Lines changed: 1 addition & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -18,6 +18,7 @@ class NoopDependencyTracker implements DependencyTracker { | 18 | 18 | addResourceDependency(): void {} | 19 | 19 | addTransitiveDependency(): void {} | 20 | 20 | addTransitiveResources(): void {} | | 21 | + recordDependencyAnalysisFailure(): void {} | 21 | 22 | } | 22 | 23 |
| 23 | 24 | export const NOOP_DEPENDENCY_TRACKER: DependencyTracker = new NoopDependencyTracker(); |
Collapse file Copy file name to clipboardExpand all lines: packages/compiler-cli/src/ngtsc/incremental/api.ts +8Lines changed: 8 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -64,4 +64,12 @@ export interface DependencyTracker<T extends {fileName: string} = ts.SourceFile> | 64 | 64 | * `resourcesOf` they will not automatically be added to `from`. | 65 | 65 | */ | 66 | 66 | 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; | 67 | 75 | } |
Collapse file Copy file name to clipboardExpand all lines: packages/compiler-cli/src/ngtsc/incremental/src/dependency_tracking.ts +13Lines changed: 13 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -53,6 +53,10 @@ export class FileDependencyGraph<T extends {fileName: string} = ts.SourceFile> i | 53 | 53 | } | 54 | 54 | } | 55 | 55 |
| | 56 | + recordDependencyAnalysisFailure(file: T): void { | | 57 | + this.nodeFor(file).failedAnalysis = true; | | 58 | + } | | 59 | + | 56 | 60 | getResourceDependencies(from: T): AbsoluteFsPath[] { | 57 | 61 | const node = this.nodes.get(from); | 58 | 62 |
| @@ -97,6 +101,7 @@ export class FileDependencyGraph<T extends {fileName: string} = ts.SourceFile> i | 97 | 101 | this.nodes.set(sf, { | 98 | 102 | dependsOn: new Set(node.dependsOn), | 99 | 103 | usesResources: new Set(node.usesResources), | | 104 | + failedAnalysis: false, | 100 | 105 | }); | 101 | 106 | } | 102 | 107 | } | @@ -109,6 +114,7 @@ export class FileDependencyGraph<T extends {fileName: string} = ts.SourceFile> i | 109 | 114 | this.nodes.set(sf, { | 110 | 115 | dependsOn: new Set<string>(), | 111 | 116 | usesResources: new Set<AbsoluteFsPath>(), | | 117 | + failedAnalysis: false, | 112 | 118 | }); | 113 | 119 | } | 114 | 120 | return this.nodes.get(sf)!; | @@ -122,6 +128,12 @@ export class FileDependencyGraph<T extends {fileName: string} = ts.SourceFile> i | 122 | 128 | function isLogicallyChanged<T extends {fileName: string}>( | 123 | 129 | sf: T, node: FileNode, changedTsPaths: ReadonlySet<string>, deletedTsPaths: ReadonlySet<string>, | 124 | 130 | 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 | + | 125 | 137 | // A file is logically changed if it has physically changed itself (including being deleted). | 126 | 138 | if (changedTsPaths.has(sf.fileName) || deletedTsPaths.has(sf.fileName)) { | 127 | 139 | return true; | @@ -146,6 +158,7 @@ function isLogicallyChanged<T extends {fileName: string}>( | 146 | 158 | interface FileNode { | 147 | 159 | dependsOn: Set<string>; | 148 | 160 | usesResources: Set<AbsoluteFsPath>; | | 161 | + failedAnalysis: boolean; | 149 | 162 | } | 150 | 163 |
| 151 | 164 | const EMPTY_SET: ReadonlySet<any> = new Set<any>(); |
Collapse file Copy file name to clipboardExpand all lines: packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts +9Lines changed: 9 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -220,6 +220,15 @@ export class StaticInterpreter { | 220 | 220 | if (node.originalKeywordKind === ts.SyntaxKind.UndefinedKeyword) { | 221 | 221 | return undefined; | 222 | 222 | } 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 | + } | 223 | 232 | return DynamicValue.fromUnknownIdentifier(node); | 224 | 233 | } | 225 | 234 | } | |
Collapse file Copy file name to clipboardExpand all lines: packages/compiler-cli/src/ngtsc/partial_evaluator/test/evaluator_spec.ts +1Lines changed: 1 addition & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -991,4 +991,5 @@ const fakeDepTracker: DependencyTracker = { | 991 | 991 | addResourceDependency: () => undefined, | 992 | 992 | addTransitiveDependency: () => undefined, | 993 | 993 | addTransitiveResources: () => undefined, | | 994 | + recordDependencyAnalysisFailure: () => undefined, | 994 | 995 | }; |
|