fix(ivy): proper resolution of Enums in Component decorator (#27971)

Prior to this change Component decorator was resolving `encapsulation` value a bit incorrectly, which resulted in `encapsulation: NaN` in compiled code. Now we resolve the value as Enum memeber and throw if it's not the case. As a part of this update, the `changeDetection` field handling is also added, the resolution logic is the same as the one used for `encapsulation` field.

PR Close #27971
This commit is contained in:
Andrew Kushnir 2019-01-07 16:35:06 -08:00
parent 142553abc6
commit c5ab3e8fd2
7 changed files with 122 additions and 13 deletions

View File

@ -12,7 +12,7 @@ import * as ts from 'typescript';
import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
import {ResolvedReference} from '../../imports'; import {ResolvedReference} from '../../imports';
import {PartialEvaluator} from '../../partial_evaluator'; import {EnumValue, PartialEvaluator} from '../../partial_evaluator';
import {Decorator, ReflectionHost, filterToMembersWithDecorator, reflectObjectLiteral} from '../../reflection'; import {Decorator, ReflectionHost, filterToMembersWithDecorator, reflectObjectLiteral} from '../../reflection';
import {AnalysisOutput, CompileResult, DecoratorHandler} from '../../transform'; import {AnalysisOutput, CompileResult, DecoratorHandler} from '../../transform';
import {TypeCheckContext, TypeCheckableDirectiveMeta} from '../../typecheck'; import {TypeCheckContext, TypeCheckableDirectiveMeta} from '../../typecheck';
@ -21,7 +21,7 @@ import {ResourceLoader} from './api';
import {extractDirectiveMetadata, extractQueriesFromDecorator, parseFieldArrayValue, queriesFromFields} from './directive'; import {extractDirectiveMetadata, extractQueriesFromDecorator, parseFieldArrayValue, queriesFromFields} from './directive';
import {generateSetClassMetadataCall} from './metadata'; import {generateSetClassMetadataCall} from './metadata';
import {ScopeDirective, SelectorScopeRegistry} from './selector_scope'; import {ScopeDirective, SelectorScopeRegistry} from './selector_scope';
import {extractDirectiveGuards, isAngularCore, unwrapExpression} from './util'; import {extractDirectiveGuards, isAngularCore, isAngularCoreReference, unwrapExpression} from './util';
const EMPTY_MAP = new Map<string, Expression>(); const EMPTY_MAP = new Map<string, Expression>();
const EMPTY_ARRAY: any[] = []; const EMPTY_ARRAY: any[] = [];
@ -226,17 +226,18 @@ export class ComponentDecoratorHandler implements
styles.push(...styleUrls.map(styleUrl => this.resourceLoader.load(styleUrl, containingFile))); styles.push(...styleUrls.map(styleUrl => this.resourceLoader.load(styleUrl, containingFile)));
} }
let encapsulation: number = 0; const encapsulation: number =
if (component.has('encapsulation')) { this._resolveEnumValue(component, 'encapsulation', 'ViewEncapsulation') || 0;
encapsulation = parseInt(this.evaluator.evaluate(component.get('encapsulation') !) as string);
} const changeDetection: number|null =
this._resolveEnumValue(component, 'changeDetection', 'ChangeDetectionStrategy');
let animations: Expression|null = null; let animations: Expression|null = null;
if (component.has('animations')) { if (component.has('animations')) {
animations = new WrappedNodeExpr(component.get('animations') !); animations = new WrappedNodeExpr(component.get('animations') !);
} }
return { const output = {
analysis: { analysis: {
meta: { meta: {
...metadata, ...metadata,
@ -260,6 +261,10 @@ export class ComponentDecoratorHandler implements
}, },
typeCheck: true, typeCheck: true,
}; };
if (changeDetection !== null) {
(output.analysis.meta as R3ComponentMetadata).changeDetection = changeDetection;
}
return output;
} }
typeCheck(ctx: TypeCheckContext, node: ts.Declaration, meta: ComponentHandlerData): void { typeCheck(ctx: TypeCheckContext, node: ts.Declaration, meta: ComponentHandlerData): void {
@ -327,6 +332,23 @@ export class ComponentDecoratorHandler implements
return meta; return meta;
} }
private _resolveEnumValue(
component: Map<string, ts.Expression>, field: string, enumSymbolName: string): number|null {
let resolved: number|null = null;
if (component.has(field)) {
const expr = component.get(field) !;
const value = this.evaluator.evaluate(expr) as any;
if (value instanceof EnumValue && isAngularCoreReference(value.enumRef, enumSymbolName)) {
resolved = value.resolved as number;
} else {
throw new FatalDiagnosticError(
ErrorCode.VALUE_HAS_WRONG_TYPE, expr,
`${field} must be a member of ${enumSymbolName} enum from @angular/core`);
}
}
return resolved;
}
private _extractStyleUrls(component: Map<string, ts.Expression>): string[]|null { private _extractStyleUrls(component: Map<string, ts.Expression>): string[]|null {
if (!component.has('styleUrls')) { if (!component.has('styleUrls')) {
return null; return null;

View File

@ -85,6 +85,11 @@ export function isAngularCore(decorator: Decorator): boolean {
return decorator.import !== null && decorator.import.from === '@angular/core'; return decorator.import !== null && decorator.import.from === '@angular/core';
} }
export function isAngularCoreReference(reference: Reference, symbolName: string) {
return reference instanceof AbsoluteReference && reference.moduleName === '@angular/core' &&
reference.symbolName === symbolName;
}
/** /**
* Unwrap a `ts.Expression`, removing outer type-casts or parentheses until the expression is in its * Unwrap a `ts.Expression`, removing outer type-casts or parentheses until the expression is in its
* lowest level form. * lowest level form.

View File

@ -257,7 +257,8 @@ export class StaticInterpreter {
node.members.forEach(member => { node.members.forEach(member => {
const name = this.stringNameFromPropertyName(member.name, context); const name = this.stringNameFromPropertyName(member.name, context);
if (name !== undefined) { if (name !== undefined) {
map.set(name, new EnumValue(enumRef, name)); const resolved = member.initializer && this.visit(member.initializer, context);
map.set(name, new EnumValue(enumRef, name, resolved));
} }
}); });
return map; return map;

View File

@ -70,7 +70,9 @@ export interface ResolvedValueArray extends Array<ResolvedValue> {}
* Contains a `Reference` to the enumeration itself, and the name of the referenced member. * Contains a `Reference` to the enumeration itself, and the name of the referenced member.
*/ */
export class EnumValue { export class EnumValue {
constructor(readonly enumRef: Reference<ts.EnumDeclaration>, readonly name: string) {} constructor(
readonly enumRef: Reference<ts.EnumDeclaration>, readonly name: string,
readonly resolved: ResolvedValue) {}
} }
/** /**

View File

@ -50,11 +50,11 @@ describe('compiler compliance: styling', () => {
const files = { const files = {
app: { app: {
'spec.ts': ` 'spec.ts': `
import {Component, NgModule} from '@angular/core'; import {Component, NgModule, ViewEncapsulation} from '@angular/core';
@Component({ @Component({
selector: "my-component", selector: "my-component",
encapsulation: ${ViewEncapsulation.None}, encapsulation: ViewEncapsulation.None,
styles: ["div.tall { height: 123px; }", ":host.small p { height:5px; }"], styles: ["div.tall { height: 123px; }", ":host.small p { height:5px; }"],
template: "..." template: "..."
}) })
@ -77,10 +77,10 @@ describe('compiler compliance: styling', () => {
const files = { const files = {
app: { app: {
'spec.ts': ` 'spec.ts': `
import {Component, NgModule} from '@angular/core'; import {Component, NgModule, ViewEncapsulation} from '@angular/core';
@Component({ @Component({
encapsulation: ${ViewEncapsulation.Native}, encapsulation: ViewEncapsulation.Native,
selector: "my-component", selector: "my-component",
styles: ["div.cool { color: blue; }", ":host.nice p { color: gold; }"], styles: ["div.cool { color: blue; }", ":host.nice p { color: gold; }"],
template: "..." template: "..."

View File

@ -64,3 +64,15 @@ export interface SimpleChanges { [propName: string]: any; }
export type ɵNgModuleDefWithMeta<ModuleT, DeclarationsT, ImportsT, ExportsT> = any; export type ɵNgModuleDefWithMeta<ModuleT, DeclarationsT, ImportsT, ExportsT> = any;
export type ɵDirectiveDefWithMeta<DirT, SelectorT, ExportAsT, InputsT, OutputsT, QueriesT> = any; export type ɵDirectiveDefWithMeta<DirT, SelectorT, ExportAsT, InputsT, OutputsT, QueriesT> = any;
export enum ViewEncapsulation {
Emulated = 0,
Native = 1,
None = 2,
ShadowDom = 3
}
export enum ChangeDetectionStrategy {
OnPush = 0,
Default = 1
}

View File

@ -820,6 +820,73 @@ describe('ngtsc behavioral tests', () => {
expect(jsContents).toContain('interpolation1("", ctx.text, "")'); expect(jsContents).toContain('interpolation1("", ctx.text, "")');
}); });
it('should handle `encapsulation` field', () => {
env.tsconfig();
env.write(`test.ts`, `
import {Component, ViewEncapsulation} from '@angular/core';
@Component({
selector: 'comp-a',
template: '...',
encapsulation: ViewEncapsulation.None
})
class CompA {}
`);
env.driveMain();
const jsContents = env.getContents('test.js');
expect(jsContents).toContain('encapsulation: 2');
});
it('should throw if `encapsulation` contains invalid value', () => {
env.tsconfig();
env.write('test.ts', `
import {Component} from '@angular/core';
@Component({
selector: 'comp-a',
template: '...',
encapsulation: 'invalid-value'
})
class CompA {}
`);
const errors = env.driveDiagnostics();
expect(errors[0].messageText)
.toContain('encapsulation must be a member of ViewEncapsulation enum from @angular/core');
});
it('should handle `changeDetection` field', () => {
env.tsconfig();
env.write(`test.ts`, `
import {Component, ChangeDetectionStrategy} from '@angular/core';
@Component({
selector: 'comp-a',
template: '...',
changeDetection: ChangeDetectionStrategy.OnPush
})
class CompA {}
`);
env.driveMain();
const jsContents = env.getContents('test.js');
expect(jsContents).toContain('changeDetection: 0');
});
it('should throw if `changeDetection` contains invalid value', () => {
env.tsconfig();
env.write('test.ts', `
import {Component} from '@angular/core';
@Component({
selector: 'comp-a',
template: '...',
changeDetection: 'invalid-value'
})
class CompA {}
`);
const errors = env.driveDiagnostics();
expect(errors[0].messageText)
.toContain(
'changeDetection must be a member of ChangeDetectionStrategy enum from @angular/core');
});
it('should correctly recognize local symbols', () => { it('should correctly recognize local symbols', () => {
env.tsconfig(); env.tsconfig();
env.write('module.ts', ` env.write('module.ts', `