fix(ivy): in ngcc, handle inline exports in commonjs code (#32129)
One of the compiler's tasks is to enumerate the exports of a given ES module. This can happen for example to resolve `foo.bar` where `foo` is a namespace import: ```typescript import * as foo from './foo'; @NgModule({ directives: [foo.DIRECTIVES], }) ``` In this case, the compiler must enumerate the exports of `foo.ts` in order to evaluate the expression `foo.DIRECTIVES`. When this operation occurs under ngcc, it must deal with the different module formats and types of exports that occur. In commonjs code, a problem arises when certain exports are downleveled. ```typescript export const DIRECTIVES = [ FooDir, BarDir, ]; ``` can be downleveled to: ```javascript exports.DIRECTIVES = [ FooDir, BarDir, ``` Previously, ngtsc and ngcc expected that any export would have an associated `ts.Declaration` node. `export class`, `export function`, etc. all retain `ts.Declaration`s even when downleveled. But the `export const` construct above does not. Therefore, ngcc would not detect `DIRECTIVES` as an export of `foo.ts`, and the evaluation of `foo.DIRECTIVES` would therefore fail. To solve this problem, the core concept of an exported `Declaration` according to the `ReflectionHost` API is split into a `ConcreteDeclaration` which has a `ts.Declaration`, and an `InlineDeclaration` which instead has a `ts.Expression`. Differentiating between these allows ngcc to return an `InlineDeclaration` for `DIRECTIVES` and correctly keep track of this export. PR Close #32129
This commit is contained in:

committed by
Andrew Kushnir

parent
69ce1c2d41
commit
02bab8cf90
@ -176,7 +176,13 @@ export class AbsoluteModuleStrategy implements ReferenceEmitStrategy {
|
||||
return null;
|
||||
}
|
||||
const exportMap = new Map<ts.Declaration, string>();
|
||||
exports.forEach((declaration, name) => { exportMap.set(declaration.node, name); });
|
||||
exports.forEach((declaration, name) => {
|
||||
// It's okay to skip inline declarations, since by definition they're not target-able with a
|
||||
// ts.Declaration anyway.
|
||||
if (declaration.node !== null) {
|
||||
exportMap.set(declaration.node, name);
|
||||
}
|
||||
});
|
||||
return exportMap;
|
||||
}
|
||||
}
|
||||
|
@ -10,7 +10,7 @@ import * as ts from 'typescript';
|
||||
|
||||
import {Reference} from '../../imports';
|
||||
import {OwningModule} from '../../imports/src/references';
|
||||
import {Declaration, ReflectionHost} from '../../reflection';
|
||||
import {Declaration, InlineDeclaration, ReflectionHost} from '../../reflection';
|
||||
import {isDeclaration} from '../../util/src/typescript';
|
||||
|
||||
import {ArrayConcatBuiltinFn, ArraySliceBuiltinFn} from './builtin';
|
||||
@ -20,6 +20,7 @@ import {BuiltinFn, EnumValue, ResolvedValue, ResolvedValueArray, ResolvedValueMa
|
||||
import {evaluateTsHelperInline} from './ts_helpers';
|
||||
|
||||
|
||||
|
||||
/**
|
||||
* Tracks the scope of a function body, which includes `ResolvedValue`s for the parameters of that
|
||||
* body.
|
||||
@ -223,8 +224,13 @@ export class StaticInterpreter {
|
||||
return DynamicValue.fromUnknownIdentifier(node);
|
||||
}
|
||||
}
|
||||
const result =
|
||||
this.visitDeclaration(decl.node, {...context, ...joinModuleContext(context, node, decl)});
|
||||
const declContext = {...context, ...joinModuleContext(context, node, decl)};
|
||||
// The identifier's declaration is either concrete (a ts.Declaration exists for it) or inline
|
||||
// (a direct reference to a ts.Expression).
|
||||
// TODO(alxhub): remove cast once TS is upgraded in g3.
|
||||
const result = decl.node !== null ?
|
||||
this.visitDeclaration(decl.node, declContext) :
|
||||
this.visitExpression((decl as InlineDeclaration).expression, declContext);
|
||||
if (result instanceof Reference) {
|
||||
// Only record identifiers to non-synthetic references. Synthetic references may not have the
|
||||
// same value at runtime as they do at compile time, so it's not legal to refer to them by the
|
||||
@ -322,10 +328,14 @@ export class StaticInterpreter {
|
||||
}
|
||||
const map = new Map<string, ResolvedValue>();
|
||||
declarations.forEach((decl, name) => {
|
||||
const value = this.visitDeclaration(
|
||||
decl.node, {
|
||||
...context, ...joinModuleContext(context, node, decl),
|
||||
});
|
||||
const declContext = {
|
||||
...context, ...joinModuleContext(context, node, decl),
|
||||
};
|
||||
// Visit both concrete and inline declarations.
|
||||
// TODO(alxhub): remove cast once TS is upgraded in g3.
|
||||
const value = decl.node !== null ?
|
||||
this.visitDeclaration(decl.node, declContext) :
|
||||
this.visitExpression((decl as InlineDeclaration).expression, declContext);
|
||||
map.set(name, value);
|
||||
});
|
||||
return map;
|
||||
|
@ -342,28 +342,65 @@ export interface Import {
|
||||
}
|
||||
|
||||
/**
|
||||
* The declaration of a symbol, along with information about how it was imported into the
|
||||
* application.
|
||||
* Base type for all `Declaration`s.
|
||||
*/
|
||||
export interface Declaration<T extends ts.Declaration = ts.Declaration> {
|
||||
/**
|
||||
* TypeScript reference to the declaration itself.
|
||||
*/
|
||||
node: T;
|
||||
|
||||
export interface BaseDeclaration<T extends ts.Declaration = ts.Declaration> {
|
||||
/**
|
||||
* The absolute module path from which the symbol was imported into the application, if the symbol
|
||||
* was imported via an absolute module (even through a chain of re-exports). If the symbol is part
|
||||
* of the application and was not imported from an absolute path, this will be `null`.
|
||||
*/
|
||||
viaModule: string|null;
|
||||
|
||||
/**
|
||||
* TypeScript reference to the declaration itself, if one exists.
|
||||
*/
|
||||
node: T|null;
|
||||
}
|
||||
|
||||
/**
|
||||
* A declaration that has an associated TypeScript `ts.Declaration`.
|
||||
*
|
||||
* The alternative is an `InlineDeclaration`.
|
||||
*/
|
||||
export interface ConcreteDeclaration<T extends ts.Declaration = ts.Declaration> extends
|
||||
BaseDeclaration<T> {
|
||||
node: T;
|
||||
}
|
||||
|
||||
/**
|
||||
* A declaration that does not have an associated TypeScript `ts.Declaration`, only a
|
||||
* `ts.Expression`.
|
||||
*
|
||||
* This can occur in some downlevelings when an `export const VAR = ...;` (a `ts.Declaration`) is
|
||||
* transpiled to an assignment statement (e.g. `exports.VAR = ...;`). There is no `ts.Declaration`
|
||||
* associated with `VAR` in that case, only an expression.
|
||||
*/
|
||||
export interface InlineDeclaration extends BaseDeclaration {
|
||||
node: null;
|
||||
|
||||
/**
|
||||
* The `ts.Expression` which constitutes the value of the declaration.
|
||||
*/
|
||||
expression: ts.Expression;
|
||||
}
|
||||
|
||||
/**
|
||||
* The declaration of a symbol, along with information about how it was imported into the
|
||||
* application.
|
||||
*
|
||||
* This can either be a `ConcreteDeclaration` if the underlying TypeScript node for the symbol is an
|
||||
* actual `ts.Declaration`, or an `InlineDeclaration` if the declaration was transpiled in certain
|
||||
* downlevelings to a `ts.Expression` instead.
|
||||
*/
|
||||
export type Declaration<T extends ts.Declaration = ts.Declaration> =
|
||||
ConcreteDeclaration<T>| InlineDeclaration;
|
||||
|
||||
/**
|
||||
* Abstracts reflection operations on a TypeScript AST.
|
||||
*
|
||||
* Depending on the format of the code being interpreted, different concepts are represented with
|
||||
* different syntactical structures. The `ReflectionHost` abstracts over those differences and
|
||||
* Depending on the format of the code being interpreted, different concepts are represented
|
||||
* with different syntactical structures. The `ReflectionHost` abstracts over those differences and
|
||||
* presents a single API by which the compiler can query specific information about the AST.
|
||||
*
|
||||
* All operations on the `ReflectionHost` require the use of TypeScript `ts.Node`s with binding
|
||||
|
@ -307,8 +307,9 @@ runInEachFileSystem(() => {
|
||||
} else if (directTargetDecl === null) {
|
||||
return fail('No declaration found for DirectTarget');
|
||||
}
|
||||
expect(targetDecl.node.getSourceFile().fileName).toBe(_('/node_modules/absolute/index.ts'));
|
||||
expect(ts.isClassDeclaration(targetDecl.node)).toBe(true);
|
||||
expect(targetDecl.node !.getSourceFile().fileName)
|
||||
.toBe(_('/node_modules/absolute/index.ts'));
|
||||
expect(ts.isClassDeclaration(targetDecl.node !)).toBe(true);
|
||||
expect(directTargetDecl.viaModule).toBe('absolute');
|
||||
expect(directTargetDecl.node).toBe(targetDecl.node);
|
||||
});
|
||||
|
Reference in New Issue
Block a user