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

fix(ivy): ngcc - handle accessor pairs in ES2015 (#28357) 

ngcc's reflection host needs to be able to determine all members of a
class, which it does by using the `ts.Symbol` from TypeScript's
TypeChecker.  Such Symbol however may represent multiple class members
in the case of accessors; an equally named getter/setter accessor pair
is combined into a single `ts.Symbol`.

This commit introduces logic to recognize such accessors in order for
both the getter and setter to be considered as class member, similar to
ngtsc's behavior when operating on original TypeScript code.

One difference wrt the TypeScript host is that ngcc cannot see to which
accessor originally had any decorators applied to them, as decorators
are applied to the property descriptor in general, not a specific accessor.
If an accessor has both a setter and getter, any decorators are only
attached to the setter member.

PR Close #28357
This commit is contained in:
JoostK 2019-01-27 17:21:29 +01:00 committed by Jason Aden
parent adfc55e2c3
commit f8c70011b1
3 changed files with 116 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -102,9 +102,10 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
// that are initialized in the class.
if (symbol.members) {
symbol.members.forEach((value, key) => {
const decorators = removeFromMap(decoratorsMap, key);
const decorators = decoratorsMap.get(key as string);
const reflectedMembers = this.reflectMembers(value, decorators);
if (reflectedMembers) {
decoratorsMap.delete(key as string);
members.push(...reflectedMembers);
}
});
@ -113,9 +114,10 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
// The static property map contains all the static properties
if (symbol.exports) {
symbol.exports.forEach((value, key) => {
const decorators = removeFromMap(decoratorsMap, key);
const decorators = decoratorsMap.get(key as string);
const reflectedMembers = this.reflectMembers(value, decorators, true);
if (reflectedMembers) {
decoratorsMap.delete(key as string);
members.push(...reflectedMembers);
}
});
@ -134,9 +136,10 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
const variableSymbol = this.checker.getSymbolAtLocation(symbol.valueDeclaration.parent.name);
if (variableSymbol && variableSymbol.exports) {
variableSymbol.exports.forEach((value, key) => {
const decorators = removeFromMap(decoratorsMap, key);
const decorators = decoratorsMap.get(key as string);
const reflectedMembers = this.reflectMembers(value, decorators, true);
if (reflectedMembers) {
decoratorsMap.delete(key as string);
members.push(...reflectedMembers);
}
});
@ -719,6 +722,18 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
/**
* Reflect over a symbol and extract the member information, combining it with the
* provided decorator information, and whether it is a static member.
*
* A single symbol may represent multiple class members in the case of accessors;
* an equally named getter/setter accessor pair is combined into a single symbol.
* When the symbol is recognized as representing an accessor, its declarations are
* analyzed such that both the setter and getter accessor are returned as separate
* class members.
*
* One difference wrt the TypeScript host is that in ES2015, we cannot see which
* accessor originally had any decorators applied to them, as decorators are applied
* to the property descriptor in general, not a specific accessor. If an accessor
* has both a setter and getter, any decorators are only attached to the setter member.
*
* @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.
@ -726,30 +741,74 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
*/
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;
let nameNode: ts.Identifier|null = null;
if (symbol.flags & ts.SymbolFlags.Accessor) {
const members: ClassMember[] = [];
const setter = symbol.declarations && symbol.declarations.find(ts.isSetAccessor);
const getter = symbol.declarations && symbol.declarations.find(ts.isGetAccessor);
const setterMember =
setter && this.reflectMember(setter, ClassMemberKind.Setter, decorators, isStatic);
if (setterMember) {
members.push(setterMember);
const node = symbol.valueDeclaration || symbol.declarations && symbol.declarations[0];
if (!node || !isClassMemberType(node)) {
return null;
// Prevent attaching the decorators to a potential getter. In ES2015, 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;
}
const getterMember =
getter && this.reflectMember(getter, ClassMemberKind.Getter, decorators, isStatic);
if (getterMember) {
members.push(getterMember);
}
return members;
}
let kind: ClassMemberKind|null = null;
if (symbol.flags & ts.SymbolFlags.Method) {
kind = ClassMemberKind.Method;
} else if (symbol.flags & ts.SymbolFlags.Property) {
kind = ClassMemberKind.Property;
} else if (symbol.flags & ts.SymbolFlags.GetAccessor) {
kind = ClassMemberKind.Getter;
} else if (symbol.flags & ts.SymbolFlags.SetAccessor) {
kind = ClassMemberKind.Setter;
}
const node = symbol.valueDeclaration || symbol.declarations && symbol.declarations[0];
if (!node) {
return null;
}
const member = this.reflectMember(node, kind, decorators, isStatic);
if (!member) {
return null;
}
return [member];
}
/**
* Reflect over a symbol and extract the member information, combining it with the
* provided decorator information, and whether it is a static member.
* @param node the declaration node for the member to reflect over.
* @param kind the assumed kind of the member, may become more accurate during reflection.
* @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(
node: ts.Declaration, kind: ClassMemberKind|null, decorators?: Decorator[],
isStatic?: boolean): ClassMember|null {
let value: ts.Expression|null = null;
let name: string|null = null;
let nameNode: ts.Identifier|null = null;
if (!isClassMemberType(node)) {
return null;
}
if (isStatic && isPropertyAccess(node)) {
name = node.name.text;
value = symbol.flags & ts.SymbolFlags.Property ? node.parent.right : null;
value = kind === ClassMemberKind.Property ? node.parent.right : null;
} else if (isThisAssignment(node)) {
kind = ClassMemberKind.Property;
name = node.left.name.text;
@ -783,11 +842,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 || []
}];
};
}
/**
@ -1169,15 +1228,6 @@ function getDecoratorArgs(node: ts.ObjectLiteralExpression): ts.Expression[] {
[];
}
function removeFromMap<T>(map: Map<string, T>, key: ts.__String): T|undefined {
const mapKey = key as string;
const value = map.get(mapKey);
if (value !== undefined) {
map.delete(mapKey);
}
return value;
}
function isPropertyAccess(node: ts.Node): node is ts.PropertyAccessExpression&
{parent: ts.BinaryExpression} {
return !!node.parent && ts.isBinaryExpression(node.parent) && ts.isPropertyAccessExpression(node);

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

@ -52,6 +52,24 @@ const SOME_DIRECTIVE_FILE = {
`,
};
const ACCESSORS_FILE = {
name: '/accessors.js',
contents: `
import { Directive, Input, Output } from '@angular/core';
class SomeDirective {
set setterAndGetter(value) { this.value = value; }
get setterAndGetter() { return null; }
}
SomeDirective.decorators = [
{ type: Directive, args: [{ selector: '[someDirective]' },] }
];
SomeDirective.propDecorators = {
"setterAndGetter": [{ type: Input },],
};
`,
};
const SIMPLE_CLASS_FILE = {
name: '/simple_class.js',
contents: `
@ -705,6 +723,27 @@ describe('Fesm2015ReflectionHost', () => {
expect(instanceProperty.value !.getText()).toEqual(`'instance'`);
});
it('should handle equally named getter/setter pairs correctly', () => {
const program = makeTestProgram(ACCESSORS_FILE);
const host = new Esm2015ReflectionHost(false, program.getTypeChecker());
const classNode =
getDeclaration(program, ACCESSORS_FILE.name, 'SomeDirective', ts.isClassDeclaration);
const members = host.getMembersOfClass(classNode);
const [combinedSetter, combinedGetter] =
members.filter(member => member.name === 'setterAndGetter');
expect(combinedSetter.kind).toEqual(ClassMemberKind.Setter);
expect(combinedSetter.isStatic).toEqual(false);
expect(ts.isSetAccessor(combinedSetter.implementation !)).toEqual(true);
expect(combinedSetter.value).toBeNull();
expect(combinedSetter.decorators !.map(d => d.name)).toEqual(['Input']);
expect(combinedGetter.kind).toEqual(ClassMemberKind.Getter);
expect(combinedGetter.isStatic).toEqual(false);
expect(ts.isGetAccessor(combinedGetter.implementation !)).toEqual(true);
expect(combinedGetter.value).toBeNull();
expect(combinedGetter.decorators !.map(d => d.name)).toEqual([]);
});
it('should find static methods on a class', () => {
const program = makeTestProgram(SOME_DIRECTIVE_FILE);
const host = new Esm2015ReflectionHost(false, program.getTypeChecker());

1
packages/compiler-cli/src/ngcc/test/host/esm5_host_spec.ts
View File

@ -747,6 +747,7 @@ describe('Esm5ReflectionHost', () => {
const staticMethod = members.find(member => member.name === 'staticMethod') !;
expect(staticMethod.kind).toEqual(ClassMemberKind.Method);
expect(staticMethod.isStatic).toEqual(true);
expect(staticMethod.value).toBeNull();
expect(ts.isFunctionExpression(staticMethod.implementation !)).toEqual(true);
});