fix(ngcc): handle presence of both `ctorParameters` and `__decorate` (#32901)

Recently ng-packagr was updated to include a transform that used to be
done in tsickle (https://github.com/ng-packagr/ng-packagr/pull/1401),
where only constructor parameter decorators are emitted in tsickle's
format, not any of the other decorators.

ngcc used to extract decorators from only a single format, so once it
saw the `ctorParameters` static property it assumed the library is using
the tsickle format. Therefore, none of the `__decorate` calls were
considered. This resulted in missing decorator information, preventing
proper processing of a package.

This commit changes how decorators are extracted by always looking at
both the static properties and the `__decorate` calls, merging these
sources appropriately.

Resolves FW-1573

PR Close #32901
This commit is contained in:
JoostK 2019-09-29 21:17:38 +02:00 committed by atscott
parent 002a97d852
commit 747f0cff9e
3 changed files with 200 additions and 18 deletions

View File

@ -655,12 +655,16 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
return this.decoratorCache.get(decl) !;
}
// First attempt extracting decorators from static properties.
let decoratorInfo = this.computeDecoratorInfoFromStaticProperties(classSymbol);
if (decoratorInfo === null) {
// If none were present, use the `__decorate` helper calls instead.
decoratorInfo = this.computeDecoratorInfoFromHelperCalls(classSymbol);
}
// Extract decorators from static properties and `__decorate` helper calls, then merge them
// together where the information from the static properties is preferred.
const staticProps = this.computeDecoratorInfoFromStaticProperties(classSymbol);
const helperCalls = this.computeDecoratorInfoFromHelperCalls(classSymbol);
const decoratorInfo: DecoratorInfo = {
classDecorators: staticProps.classDecorators || helperCalls.classDecorators,
memberDecorators: staticProps.memberDecorators || helperCalls.memberDecorators,
constructorParamInfo: staticProps.constructorParamInfo || helperCalls.constructorParamInfo,
};
this.decoratorCache.set(decl, decoratorInfo);
return decoratorInfo;
@ -676,8 +680,10 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
* @returns All information on the decorators as extracted from static properties, or `null` if
* none of the static properties exist.
*/
protected computeDecoratorInfoFromStaticProperties(classSymbol: NgccClassSymbol): DecoratorInfo
|null {
protected computeDecoratorInfoFromStaticProperties(classSymbol: NgccClassSymbol): {
classDecorators: Decorator[] | null; memberDecorators: Map<string, Decorator[]>| null;
constructorParamInfo: ParamInfo[] | null;
} {
let classDecorators: Decorator[]|null = null;
let memberDecorators: Map<string, Decorator[]>|null = null;
let constructorParamInfo: ParamInfo[]|null = null;
@ -697,16 +703,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
constructorParamInfo = this.getParamInfoFromStaticProperty(constructorParamsProperty);
}
// If none of the static properties were present, no decorator info could be computed.
if (classDecorators === null && memberDecorators === null && constructorParamInfo === null) {
return null;
}
return {
classDecorators,
memberDecorators: memberDecorators || new Map<string, Decorator[]>(),
constructorParamInfo: constructorParamInfo || [],
};
return {classDecorators, memberDecorators, constructorParamInfo};
}
/**

View File

@ -62,6 +62,36 @@ runInEachFileSystem(() => {
TemplateRef, String])
], SomeDirective);
export { SomeDirective };
`,
},
{
name: _('/some_directive_ctor_parameters.js'),
contents: `
import * as tslib_1 from 'tslib';
import { Directive, Inject, InjectionToken, Input } from '@angular/core';
const INJECTED_TOKEN = new InjectionToken('injected');
class ViewContainerRef {
}
class TemplateRef {
}
let SomeDirective = class SomeDirective {
constructor(_viewContainer, _template, injected) {
this.input1 = '';
}
};
SomeDirective.ctorParameters = () => [
{ type: ViewContainerRef, },
{ type: TemplateRef, },
{ type: undefined, decorators: [{ type: Inject, args: [INJECTED_TOKEN,] },] },
];
tslib_1.__decorate([
Input(),
], SomeDirective.prototype, "input1", void 0);
SomeDirective = tslib_1.__decorate([
Directive({ selector: '[someDirective]' }),
tslib_1.__param(2, Inject(INJECTED_TOKEN)),
], SomeDirective);
export { SomeDirective };
`,
},
{
@ -152,6 +182,28 @@ runInEachFileSystem(() => {
]);
});
it('should find the decorators on a class when mixing `ctorParameters` and `__decorate`',
() => {
const {program} = makeTestBundleProgram(_('/some_directive_ctor_parameters.js'));
const host =
new Esm2015ReflectionHost(new MockLogger(), false, program.getTypeChecker());
const classNode = getDeclaration(
program, _('/some_directive_ctor_parameters.js'), 'SomeDirective',
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 support decorators being used inside @angular/core', () => {
const {program} =
makeTestBundleProgram(_('/node_modules/@angular/core/some_directive.js'));
@ -195,6 +247,22 @@ runInEachFileSystem(() => {
expect(input1.decorators !.map(d => d.name)).toEqual(['Input']);
});
it('should find decorated members on a class when mixing `ctorParameters` and `__decorate`',
() => {
const {program} = makeTestBundleProgram(_('/some_directive_ctor_parameters.js'));
const host =
new Esm2015ReflectionHost(new MockLogger(), false, program.getTypeChecker());
const classNode = getDeclaration(
program, _('/some_directive_ctor_parameters.js'), 'SomeDirective',
isNamedVariableDeclaration);
const members = host.getMembersOfClass(classNode);
const input1 = members.find(member => member.name === 'input1') !;
expect(input1.kind).toEqual(ClassMemberKind.Property);
expect(input1.isStatic).toEqual(false);
expect(input1.decorators !.map(d => d.name)).toEqual(['Input']);
});
it('should find non decorated properties on a class', () => {
const {program} = makeTestBundleProgram(_('/some_directive.js'));
const host =
@ -292,6 +360,27 @@ runInEachFileSystem(() => {
]);
});
it('should find the decorated constructor parameters when mixing `ctorParameters` and `__decorate`',
() => {
const {program} = makeTestBundleProgram(_('/some_directive_ctor_parameters.js'));
const host =
new Esm2015ReflectionHost(new MockLogger(), false, program.getTypeChecker());
const classNode = getDeclaration(
program, _('/some_directive_ctor_parameters.js'), 'SomeDirective',
isNamedVariableDeclaration);
const parameters = host.getConstructorParameters(classNode);
expect(parameters).toBeDefined();
expect(parameters !.map(parameter => parameter.name)).toEqual([
'_viewContainer', '_template', 'injected'
]);
expectTypeValueReferencesForParameters(parameters !, [
'ViewContainerRef',
'TemplateRef',
null,
]);
});
describe('(returned parameters `decorators`)', () => {
it('should use `getImportOfIdentifier()` to retrieve import info', () => {
const mockImportInfo = {} as Import;

View File

@ -76,6 +76,43 @@ runInEachFileSystem(() => {
return SomeDirective;
}());
export { SomeDirective };
`,
},
{
name: _('/some_directive_ctor_parameters.js'),
contents: `
import * as tslib_1 from 'tslib';
import { Directive, Inject, InjectionToken, Input } from '@angular/core';
var INJECTED_TOKEN = new InjectionToken('injected');
var ViewContainerRef = /** @class */ (function () {
function ViewContainerRef() {
}
return ViewContainerRef;
}());
var TemplateRef = /** @class */ (function () {
function TemplateRef() {
}
return TemplateRef;
}());
var SomeDirective = /** @class */ (function () {
function SomeDirective(_viewContainer, _template, injected) {
this.input1 = '';
}
SomeDirective.ctorParameters = function() { return [
{ type: ViewContainerRef, },
{ type: TemplateRef, },
{ type: undefined, decorators: [{ type: Inject, args: [INJECTED_TOKEN,] },] },
]; };
tslib_1.__decorate([
Input(),
], SomeDirective.prototype, "input1", void 0);
SomeDirective = tslib_1.__decorate([
Directive({ selector: '[someDirective]' }),
tslib_1.__param(2, Inject(INJECTED_TOKEN)),
], SomeDirective);
return SomeDirective;
}());
export { SomeDirective };
`,
},
{
@ -172,6 +209,28 @@ export { SomeDirective };
]);
});
it('should find the decorators on a class when mixing `ctorParameters` and `__decorate`',
() => {
const {program} = makeTestBundleProgram(_('/some_directive_ctor_parameters.js'));
const host =
new Esm5ReflectionHost(new MockLogger(), false, program.getTypeChecker());
const classNode = getDeclaration(
program, _('/some_directive_ctor_parameters.js'), 'SomeDirective',
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 support decorators being used inside @angular/core', () => {
const {program} =
makeTestBundleProgram(_('/node_modules/@angular/core/some_directive.js'));
@ -213,6 +272,22 @@ export { SomeDirective };
expect(input1.decorators !.map(d => d.name)).toEqual(['Input']);
});
it('should find decorated members on a class when mixing `ctorParameters` and `__decorate`',
() => {
const {program} = makeTestBundleProgram(_('/some_directive_ctor_parameters.js'));
const host =
new Esm5ReflectionHost(new MockLogger(), false, program.getTypeChecker());
const classNode = getDeclaration(
program, _('/some_directive_ctor_parameters.js'), 'SomeDirective',
isNamedVariableDeclaration);
const members = host.getMembersOfClass(classNode);
const input1 = members.find(member => member.name === 'input1') !;
expect(input1.kind).toEqual(ClassMemberKind.Property);
expect(input1.isStatic).toEqual(false);
expect(input1.decorators !.map(d => d.name)).toEqual(['Input']);
});
it('should find non decorated properties on a class', () => {
const {program} = makeTestBundleProgram(_('/some_directive.js'));
const host = new Esm5ReflectionHost(new MockLogger(), false, program.getTypeChecker());
@ -288,6 +363,27 @@ export { SomeDirective };
]);
});
it('should find the decorated constructor parameters when mixing `ctorParameters` and `__decorate`',
() => {
const {program} = makeTestBundleProgram(_('/some_directive_ctor_parameters.js'));
const host =
new Esm5ReflectionHost(new MockLogger(), false, program.getTypeChecker());
const classNode = getDeclaration(
program, _('/some_directive_ctor_parameters.js'), 'SomeDirective',
isNamedVariableDeclaration);
const parameters = host.getConstructorParameters(classNode);
expect(parameters).toBeDefined();
expect(parameters !.map(parameter => parameter.name)).toEqual([
'_viewContainer', '_template', 'injected'
]);
expectTypeValueReferencesForParameters(parameters !, [
'ViewContainerRef',
'TemplateRef',
null,
]);
});
describe('(returned parameters `decorators`)', () => {
it('should have import information on decorators', () => {
const {program} = makeTestBundleProgram(_('/some_directive.js'));