Skip to content

Commit 76391f8

Browse files
petebacondarwinjasonaden
authored andcommitted
fix(ivy): use ReflectionHost in AbsoluteModuleStrategy (angular#30200)
The AbsoluteModuleStrategy in ngtsc assumed that the source code is formatted as TypeScript with regards to module exports. In ngcc this is not always the case, so this commit changes `AbsoluteModuleStrategy` so that it relies upon a `ReflectionHost` to compute the exports of a module. PR Close angular#30200
1 parent 02523de commit 76391f8

File tree

4 files changed

+18
-33
lines changed

4 files changed

+18
-33
lines changed

packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ export class DecorationAnalyzer {
7070
fullMetaReader = new CompoundMetadataReader([this.metaRegistry, this.dtsMetaReader]);
7171
refEmitter = new ReferenceEmitter([
7272
new LocalIdentifierStrategy(),
73-
new AbsoluteModuleStrategy(this.program, this.typeChecker, this.options, this.host),
73+
new AbsoluteModuleStrategy(
74+
this.program, this.typeChecker, this.options, this.host, this.reflectionHost),
7475
// TODO(alxhub): there's no reason why ngcc needs the "logical file system" logic here, as ngcc
7576
// projects only ever have one rootDir. Instead, ngcc should just switch its emitted import
7677
// based on whether a bestGuessOwningModule is present in the Reference.

packages/compiler-cli/src/ngtsc/imports/src/emitter.ts

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,13 @@ import {ExternalReference} from '@angular/compiler/src/compiler';
1111
import * as ts from 'typescript';
1212

1313
import {LogicalFileSystem, LogicalProjectPath} from '../../path';
14+
import {ReflectionHost} from '../../reflection';
1415
import {getSourceFile, isDeclaration, nodeNameForError, resolveModuleName} from '../../util/src/typescript';
1516

1617
import {findExportedNameOfNode} from './find_export';
1718
import {ImportMode, Reference} from './references';
1819

20+
1921
/**
2022
* A host which supports an operation to convert a file name into a module name.
2123
*
@@ -115,8 +117,9 @@ export class AbsoluteModuleStrategy implements ReferenceEmitStrategy {
115117
private moduleExportsCache = new Map<string, Map<ts.Declaration, string>|null>();
116118

117119
constructor(
118-
private program: ts.Program, private checker: ts.TypeChecker,
119-
private options: ts.CompilerOptions, private host: ts.CompilerHost) {}
120+
protected program: ts.Program, protected checker: ts.TypeChecker,
121+
protected options: ts.CompilerOptions, protected host: ts.CompilerHost,
122+
private reflectionHost: ReflectionHost) {}
120123

121124
emit(ref: Reference<ts.Node>, context: ts.SourceFile, importMode: ImportMode): Expression|null {
122125
if (ref.bestGuessOwningModule === null) {
@@ -159,7 +162,7 @@ export class AbsoluteModuleStrategy implements ReferenceEmitStrategy {
159162
return this.moduleExportsCache.get(moduleName) !;
160163
}
161164

162-
private enumerateExportsOfModule(specifier: string, fromFile: string):
165+
protected enumerateExportsOfModule(specifier: string, fromFile: string):
163166
Map<ts.Declaration, string>|null {
164167
// First, resolve the module specifier to its entry point, and get the ts.Symbol for it.
165168
const resolvedModule = resolveModuleName(specifier, fromFile, this.options, this.host);
@@ -172,34 +175,12 @@ export class AbsoluteModuleStrategy implements ReferenceEmitStrategy {
172175
return null;
173176
}
174177

175-
const entryPointSymbol = this.checker.getSymbolAtLocation(entryPointFile);
176-
if (entryPointSymbol === undefined) {
178+
const exports = this.reflectionHost.getExportsOfModule(entryPointFile);
179+
if (exports === null) {
177180
return null;
178181
}
179-
180-
// Next, build a Map of all the ts.Declarations exported via the specifier and their exported
181-
// names.
182182
const exportMap = new Map<ts.Declaration, string>();
183-
184-
const exports = this.checker.getExportsOfModule(entryPointSymbol);
185-
for (const expSymbol of exports) {
186-
// Resolve export symbols to their actual declarations.
187-
const declSymbol = expSymbol.flags & ts.SymbolFlags.Alias ?
188-
this.checker.getAliasedSymbol(expSymbol) :
189-
expSymbol;
190-
191-
// At this point the valueDeclaration of the symbol should be defined.
192-
const decl = declSymbol.valueDeclaration;
193-
if (decl === undefined) {
194-
continue;
195-
}
196-
197-
// Prefer importing the symbol via its declared name, but take any export of it otherwise.
198-
if (declSymbol.name === expSymbol.name || !exportMap.has(decl)) {
199-
exportMap.set(decl, expSymbol.name);
200-
}
201-
}
202-
183+
exports.forEach((declaration, name) => { exportMap.set(declaration.node, name); });
203184
return exportMap;
204185
}
205186
}

packages/compiler-cli/src/ngtsc/program.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,8 @@ export class NgtscProgram implements api.Program {
438438
// First, try to use local identifiers if available.
439439
new LocalIdentifierStrategy(),
440440
// Next, attempt to use an absolute import.
441-
new AbsoluteModuleStrategy(this.tsProgram, checker, this.options, this.host),
441+
new AbsoluteModuleStrategy(
442+
this.tsProgram, checker, this.options, this.host, this.reflector),
442443
// Finally, check if the reference is being written into a file within the project's logical
443444
// file system, and use a relative import if so. If this fails, ReferenceEmitter will throw
444445
// an error.

packages/compiler-cli/src/ngtsc/typecheck/test/type_constructor_spec.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import * as ts from 'typescript';
1010

1111
import {AbsoluteModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, Reference, ReferenceEmitter} from '../../imports';
1212
import {AbsoluteFsPath, LogicalFileSystem} from '../../path';
13-
import {isNamedClassDeclaration} from '../../reflection';
13+
import {TypeScriptReflectionHost, isNamedClassDeclaration} from '../../reflection';
1414
import {getDeclaration, makeProgram} from '../../testing/in_memory_typescript';
1515
import {getRootDirs} from '../../util/src/typescript';
1616
import {TypeCheckingConfig} from '../src/api';
@@ -54,7 +54,8 @@ TestClass.ngTypeCtor({value: 'test'});
5454
const logicalFs = new LogicalFileSystem(getRootDirs(host, options));
5555
const emitter = new ReferenceEmitter([
5656
new LocalIdentifierStrategy(),
57-
new AbsoluteModuleStrategy(program, checker, options, host),
57+
new AbsoluteModuleStrategy(
58+
program, checker, options, host, new TypeScriptReflectionHost(checker)),
5859
new LogicalProjectStrategy(checker, logicalFs),
5960
]);
6061
const ctx = new TypeCheckContext(
@@ -84,7 +85,8 @@ TestClass.ngTypeCtor({value: 'test'});
8485
const logicalFs = new LogicalFileSystem(getRootDirs(host, options));
8586
const emitter = new ReferenceEmitter([
8687
new LocalIdentifierStrategy(),
87-
new AbsoluteModuleStrategy(program, checker, options, host),
88+
new AbsoluteModuleStrategy(
89+
program, checker, options, host, new TypeScriptReflectionHost(checker)),
8890
new LogicalProjectStrategy(checker, logicalFs),
8991
]);
9092
const ctx = new TypeCheckContext(

0 commit comments

Comments
 (0)