diff --git a/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts b/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts index 7d9c9b5c45..7ddb1005bf 100644 --- a/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts +++ b/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts @@ -103,9 +103,9 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N if (symbol.members) { symbol.members.forEach((value, key) => { const decorators = removeFromMap(decoratorsMap, key); - const member = this.reflectMember(value, decorators); - if (member) { - members.push(member); + const reflectedMembers = this.reflectMembers(value, decorators); + if (reflectedMembers) { + members.push(...reflectedMembers); } }); } @@ -114,9 +114,9 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N if (symbol.exports) { symbol.exports.forEach((value, key) => { const decorators = removeFromMap(decoratorsMap, key); - const member = this.reflectMember(value, decorators, true); - if (member) { - members.push(member); + const reflectedMembers = this.reflectMembers(value, decorators, true); + if (reflectedMembers) { + members.push(...reflectedMembers); } }); } @@ -135,9 +135,9 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N if (variableSymbol && variableSymbol.exports) { variableSymbol.exports.forEach((value, key) => { const decorators = removeFromMap(decoratorsMap, key); - const member = this.reflectMember(value, decorators, true); - if (member) { - members.push(member); + const reflectedMembers = this.reflectMembers(value, decorators, true); + if (reflectedMembers) { + members.push(...reflectedMembers); } }); } @@ -724,8 +724,8 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N * @param isStatic true if this member is static, false if it is an instance property. * @returns the reflected member information, or null if the symbol is not a member. */ - protected reflectMember(symbol: ts.Symbol, decorators?: Decorator[], isStatic?: boolean): - ClassMember|null { + protected reflectMembers(symbol: ts.Symbol, decorators?: Decorator[], isStatic?: boolean): + ClassMember[]|null { let kind: ClassMemberKind|null = null; let value: ts.Expression|null = null; let name: string|null = null; @@ -783,11 +783,11 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N } const type: ts.TypeNode = (node as any).type || null; - return { + return [{ node, implementation: node, kind, type, name, nameNode, value, isStatic, decorators: decorators || [] - }; + }]; } /** diff --git a/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts b/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts index db04510eaa..8079e290c3 100644 --- a/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts +++ b/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts @@ -208,24 +208,69 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost { /** * Reflect over a symbol and extract the member information, combining it with the * provided decorator information, and whether it is a static member. + * + * If a class member uses accessors (e.g getters and/or setters) then it gets downleveled + * in ES5 to a single `Object.defineProperty()` call. In that case we must parse this + * call to extract the one or two ClassMember objects that represent the accessors. + * * @param symbol the symbol for the member to reflect over. * @param decorators an array of decorators associated with the member. * @param isStatic true if this member is static, false if it is an instance property. * @returns the reflected member information, or null if the symbol is not a member. */ - protected reflectMember(symbol: ts.Symbol, decorators?: Decorator[], isStatic?: boolean): - ClassMember|null { - const member = super.reflectMember(symbol, decorators, isStatic); - if (member && member.kind === ClassMemberKind.Method && member.isStatic && member.node && - ts.isPropertyAccessExpression(member.node) && member.node.parent && - ts.isBinaryExpression(member.node.parent) && - ts.isFunctionExpression(member.node.parent.right)) { - // Recompute the implementation for this member: - // ES5 static methods are variable declarations so the declaration is actually the - // initializer of the variable assignment - member.implementation = member.node.parent.right; + protected reflectMembers(symbol: ts.Symbol, decorators?: Decorator[], isStatic?: boolean): + ClassMember[]|null { + const node = symbol.valueDeclaration || symbol.declarations && symbol.declarations[0]; + const propertyDefinition = getPropertyDefinition(node); + if (propertyDefinition) { + const members: ClassMember[] = []; + if (propertyDefinition.setter) { + members.push({ + node, + implementation: propertyDefinition.setter, + kind: ClassMemberKind.Setter, + type: null, + name: symbol.name, + nameNode: null, + value: null, + isStatic: isStatic || false, + decorators: decorators || [], + }); + + // Prevent attaching the decorators to a potential getter. In ES5, we can't tell where the + // decorators were originally attached to, however we only want to attach them to a single + // `ClassMember` as otherwise ngtsc would handle the same decorators twice. + decorators = undefined; + } + if (propertyDefinition.getter) { + members.push({ + node, + implementation: propertyDefinition.getter, + kind: ClassMemberKind.Getter, + type: null, + name: symbol.name, + nameNode: null, + value: null, + isStatic: isStatic || false, + decorators: decorators || [], + }); + } + return members; } - return member; + + const members = super.reflectMembers(symbol, decorators, isStatic); + members && members.forEach(member => { + if (member && member.kind === ClassMemberKind.Method && member.isStatic && member.node && + ts.isPropertyAccessExpression(member.node) && member.node.parent && + ts.isBinaryExpression(member.node.parent) && + ts.isFunctionExpression(member.node.parent.right)) { + // Recompute the implementation for this member: + // ES5 static methods are variable declarations so the declaration is actually the + // initializer of the variable assignment + member.implementation = member.node.parent.right; + } + }); + return members; } /** @@ -246,6 +291,62 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost { ///////////// Internal Helpers ///////////// +/** + * Represents the details about property definitions that were set using `Object.defineProperty`. + */ +interface PropertyDefinition { + setter: ts.FunctionExpression|null; + getter: ts.FunctionExpression|null; +} + +/** + * In ES5, getters and setters have been downleveled into call expressions of + * `Object.defineProperty`, such as + * + * ``` + * Object.defineProperty(Clazz.prototype, "property", { + * get: function () { + * return 'value'; + * }, + * set: function (value) { + * this.value = value; + * }, + * enumerable: true, + * configurable: true + * }); + * ``` + * + * This function inspects the given node to determine if it corresponds with such a call, and if so + * extracts the `set` and `get` function expressions from the descriptor object, if they exist. + * + * @param node The node to obtain the property definition from. + * @returns The property definition if the node corresponds with accessor, null otherwise. + */ +function getPropertyDefinition(node: ts.Node): PropertyDefinition|null { + if (!ts.isCallExpression(node)) return null; + + const fn = node.expression; + if (!ts.isPropertyAccessExpression(fn) || !ts.isIdentifier(fn.expression) || + fn.expression.text !== 'Object' || fn.name.text !== 'defineProperty') + return null; + + const descriptor = node.arguments[2]; + if (!descriptor || !ts.isObjectLiteralExpression(descriptor)) return null; + + return { + setter: readPropertyFunctionExpression(descriptor, 'set'), + getter: readPropertyFunctionExpression(descriptor, 'get'), + }; +} + +function readPropertyFunctionExpression(object: ts.ObjectLiteralExpression, name: string) { + const property = object.properties.find( + (p): p is ts.PropertyAssignment => + ts.isPropertyAssignment(p) && ts.isIdentifier(p.name) && p.name.text === name); + + return property && ts.isFunctionExpression(property.initializer) && property.initializer || null; +} + function getIifeBody(declaration: ts.VariableDeclaration): ts.Block|undefined { if (!declaration.initializer || !ts.isParenthesizedExpression(declaration.initializer)) { return undefined; diff --git a/packages/compiler-cli/src/ngcc/test/host/esm5_host_spec.ts b/packages/compiler-cli/src/ngcc/test/host/esm5_host_spec.ts index 1501869fc9..6a3cfcd69c 100644 --- a/packages/compiler-cli/src/ngcc/test/host/esm5_host_spec.ts +++ b/packages/compiler-cli/src/ngcc/test/host/esm5_host_spec.ts @@ -47,6 +47,52 @@ const SOME_DIRECTIVE_FILE = { }()); `, }; +const ACCESSORS_FILE = { + name: '/accessors.js', + contents: ` + import { Directive, Input, Output } from '@angular/core'; + + var SomeDirective = (function() { + function SomeDirective() { + } + Object.defineProperty(SomeDirective.prototype, "setter", { + set: function (value) { this.value = value; }, + enumerable: true, + configurable: true + }); + Object.defineProperty(SomeDirective.prototype, "getter", { + get: function () { return null; }, + enumerable: true, + configurable: true + }); + Object.defineProperty(SomeDirective.prototype, "setterAndGetter", { + get: function () { return null; }, + set: function (value) { this.value = value; }, + enumerable: true, + configurable: true + }); + Object.defineProperty(SomeDirective, "staticSetter", { + set: function (value) { this.value = value; }, + enumerable: true, + configurable: true + }); + Object.defineProperty(SomeDirective.prototype, "none", { + enumerable: true, + configurable: true + }); + Object.defineProperty(SomeDirective.prototype, "incomplete"); + SomeDirective.decorators = [ + { type: Directive, args: [{ selector: '[someDirective]' },] } + ]; + SomeDirective.propDecorators = { + "setter": [{ type: Input },], + "getter": [{ type: Output },], + "setterAndGetter": [{ type: Input },], + }; + return SomeDirective; + }()); + `, +}; const SIMPLE_CLASS_FILE = { name: '/simple_class.js', @@ -627,7 +673,54 @@ describe('Esm5ReflectionHost', () => { const input2 = members.find(member => member.name === 'input2') !; expect(input2.kind).toEqual(ClassMemberKind.Property); expect(input2.isStatic).toEqual(false); - expect(input1.decorators !.map(d => d.name)).toEqual(['Input']); + expect(input2.decorators !.map(d => d.name)).toEqual(['Input']); + }); + + it('should find Object.defineProperty members on a class', () => { + const program = makeTestProgram(ACCESSORS_FILE); + const host = new Esm5ReflectionHost(false, program.getTypeChecker()); + const classNode = + getDeclaration(program, ACCESSORS_FILE.name, 'SomeDirective', ts.isVariableDeclaration); + const members = host.getMembersOfClass(classNode); + + const setter = members.find(member => member.name === 'setter') !; + expect(setter.kind).toEqual(ClassMemberKind.Setter); + expect(setter.isStatic).toEqual(false); + expect(setter.value).toBeNull(); + expect(setter.decorators !.map(d => d.name)).toEqual(['Input']); + expect(ts.isFunctionExpression(setter.implementation !)).toEqual(true); + expect((setter.implementation as ts.FunctionExpression).body.statements[0].getText()) + .toEqual('this.value = value;'); + + const getter = members.find(member => member.name === 'getter') !; + expect(getter.kind).toEqual(ClassMemberKind.Getter); + expect(getter.isStatic).toEqual(false); + expect(getter.value).toBeNull(); + expect(getter.decorators !.map(d => d.name)).toEqual(['Output']); + expect(ts.isFunctionExpression(getter.implementation !)).toEqual(true); + expect((getter.implementation as ts.FunctionExpression).body.statements[0].getText()) + .toEqual('return null;'); + + const [combinedSetter, combinedGetter] = + members.filter(member => member.name === 'setterAndGetter'); + expect(combinedSetter.kind).toEqual(ClassMemberKind.Setter); + expect(combinedSetter.isStatic).toEqual(false); + expect(combinedSetter.decorators !.map(d => d.name)).toEqual(['Input']); + expect(combinedGetter.kind).toEqual(ClassMemberKind.Getter); + expect(combinedGetter.isStatic).toEqual(false); + expect(combinedGetter.decorators !.map(d => d.name)).toEqual([]); + + const staticSetter = members.find(member => member.name === 'staticSetter') !; + expect(staticSetter.kind).toEqual(ClassMemberKind.Setter); + expect(staticSetter.isStatic).toEqual(true); + expect(staticSetter.value).toBeNull(); + expect(staticSetter.decorators !.map(d => d.name)).toEqual([]); + + const none = members.find(member => member.name === 'none'); + expect(none).toBeUndefined(); + + const incomplete = members.find(member => member.name === 'incomplete'); + expect(incomplete).toBeUndefined(); }); it('should find non decorated properties on a class', () => {