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

fix(ngcc): correctly associate decorators with aliased classes (#33878) 

In flat bundle formats, multiple classes that have the same name can be
suffixed to become unique. In ES5-like bundles this results in the outer
declaration from having a different name from the "implementation"
declaration within the class' IIFE, as the implementation declaration
may not have been suffixed.

As an example, the following code would fail to have a `Directive`
decorator as ngcc would search for `__decorate` calls that refer to
`AliasedDirective$1` by name, whereas the `__decorate` call actually
uses the `AliasedDirective` name.

```javascript
var AliasedDirective$1 = /** @class */ (function () {
    function AliasedDirective() {}
    AliasedDirective = tslib_1.__decorate([
        Directive({ selector: '[someDirective]' }),
    ], AliasedDirective);
    return AliasedDirective;
}());
```

This commit fixes the problem by not relying on comparing names, but
instead finding the declaration and matching it with both the outer
and inner declaration.

PR Close #33878
This commit is contained in:
JoostK 2019-11-16 23:57:57 +01:00 committed by Alex Rickabaugh
parent 19a6c158d2
commit e666d283dd
4 changed files with 119 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
packages/compiler-cli/ngcc/src/host/esm2015_host.ts
View File

@ -931,8 +931,21 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
// has to be checked to actually correspond with the class symbol.
const helperCalls = this.getHelperCallsForClass(classSymbol, ['__decorate']);
const outerDeclaration = classSymbol.declaration.valueDeclaration;
const innerDeclaration = classSymbol.implementation.valueDeclaration;
const matchesClass = (identifier: ts.Identifier) => {
const decl = this.getDeclarationOfIdentifier(identifier);
if (decl === null) {
return false;
}
// The identifier corresponds with the class if its declaration is either the outer or inner
// declaration.
return decl.node === outerDeclaration || decl.node === innerDeclaration;
};
for (const helperCall of helperCalls) {
if (isClassDecorateCall(helperCall, classSymbol.name)) {
if (isClassDecorateCall(helperCall, matchesClass)) {
// This `__decorate` call is targeting the class itself.
const helperArgs = helperCall.arguments[0];
@ -959,7 +972,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
(type, index) => getConstructorParamInfo(index).typeExpression = type);
}
}
} else if (isMemberDecorateCall(helperCall, classSymbol.name)) {
} else if (isMemberDecorateCall(helperCall, matchesClass)) {
// The `__decorate` call is targeting a member of the class
const helperArgs = helperCall.arguments[0];
const memberName = helperCall.arguments[2].text;
@ -1688,9 +1701,10 @@ export function isAssignment(node: ts.Node): node is ts.AssignmentExpression<ts.
* ```
*
* @param call the call expression that is tested to represent a class decorator call.
* @param className the name of the class that the call needs to correspond with.
* @param matches predicate function to test whether the call is associated with the desired class.
*/
export function isClassDecorateCall(call: ts.CallExpression, className: string):
export function isClassDecorateCall(
call: ts.CallExpression, matches: (identifier: ts.Identifier) => boolean):
call is ts.CallExpression&{arguments: [ts.ArrayLiteralExpression, ts.Expression]} {
const helperArgs = call.arguments[0];
if (helperArgs === undefined || !ts.isArrayLiteralExpression(helperArgs)) {
@ -1698,7 +1712,7 @@ export function isClassDecorateCall(call: ts.CallExpression, className: string):
}
const target = call.arguments[1];
return target !== undefined && ts.isIdentifier(target) && target.text === className;
return target !== undefined && ts.isIdentifier(target) && matches(target);
}
/**
@ -1710,9 +1724,10 @@ export function isClassDecorateCall(call: ts.CallExpression, className: string):
* ```
*
* @param call the call expression that is tested to represent a member decorator call.
* @param className the name of the class that the call needs to correspond with.
* @param matches predicate function to test whether the call is associated with the desired class.
*/
export function isMemberDecorateCall(call: ts.CallExpression, className: string):
export function isMemberDecorateCall(
call: ts.CallExpression, matches: (identifier: ts.Identifier) => boolean):
call is ts.CallExpression&
{arguments: [ts.ArrayLiteralExpression, ts.StringLiteral, ts.StringLiteral]} {
const helperArgs = call.arguments[0];
@ -1722,7 +1737,7 @@ export function isMemberDecorateCall(call: ts.CallExpression, className: string)
const target = call.arguments[1];
if (target === undefined || !ts.isPropertyAccessExpression(target) ||
!ts.isIdentifier(target.expression) || target.expression.text !== className ||
!ts.isIdentifier(target.expression) || !matches(target.expression) ||
target.name.text !== 'prototype') {
return false;
}

30
packages/compiler-cli/ngcc/test/host/commonjs_host_import_helper_spec.ts
View File

@ -68,6 +68,15 @@ __decorate([
core.Input(),
], OtherDirective.prototype, "input2", void 0);
exports.OtherDirective = OtherDirective;
var AliasedDirective$1 = (function () {
function AliasedDirective() {}
return AliasedDirective;
}());
AliasedDirective$1 = __decorate([
core.Directive({ selector: '[someDirective]' }),
], AliasedDirective$1);
exports.AliasedDirective$1 = AliasedDirective$1;
`
};
});
@ -92,6 +101,27 @@ exports.OtherDirective = OtherDirective;
'{ selector: \'[someDirective]\' }',
]);
});
it('should find the decorators on an aliased class at the top level', () => {
loadFakeCore(getFileSystem());
loadTestFiles([TOPLEVEL_DECORATORS_FILE]);
const {program, host: compilerHost} = makeTestBundleProgram(TOPLEVEL_DECORATORS_FILE.name);
const host = new CommonJsReflectionHost(new MockLogger(), false, program, compilerHost);
const classNode = getDeclaration(
program, TOPLEVEL_DECORATORS_FILE.name, 'AliasedDirective$1',
isNamedVariableDeclaration);
const decorators = host.getDecoratorsOfDeclaration(classNode) !;
expect(decorators.length).toEqual(1);
const decorator = decorators[0];
expect(decorator.name).toEqual('Directive');
expect(decorator.identifier !.getText()).toEqual('core.Directive');
expect(decorator.import).toEqual({name: 'Directive', from: '@angular/core'});
expect(decorator.args !.map(arg => arg.getText())).toEqual([
'{ selector: \'[someDirective]\' }',
]);
});
});
});
});

39
packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts
View File

@ -174,12 +174,27 @@ import { Directive } from '@angular/core';
// Note that the IIFE is not in parentheses
var SomeDirective = function () {
function SomeDirective() {}
// Note that the decorator is combined with the return statment
// Note that the decorator is combined with the return statement
return SomeDirective = tslib_1.__decorate([
Directive({ selector: '[someDirective]' }),
], SomeDirective);
}());
}();
export { SomeDirective };
`,
},
{
name: _('/some_aliased_directive.js'),
contents: `
import * as tslib_1 from 'tslib';
import { Directive } from '@angular/core';
var AliasedDirective$1 = /** @class */ (function () {
function AliasedDirective() {}
AliasedDirective = tslib_1.__decorate([
Directive({ selector: '[someDirective]' }),
], AliasedDirective);
return AliasedDirective;
}());
export { AliasedDirective$1 };
`,
},
];
@ -247,6 +262,26 @@ export { SomeDirective };
});
it('should find the decorators on an aliased class', () => {
const {program} = makeTestBundleProgram(_('/some_aliased_directive.js'));
const host = new Esm5ReflectionHost(new MockLogger(), false, program.getTypeChecker());
const classNode = getDeclaration(
program, _('/some_aliased_directive.js'), 'AliasedDirective$1',
isNamedVariableDeclaration);
const decorators = host.getDecoratorsOfDeclaration(classNode) !;
expect(decorators).toBeDefined();
expect(decorators.length).toEqual(1);
const decorator = decorators[0];
expect(decorator.name).toEqual('Directive');
expect(decorator.identifier !.getText()).toEqual('Directive');
expect(decorator.import).toEqual({name: 'Directive', from: '@angular/core'});
expect(decorator.args !.map(arg => arg.getText())).toEqual([
'{ selector: \'[someDirective]\' }',
]);
});
it('should find the decorators on a class when mixing `ctorParameters` and `__decorate`',
() => {
const {program} = makeTestBundleProgram(_('/some_directive_ctor_parameters.js'));

29
packages/compiler-cli/ngcc/test/host/umd_host_import_helper_spec.ts
View File

@ -61,6 +61,15 @@ runInEachFileSystem(() => {
return SomeDirective;
}());
exports.SomeDirective = SomeDirective;
var AliasedDirective$1 = /** @class */ (function () {
function AliasedDirective() {}
AliasedDirective = __decorate([
core.Directive({ selector: '[someDirective]' }),
], AliasedDirective);
return AliasedDirective;
}());
exports.AliasedDirective$1 = AliasedDirective$1;
})));`,
};
});
@ -85,6 +94,26 @@ runInEachFileSystem(() => {
'{ selector: \'[someDirective]\' }',
]);
});
it('should find the decorators on an aliased class', () => {
loadTestFiles([SOME_DIRECTIVE_FILE]);
const {program, host: compilerHost} = makeTestBundleProgram(SOME_DIRECTIVE_FILE.name);
const host = new UmdReflectionHost(new MockLogger(), false, program, compilerHost);
const classNode = getDeclaration(
program, SOME_DIRECTIVE_FILE.name, 'AliasedDirective$1', isNamedVariableDeclaration);
const decorators = host.getDecoratorsOfDeclaration(classNode) !;
expect(decorators).toBeDefined();
expect(decorators.length).toEqual(1);
const decorator = decorators[0];
expect(decorator.name).toEqual('Directive');
expect(decorator.identifier !.getText()).toEqual('core.Directive');
expect(decorator.import).toEqual({name: 'Directive', from: '@angular/core'});
expect(decorator.args !.map(arg => arg.getText())).toEqual([
'{ selector: \'[someDirective]\' }',
]);
});
});
describe('getMembersOfClass()', () => {