fix(ngcc): Esm5ReflectionHost.getDeclarationOfIdentifier should handle aliased inner declarations (#33252)

In ES5 modules, the class declarations consist of an IIFE with inner
and outer declarations that represent the class. The `EsmReflectionHost`
has logic to ensure that `getDeclarationOfIdentifier()` always returns the
outer declaration.

Before this commit, if an identifier referred to an alias of the inner
declaration, then `getDeclarationOfIdentifier()` was failing to find
the outer declaration - instead returning the inner declaration.

Now the identifier is correctly resolved up to the outer declaration
as expected.

This should fix some of the failing 3rd party packages discussed in
https://github.com/angular/ngcc-validation/issues/57.

PR Close #33252
This commit is contained in:
Pete Bacon Darwin 2019-10-18 14:45:52 +01:00 committed by Matias Niemelä
parent 01d3599f32
commit bfd07b3c94
2 changed files with 76 additions and 15 deletions

View File

@ -137,7 +137,7 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost {
} }
const outerDeclaration = getClassDeclarationFromInnerFunctionDeclaration(declaration); const outerDeclaration = getClassDeclarationFromInnerFunctionDeclaration(declaration);
if (outerDeclaration === undefined || !hasNameIdentifier(outerDeclaration)) { if (outerDeclaration === null || !hasNameIdentifier(outerDeclaration)) {
return undefined; return undefined;
} }
@ -162,12 +162,23 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost {
* otherwise. * otherwise.
*/ */
getDeclarationOfIdentifier(id: ts.Identifier): Declaration|null { getDeclarationOfIdentifier(id: ts.Identifier): Declaration|null {
// Get the identifier for the outer class node (if any). const superDeclaration = super.getDeclarationOfIdentifier(id);
const outerClassNode = getClassDeclarationFromInnerFunctionDeclaration(id.parent);
const declaration = super.getDeclarationOfIdentifier(outerClassNode ? outerClassNode.name : id);
if (!declaration || declaration.node === null || !ts.isVariableDeclaration(declaration.node) || if (superDeclaration === null || superDeclaration.node === null) {
declaration.node.initializer !== undefined || return superDeclaration;
}
// Get the identifier for the outer class node (if any).
const outerClassNode = getClassDeclarationFromInnerFunctionDeclaration(superDeclaration.node);
const declaration = outerClassNode !== null ?
super.getDeclarationOfIdentifier(outerClassNode.name) :
superDeclaration;
if (!declaration || declaration.node === null) {
return declaration;
}
if (!ts.isVariableDeclaration(declaration.node) || declaration.node.initializer !== undefined ||
// VariableDeclaration => VariableDeclarationList => VariableStatement => IIFE Block // VariableDeclaration => VariableDeclarationList => VariableStatement => IIFE Block
!ts.isBlock(declaration.node.parent.parent.parent)) { !ts.isBlock(declaration.node.parent.parent.parent)) {
return declaration; return declaration;
@ -528,35 +539,35 @@ function readPropertyFunctionExpression(object: ts.ObjectLiteralExpression, name
* @returns the outer variable declaration or `undefined` if it is not a "class". * @returns the outer variable declaration or `undefined` if it is not a "class".
*/ */
function getClassDeclarationFromInnerFunctionDeclaration(node: ts.Node): function getClassDeclarationFromInnerFunctionDeclaration(node: ts.Node):
ClassDeclaration<ts.VariableDeclaration>|undefined { ClassDeclaration<ts.VariableDeclaration>|null {
if (ts.isFunctionDeclaration(node)) { if (ts.isFunctionDeclaration(node)) {
// It might be the function expression inside the IIFE. We need to go 5 levels up... // It might be the function expression inside the IIFE. We need to go 5 levels up...
// 1. IIFE body. // 1. IIFE body.
let outerNode = node.parent; let outerNode = node.parent;
if (!outerNode || !ts.isBlock(outerNode)) return undefined; if (!outerNode || !ts.isBlock(outerNode)) return null;
// 2. IIFE function expression. // 2. IIFE function expression.
outerNode = outerNode.parent; outerNode = outerNode.parent;
if (!outerNode || !ts.isFunctionExpression(outerNode)) return undefined; if (!outerNode || !ts.isFunctionExpression(outerNode)) return null;
// 3. IIFE call expression. // 3. IIFE call expression.
outerNode = outerNode.parent; outerNode = outerNode.parent;
if (!outerNode || !ts.isCallExpression(outerNode)) return undefined; if (!outerNode || !ts.isCallExpression(outerNode)) return null;
// 4. Parenthesis around IIFE. // 4. Parenthesis around IIFE.
outerNode = outerNode.parent; outerNode = outerNode.parent;
if (!outerNode || !ts.isParenthesizedExpression(outerNode)) return undefined; if (!outerNode || !ts.isParenthesizedExpression(outerNode)) return null;
// 5. Outer variable declaration. // 5. Outer variable declaration.
outerNode = outerNode.parent; outerNode = outerNode.parent;
if (!outerNode || !ts.isVariableDeclaration(outerNode)) return undefined; if (!outerNode || !ts.isVariableDeclaration(outerNode)) return null;
// Finally, ensure that the variable declaration has a `name` identifier. // Finally, ensure that the variable declaration has a `name` identifier.
return hasNameIdentifier(outerNode) ? outerNode : undefined; return hasNameIdentifier(outerNode) ? outerNode : null;
} }
return undefined; return null;
} }
export function getIifeBody(declaration: ts.Declaration): ts.Block|undefined { export function getIifeBody(declaration: ts.Declaration): ts.Block|undefined {

View File

@ -1817,8 +1817,58 @@ runInEachFileSystem(() => {
superGetDeclarationOfIdentifierSpy.calls.reset(); superGetDeclarationOfIdentifierSpy.calls.reset();
expect(host.getDeclarationOfIdentifier(innerIdentifier) !.node).toBe(outerDeclaration); expect(host.getDeclarationOfIdentifier(innerIdentifier) !.node).toBe(outerDeclaration);
expect(superGetDeclarationOfIdentifierSpy).toHaveBeenCalledWith(innerIdentifier);
expect(superGetDeclarationOfIdentifierSpy).toHaveBeenCalledWith(outerIdentifier); expect(superGetDeclarationOfIdentifierSpy).toHaveBeenCalledWith(outerIdentifier);
expect(superGetDeclarationOfIdentifierSpy).toHaveBeenCalledTimes(1); expect(superGetDeclarationOfIdentifierSpy).toHaveBeenCalledTimes(2);
});
it('should return the correct outer declaration for an aliased inner class declaration inside an ES5 IIFE',
() => {
// Note that the inner class declaration `function FroalaEditorModule() {}` is aliased
// internally to `FroalaEditorModule_1`, which is used in the object returned from
// `forRoot()`.
const PROGRAM_FILE: TestFile = {
name: _('/test.js'),
contents: `
var FroalaEditorModule = /** @class */ (function () {
function FroalaEditorModule() {
}
FroalaEditorModule_1 = FroalaEditorModule;
FroalaEditorModule.forRoot = function () {
return { ngModule: FroalaEditorModule_1, providers: [] };
};
var FroalaEditorModule_1;
FroalaEditorModule = FroalaEditorModule_1 = __decorate([
NgModule({
declarations: [FroalaEditorDirective],
exports: [FroalaEditorDirective]
})
], FroalaEditorModule);
return FroalaEditorModule;
}());
export { FroalaEditorModule };
`
};
loadTestFiles([PROGRAM_FILE]);
const {program} = makeTestBundleProgram(PROGRAM_FILE.name);
const host = new Esm5ReflectionHost(new MockLogger(), false, program.getTypeChecker());
const expectedDeclaration = getDeclaration(
program, PROGRAM_FILE.name, 'FroalaEditorModule', isNamedVariableDeclaration);
// Grab the `FroalaEditorModule_1` identifier returned from the `forRoot()` method
const forRootMethod = ((((expectedDeclaration.initializer as ts.ParenthesizedExpression)
.expression as ts.CallExpression)
.expression as ts.FunctionExpression)
.body.statements[2] as ts.ExpressionStatement);
const identifier =
(((((forRootMethod.expression as ts.BinaryExpression).right as ts.FunctionExpression)
.body.statements[0] as ts.ReturnStatement)
.expression as ts.ObjectLiteralExpression)
.properties[0] as ts.PropertyAssignment)
.initializer as ts.Identifier;
const actualDeclaration = host.getDeclarationOfIdentifier(identifier) !;
expect(actualDeclaration.node !.getText()).toBe(expectedDeclaration.getText());
}); });
}); });