Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ngcc): ensure that "inline exports" can be interpreted correctly #39267

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
45 changes: 27 additions & 18 deletions packages/compiler-cli/ngcc/src/host/commonjs_host.ts
Expand Up @@ -14,7 +14,7 @@ import {Declaration, DeclarationKind, Import} from '../../../src/ngtsc/reflectio
import {BundleProgram} from '../packages/bundle_program';
import {FactoryMap, isDefined} from '../utils';

import {DefinePropertyReexportStatement, ExportDeclaration, ExportsStatement, extractGetterFnExpression, findNamespaceOfIdentifier, findRequireCallReference, isDefinePropertyReexportStatement, isExportsStatement, isExternalImport, isRequireCall, isWildcardReexportStatement, RequireCall, WildcardReexportStatement} from './commonjs_umd_utils';
import {DefinePropertyReexportStatement, ExportDeclaration, ExportsStatement, extractGetterFnExpression, findNamespaceOfIdentifier, findRequireCallReference, isDefinePropertyReexportStatement, isExportsStatement, isExternalImport, isRequireCall, isWildcardReexportStatement, RequireCall, skipAliases, WildcardReexportStatement} from './commonjs_umd_utils';
import {Esm5ReflectionHost} from './esm5_host';
import {NgccClassSymbol} from './ngcc_host';

Expand Down Expand Up @@ -117,9 +117,16 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost {
}

private extractBasicCommonJsExportDeclaration(statement: ExportsStatement): ExportDeclaration {
const exportExpression = statement.expression.right;
const name = statement.expression.left.name.text;
return this.extractCommonJsExportDeclaration(name, exportExpression);
const exportExpression = skipAliases(statement.expression.right);
const node = statement.expression.left;
const declaration = this.getDeclarationOfExpression(exportExpression) ?? {
kind: DeclarationKind.Inline,
node,
implementation: exportExpression,
known: null,
viaModule: null,
};
return {name: node.name.text, declaration};
}

private extractCommonJsWildcardReexports(
Expand Down Expand Up @@ -163,7 +170,22 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost {
if (getterFnExpression === null) {
return null;
}
return this.extractCommonJsExportDeclaration(name, getterFnExpression);

const declaration = this.getDeclarationOfExpression(getterFnExpression);
if (declaration !== null) {
return {name, declaration};
}

return {
name,
declaration: {
kind: DeclarationKind.Inline,
node: args[1],
implementation: getterFnExpression,
known: null,
viaModule: null,
},
};
}

private findCommonJsImport(id: ts.Identifier): RequireCall|null {
Expand All @@ -173,19 +195,6 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost {
return nsIdentifier && findRequireCallReference(nsIdentifier, this.checker);
}

private extractCommonJsExportDeclaration(name: string, expression: ts.Expression):
ExportDeclaration {
const declaration = this.getDeclarationOfExpression(expression);
if (declaration !== null) {
return {name, declaration};
} else {
return {
name,
declaration: {node: expression, known: null, kind: DeclarationKind.Inline, viaModule: null},
};
}
}

/**
* Handle the case where the identifier represents a reference to a whole CommonJS
* module, i.e. the result of a call to `require(...)`.
Expand Down
17 changes: 17 additions & 0 deletions packages/compiler-cli/ngcc/src/host/commonjs_umd_utils.ts
Expand Up @@ -264,3 +264,20 @@ export interface ExportsStatement extends ts.ExpressionStatement {
export function isExportsStatement(stmt: ts.Node): stmt is ExportsStatement {
return ts.isExpressionStatement(stmt) && isExportsAssignment(stmt.expression);
}

/**
* Find the far right hand side of a sequence of aliased assignements of the form
*
* ```
* exports.MyClass = alias1 = alias2 = <<declaration>>
* ```
*
* @param node the expression to parse
* @returns the original `node` or the far right expression of a series of assignments.
*/
export function skipAliases(node: ts.Expression): ts.Expression {
while (isAssignment(node)) {
node = node.right;
}
return node;
}
24 changes: 5 additions & 19 deletions packages/compiler-cli/ngcc/src/host/umd_host.ts
Expand Up @@ -14,7 +14,7 @@ import {Declaration, DeclarationKind, Import, isNamedFunctionDeclaration} from '
import {BundleProgram} from '../packages/bundle_program';
import {FactoryMap, getTsHelperFnFromIdentifier, stripExtension} from '../utils';

import {DefinePropertyReexportStatement, ExportDeclaration, ExportsStatement, extractGetterFnExpression, findNamespaceOfIdentifier, findRequireCallReference, isDefinePropertyReexportStatement, isExportsAssignment, isExportsDeclaration, isExportsStatement, isExternalImport, isRequireCall, isWildcardReexportStatement, WildcardReexportStatement} from './commonjs_umd_utils';
import {DefinePropertyReexportStatement, ExportDeclaration, ExportsStatement, extractGetterFnExpression, findNamespaceOfIdentifier, findRequireCallReference, isDefinePropertyReexportStatement, isExportsAssignment, isExportsDeclaration, isExportsStatement, isExternalImport, isRequireCall, isWildcardReexportStatement, skipAliases, WildcardReexportStatement} from './commonjs_umd_utils';
import {getInnerClassDeclaration, getOuterNodeFromInnerDeclaration, isAssignment} from './esm2015_host';
import {Esm5ReflectionHost} from './esm5_host';
import {NgccClassSymbol} from './ngcc_host';
Expand Down Expand Up @@ -77,6 +77,7 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
return {
kind: DeclarationKind.Inline,
node: outerNode.left,
implementation: outerNode.right,
known: null,
viaModule: null,
};
Expand Down Expand Up @@ -278,6 +279,7 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
const declaration = this.getDeclarationOfExpression(exportExpression) ?? {
kind: DeclarationKind.Inline,
node: statement.expression.left,
implementation: statement.expression.right,
known: null,
viaModule: null,
};
Expand Down Expand Up @@ -340,7 +342,8 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
name,
declaration: {
kind: DeclarationKind.Inline,
node: getterFnExpression,
node: args[1],
implementation: getterFnExpression,
known: null,
viaModule: null,
},
Expand Down Expand Up @@ -564,20 +567,3 @@ function getRequiredModulePath(wrapperFn: ts.FunctionExpression, paramIndex: num
function isExportsIdentifier(node: ts.Node): node is ts.Identifier {
return ts.isIdentifier(node) && node.text === 'exports';
}

/**
* Find the far right hand side of a sequence of aliased assignements of the form
*
* ```
* exports.MyClass = alias1 = alias2 = <<declaration>>
* ```
*
* @param node the expression to parse
* @returns the original `node` or the far right expression of a series of assignments.
*/
function skipAliases(node: ts.Expression): ts.Expression {
while (isAssignment(node)) {
node = node.right;
}
return node;
}
5 changes: 3 additions & 2 deletions packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts
Expand Up @@ -2422,9 +2422,10 @@ exports.MissingClass2 = MissingClass2;
const file = getSourceFileOrError(bundle.program, _('/inline_export.js'));
const exportDeclarations = host.getExportsOfModule(file);
expect(exportDeclarations).not.toBeNull();
const decl = exportDeclarations!.get('directives')!;
const decl = exportDeclarations!.get('directives') as InlineDeclaration;
expect(decl).toBeDefined();
expect(decl.node).toBeDefined();
expect(decl.node.getText()).toEqual('exports.directives');
expect(decl.implementation!.getText()).toEqual('[foo]');
expect(decl.kind).toEqual(DeclarationKind.Inline);
});

Expand Down
9 changes: 5 additions & 4 deletions packages/compiler-cli/ngcc/test/host/umd_host_spec.ts
Expand Up @@ -2750,13 +2750,13 @@ runInEachFileSystem(() => {
const exportDeclarations = host.getExportsOfModule(file);
expect(exportDeclarations).not.toBe(null);
expect(exportDeclarations!.size).toEqual(1);
const classDecl = exportDeclarations!.get('DecoratedClass')!;
const classDecl = exportDeclarations!.get('DecoratedClass') as InlineDeclaration;
expect(classDecl).toBeDefined();
expect(classDecl.kind).toEqual(DeclarationKind.Inline);
expect(classDecl.known).toBe(null);
expect(classDecl.viaModule).toBe(null);
expect(classDecl.node.getText()).toEqual('exports.DecoratedClass');
expect(classDecl.node.parent.parent.getText()).toContain('function DecoratedClass() {');
expect(classDecl.implementation!.getText()).toContain('function DecoratedClass() {');
});

it('should handle wildcard re-exports of other modules (with emitted helpers)', () => {
Expand Down Expand Up @@ -2824,9 +2824,10 @@ runInEachFileSystem(() => {
const file = getSourceFileOrError(bundle.program, INLINE_EXPORT_FILE.name);
const exportDeclarations = host.getExportsOfModule(file);
expect(exportDeclarations).not.toBe(null);
const decl = exportDeclarations!.get('directives')!;
const decl = exportDeclarations!.get('directives') as InlineDeclaration;
expect(decl).toBeDefined();
expect(decl.node).toBeDefined();
expect(decl.node.getText()).toEqual('exports.directives');
expect(decl.implementation!.getText()).toEqual('[foo]');
expect(decl.kind).toEqual(DeclarationKind.Inline);
});

Expand Down
Expand Up @@ -11,7 +11,7 @@ import * as ts from 'typescript';
import {Reference} from '../../imports';
import {OwningModule} from '../../imports/src/references';
import {DependencyTracker} from '../../incremental/api';
import {Declaration, DeclarationNode, EnumMember, FunctionDefinition, isConcreteDeclaration, ReflectionHost, SpecialDeclarationKind} from '../../reflection';
import {Declaration, DeclarationKind, DeclarationNode, EnumMember, FunctionDefinition, isConcreteDeclaration, ReflectionHost, SpecialDeclarationKind} from '../../reflection';
import {isDeclaration} from '../../util/src/typescript';

import {ArrayConcatBuiltinFn, ArraySliceBuiltinFn} from './builtin';
Expand Down Expand Up @@ -231,7 +231,7 @@ export class StaticInterpreter {
return this.getResolvedEnum(decl.node, decl.identity.enumMembers, context);
}
const declContext = {...context, ...joinModuleContext(context, node, decl)};
const result = this.visitDeclaration(decl.node, declContext);
const result = this.visitAmbiguousDeclaration(decl, 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
Expand Down Expand Up @@ -337,10 +337,18 @@ export class StaticInterpreter {
};

// Visit both concrete and inline declarations.
return this.visitDeclaration(decl.node, declContext);
return this.visitAmbiguousDeclaration(decl, declContext);
});
}

private visitAmbiguousDeclaration(decl: Declaration, declContext: Context) {
return decl.kind === DeclarationKind.Inline && decl.implementation !== undefined ?
// Inline declarations with an `implementation` should be visited as expressions
this.visitExpression(decl.implementation, declContext) :
// Otherwise just visit the declaration `node`
this.visitDeclaration(decl.node, declContext);
}

private accessHelper(node: ts.Node, lhs: ResolvedValue, rhs: string|number, context: Context):
ResolvedValue {
const strIndex = `${rhs}`;
Expand Down
1 change: 1 addition & 0 deletions packages/compiler-cli/src/ngtsc/reflection/src/host.ts
Expand Up @@ -625,6 +625,7 @@ export interface DownleveledEnum {
export interface InlineDeclaration extends
BaseDeclaration<Exclude<DeclarationNode, ts.Declaration>> {
kind: DeclarationKind.Inline;
implementation?: ts.Expression;
}

/**
Expand Down