honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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

Previously, inline exports of the form `exports.foo = <implementation>;` were
being interpreted (by the ngtsc `PartialInterpeter`) as `Reference` objects.
This is not what is desired since it prevents the value of the export
from being unpacked, such as when analyzing `NgModule` declarations:

```
exports.directives = [Directive1, Directive2];

@NgImport({declarations: [exports.directives]})
class AppModule {}
```

In this example the interpreter would think that `exports.directives`
was a reference rather than an array that needs to be unpacked.

This bug was picked up by the ngcc-validation repository. See
https://github.com/angular/ngcc-validation/pull/1990 and
https://circleci.com/gh/angular/ngcc-validation/17130

PR Close #39267
This commit is contained in:
Pete Bacon Darwin 2020-10-14 16:30:09 +01:00 committed by atscott
parent ac0016cd82
commit 822b838fbc
5 changed files with 57 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
packages/compiler-cli/ngcc/src/host/commonjs_host.ts
View File

@ -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';
@ -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(
@ -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 {
@ -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(...)`.

17
packages/compiler-cli/ngcc/src/host/commonjs_umd_utils.ts
View File

@ -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
packages/compiler-cli/ngcc/src/host/umd_host.ts
View File

@ -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';
@ -77,6 +77,7 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
return {
kind: DeclarationKind.Inline,
node: outerNode.left,
implementation: outerNode.right,
known: null,
viaModule: null,
};
@ -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,
};
@ -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,
},
@ -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
packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts
View File

@ -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);
});

9
packages/compiler-cli/ngcc/test/host/umd_host_spec.ts
View File

@ -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)', () => {
@ -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);
});