From b50283ed6750dc20c38d94fbe475fee8c2f1ef5a Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Wed, 27 Feb 2019 16:54:37 -0800 Subject: [PATCH] fix(ivy): support dynamic host attribute bindings (#29033) In the @Component decorator, the 'host' field is an object which represents host bindings. The type of this field is complex, but is generally of the form {[key: string]: string}. Several different kinds of bindings can be specified, depending on the structure of the key. For example: ``` @Component({ host: {'[prop]': 'someExpr'} }) ``` will bind an expression 'someExpr' to the property 'prop'. This is known to be a property binding because of the square brackets in the binding key. If the binding key is a plain string (no brackets or parentheses), then it is known as an attribute binding. In this case, the right-hand side is not interpreted as an expression, but is instead a constant string. There is no actual requirement that at build time, these constant strings are known to the compiler, but this was previously enforced as a side effect of requiring the binding expressions for property and event bindings to be statically known (as they need to be parsed). This commit breaks that relationship and allows the attribute bindings to be dynamic. In the case that they are dynamic, the references to the dynamic values are reflected into the Ivy instructions for attribute bindings. PR Close #29033 --- .../src/ngtsc/annotations/src/directive.ts | 32 +++-- .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 25 ++++ packages/compiler/src/compiler.ts | 2 +- packages/compiler/src/jit_compiler_facade.ts | 8 +- packages/compiler/src/render3/view/api.ts | 6 +- .../compiler/src/render3/view/compiler.ts | 117 +++++++++--------- .../src/render3/view/styling_builder.ts | 5 +- 7 files changed, 113 insertions(+), 82 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts index bbcd000f44..4c36a13521 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts @@ -6,12 +6,13 @@ * found in the LICENSE file at https://angular.io/license */ -import {ConstantPool, Expression, ParseError, R3DirectiveMetadata, R3QueryMetadata, Statement, WrappedNodeExpr, compileDirectiveFromMetadata, makeBindingParser, parseHostBindings, verifyHostBindings} from '@angular/compiler'; +import {ConstantPool, Expression, ParseError, ParsedHostBindings, R3DirectiveMetadata, R3QueryMetadata, Statement, WrappedNodeExpr, compileDirectiveFromMetadata, makeBindingParser, parseHostBindings, verifyHostBindings} from '@angular/compiler'; + import * as ts from 'typescript'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; import {Reference} from '../../imports'; -import {EnumValue, PartialEvaluator} from '../../partial_evaluator'; +import {DynamicValue, EnumValue, PartialEvaluator} from '../../partial_evaluator'; import {ClassMember, ClassMemberKind, Decorator, ReflectionHost, filterToMembersWithDecorator, reflectObjectLiteral} from '../../reflection'; import {LocalModuleScopeRegistry} from '../../scope/src/local'; import {extractDirectiveGuards} from '../../scope/src/util'; @@ -441,18 +442,14 @@ function isPropertyTypeMember(member: ClassMember): boolean { member.kind === ClassMemberKind.Property; } -type StringMap = { - [key: string]: string +type StringMap = { + [key: string]: T; }; function extractHostBindings( metadata: Map, members: ClassMember[], evaluator: PartialEvaluator, - coreModule: string | undefined): { - attributes: StringMap, - listeners: StringMap, - properties: StringMap, -} { - let hostMetadata: StringMap = {}; + coreModule: string | undefined): ParsedHostBindings { + let hostMetadata: StringMap = {}; if (metadata.has('host')) { const expr = metadata.get('host') !; const hostMetaMap = evaluator.evaluate(expr); @@ -466,10 +463,19 @@ function extractHostBindings( value = value.resolved; } - if (typeof value !== 'string' || typeof key !== 'string') { - throw new Error(`Decorator host metadata must be a string -> string object, got ${value}`); + if (typeof key !== 'string') { + throw new Error( + `Decorator host metadata must be a string -> string object, but found unparseable key ${key}`); + } + + if (typeof value == 'string') { + hostMetadata[key] = value; + } else if (value instanceof DynamicValue) { + hostMetadata[key] = new WrappedNodeExpr(value.node as ts.Expression); + } else { + throw new Error( + `Decorator host metadata must be a string -> string object, but found unparseable value ${value}`); } - hostMetadata[key] = value; }); } diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index f0ada1735e..9fcb0c8f1a 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -1306,6 +1306,31 @@ describe('ngtsc behavioral tests', () => { expect(trim(jsContents)).toContain(trim(hostBindingsFn)); }); + it('should accept dynamic host attribute bindings', () => { + env.tsconfig(); + env.write('other.d.ts', ` + export declare const foo: any; + `); + env.write('test.ts', ` + import {Component} from '@angular/core'; + import {foo} from './other'; + + const test = foo.bar(); + + @Component({ + selector: 'test', + template: '', + host: { + 'test': test, + }, + }) + export class TestCmp {} + `); + env.driveMain(); + const jsContents = env.getContents('test.js'); + expect(jsContents).toContain('i0.ɵelementHostAttrs(ctx, ["test", test])'); + }); + it('should accept enum values as host bindings', () => { env.tsconfig(); env.write(`test.ts`, ` diff --git a/packages/compiler/src/compiler.ts b/packages/compiler/src/compiler.ts index dbc9f643f4..6e2c7d555c 100644 --- a/packages/compiler/src/compiler.ts +++ b/packages/compiler/src/compiler.ts @@ -99,7 +99,7 @@ export {compileInjector, compileNgModule, R3InjectorMetadata, R3NgModuleMetadata export {compilePipeFromMetadata, R3PipeMetadata} from './render3/r3_pipe_compiler'; export {makeBindingParser, parseTemplate} from './render3/view/template'; export {R3Reference} from './render3/util'; -export {compileBaseDefFromMetadata, R3BaseRefMetaData, compileComponentFromMetadata, compileDirectiveFromMetadata, parseHostBindings, verifyHostBindings} from './render3/view/compiler'; +export {compileBaseDefFromMetadata, R3BaseRefMetaData, compileComponentFromMetadata, compileDirectiveFromMetadata, parseHostBindings, ParsedHostBindings, verifyHostBindings} from './render3/view/compiler'; export {publishFacade} from './jit_compiler_facade'; // This file only reexports content of the `src` folder. Keep it that way. diff --git a/packages/compiler/src/jit_compiler_facade.ts b/packages/compiler/src/jit_compiler_facade.ts index eeff934a04..15483a5f65 100644 --- a/packages/compiler/src/jit_compiler_facade.ts +++ b/packages/compiler/src/jit_compiler_facade.ts @@ -21,7 +21,7 @@ import {R3InjectorMetadata, R3NgModuleMetadata, compileInjector, compileNgModule import {compilePipeFromMetadata} from './render3/r3_pipe_compiler'; import {R3Reference} from './render3/util'; import {R3DirectiveMetadata, R3QueryMetadata} from './render3/view/api'; -import {compileComponentFromMetadata, compileDirectiveFromMetadata, parseHostBindings, verifyHostBindings} from './render3/view/compiler'; +import {ParsedHostBindings, compileComponentFromMetadata, compileDirectiveFromMetadata, parseHostBindings, verifyHostBindings} from './render3/view/compiler'; import {makeBindingParser, parseTemplate} from './render3/view/template'; import {DomElementSchemaRegistry} from './schema/dom_element_schema_registry'; @@ -281,11 +281,7 @@ function convertR3DependencyMetadataArray(facades: R3DependencyMetadataFacade[] function extractHostBindings( host: {[key: string]: string}, propMetadata: {[key: string]: any[]}, - sourceSpan: ParseSourceSpan): { - attributes: StringMap, - listeners: StringMap, - properties: StringMap, -} { + sourceSpan: ParseSourceSpan): ParsedHostBindings { // First parse the declarations from the metadata. const bindings = parseHostBindings(host || {}); diff --git a/packages/compiler/src/render3/view/api.ts b/packages/compiler/src/render3/view/api.ts index d82cc05540..a164e887df 100644 --- a/packages/compiler/src/render3/view/api.ts +++ b/packages/compiler/src/render3/view/api.ts @@ -59,9 +59,9 @@ export interface R3DirectiveMetadata { */ host: { /** - * A mapping of attribute binding keys to unparsed expressions. + * A mapping of attribute binding keys to `o.Expression`s. */ - attributes: {[key: string]: string}; + attributes: {[key: string]: o.Expression}; /** * A mapping of event binding keys to unparsed expressions. @@ -72,6 +72,8 @@ export interface R3DirectiveMetadata { * A mapping of property binding keys to unparsed expressions. */ properties: {[key: string]: string}; + + specialAttributes: {styleAttr?: string; classAttr?: string;} }; /** diff --git a/packages/compiler/src/render3/view/compiler.ts b/packages/compiler/src/render3/view/compiler.ts index 782938fcb9..f9a8771bb5 100644 --- a/packages/compiler/src/render3/view/compiler.ts +++ b/packages/compiler/src/render3/view/compiler.ts @@ -82,30 +82,18 @@ function baseDirectiveFields( const contextVarExp = o.variable(CONTEXT_NAME); const styleBuilder = new StylingBuilder(elVarExp, contextVarExp); - const allOtherAttributes: any = {}; - const attrNames = Object.getOwnPropertyNames(meta.host.attributes); - for (let i = 0; i < attrNames.length; i++) { - const attr = attrNames[i]; - const value = meta.host.attributes[attr]; - switch (attr) { - // style attributes are handled in the styling context - case 'style': - styleBuilder.registerStyleAttr(value); - break; - // class attributes are handled in the styling context - case 'class': - styleBuilder.registerClassAttr(value); - break; - default: - allOtherAttributes[attr] = value; - break; - } + const {styleAttr, classAttr} = meta.host.specialAttributes; + if (styleAttr !== undefined) { + styleBuilder.registerStyleAttr(styleAttr); + } + if (classAttr !== undefined) { + styleBuilder.registerClassAttr(classAttr); } // e.g. `hostBindings: (rf, ctx, elIndex) => { ... } definitionMap.set( 'hostBindings', createHostBindingsFunction( - meta, elVarExp, contextVarExp, allOtherAttributes, styleBuilder, + meta, elVarExp, contextVarExp, meta.host.attributes, styleBuilder, bindingParser, constantPool, hostVarsCount)); // e.g 'inputs: {a: 'a'}` @@ -412,34 +400,8 @@ export function compileComponentFromRender2( function directiveMetadataFromGlobalMetadata( directive: CompileDirectiveMetadata, outputCtx: OutputContext, reflector: CompileReflector): R3DirectiveMetadata { - const summary = directive.toSummary(); - const name = identifierName(directive.type) !; - name || error(`Cannot resolver the name of ${directive.type}`); - - return { - name, - type: outputCtx.importExpr(directive.type.reference), - typeArgumentCount: 0, - typeSourceSpan: - typeSourceSpan(directive.isComponent ? 'Component' : 'Directive', directive.type), - selector: directive.selector, - deps: dependenciesFromGlobalMetadata(directive.type, outputCtx, reflector), - queries: queriesFromGlobalMetadata(directive.queries, outputCtx), - lifecycle: { - usesOnChanges: - directive.type.lifecycleHooks.some(lifecycle => lifecycle == LifecycleHooks.OnChanges), - }, - host: { - attributes: directive.hostAttributes, - listeners: summary.hostListeners, - properties: summary.hostProperties, - }, - inputs: directive.inputs, - outputs: directive.outputs, - usesInheritance: false, - exportAs: null, - providers: directive.providers.length > 0 ? new o.WrappedNodeExpr(directive.providers) : null - }; + // The global-analysis based Ivy mode in ngc is no longer utilized/supported. + throw new Error('unsupported'); } /** @@ -501,11 +463,12 @@ function createDirectiveSelector(selector: string | null): o.Expression { return asLiteral(core.parseSelectorToR3Selector(selector)); } -function convertAttributesToExpressions(attributes: any): o.Expression[] { +function convertAttributesToExpressions(attributes: {[name: string]: o.Expression}): + o.Expression[] { const values: o.Expression[] = []; for (let key of Object.getOwnPropertyNames(attributes)) { const value = attributes[key]; - values.push(o.literal(key), o.literal(value)); + values.push(o.literal(key), value); } return values; } @@ -622,8 +585,9 @@ function createViewQueriesFunction( // Return a host binding function or null if one is not necessary. function createHostBindingsFunction( meta: R3DirectiveMetadata, elVarExp: o.ReadVarExpr, bindingContext: o.ReadVarExpr, - staticAttributesAndValues: any[], styleBuilder: StylingBuilder, bindingParser: BindingParser, - constantPool: ConstantPool, hostVarsCount: number): o.Expression|null { + staticAttributesAndValues: {[name: string]: o.Expression}, styleBuilder: StylingBuilder, + bindingParser: BindingParser, constantPool: ConstantPool, hostVarsCount: number): o.Expression| + null { const createStatements: o.Statement[] = []; const updateStatements: o.Statement[] = []; @@ -826,7 +790,9 @@ function createHostListeners( function metadataAsSummary(meta: R3DirectiveMetadata): CompileDirectiveSummary { // clang-format off return { - hostAttributes: meta.host.attributes, + // This is used by the BindingParser, which only deals with listeners and properties. There's no + // need to pass attributes to it. + hostAttributes: {}, hostListeners: meta.host.listeners, hostProperties: meta.host.properties, } as CompileDirectiveSummary; @@ -855,32 +821,65 @@ const enum HostBindingGroup { // Defines Host Bindings structure that contains attributes, listeners, and properties, // parsed from the `host` object defined for a Type. export interface ParsedHostBindings { - attributes: {[key: string]: string}; + attributes: {[key: string]: o.Expression}; listeners: {[key: string]: string}; properties: {[key: string]: string}; + specialAttributes: {styleAttr?: string; classAttr?: string;}; } -export function parseHostBindings(host: {[key: string]: string}): ParsedHostBindings { - const attributes: {[key: string]: string} = {}; +export function parseHostBindings(host: {[key: string]: string | o.Expression}): + ParsedHostBindings { + const attributes: {[key: string]: o.Expression} = {}; const listeners: {[key: string]: string} = {}; const properties: {[key: string]: string} = {}; + const specialAttributes: {styleAttr?: string; classAttr?: string;} = {}; - Object.keys(host).forEach(key => { + for (const key of Object.keys(host)) { const value = host[key]; const matches = key.match(HOST_REG_EXP); + if (matches === null) { - attributes[key] = value; + switch (key) { + case 'class': + if (typeof value !== 'string') { + // TODO(alxhub): make this a diagnostic. + throw new Error(`Class binding must be string`); + } + specialAttributes.classAttr = value; + break; + case 'style': + if (typeof value !== 'string') { + // TODO(alxhub): make this a diagnostic. + throw new Error(`Style binding must be string`); + } + specialAttributes.styleAttr = value; + break; + default: + if (typeof value === 'string') { + attributes[key] = o.literal(value); + } else { + attributes[key] = value; + } + } } else if (matches[HostBindingGroup.Binding] != null) { + if (typeof value !== 'string') { + // TODO(alxhub): make this a diagnostic. + throw new Error(`Property binding must be string`); + } // synthetic properties (the ones that have a `@` as a prefix) // are still treated the same as regular properties. Therefore // there is no point in storing them in a separate map. properties[matches[HostBindingGroup.Binding]] = value; } else if (matches[HostBindingGroup.Event] != null) { + if (typeof value !== 'string') { + // TODO(alxhub): make this a diagnostic. + throw new Error(`Event binding must be string`); + } listeners[matches[HostBindingGroup.Event]] = value; } - }); + } - return {attributes, listeners, properties}; + return {attributes, listeners, properties, specialAttributes}; } /** diff --git a/packages/compiler/src/render3/view/styling_builder.ts b/packages/compiler/src/render3/view/styling_builder.ts index 774214a062..9dd5942071 100644 --- a/packages/compiler/src/render3/view/styling_builder.ts +++ b/packages/compiler/src/render3/view/styling_builder.ts @@ -258,7 +258,10 @@ export class StylingBuilder { buildParams: () => { // params => elementHostAttrs(directive, attrs) this.populateInitialStylingAttrs(attrs); - return [this._directiveExpr !, getConstantLiteralFromArray(constantPool, attrs)]; + const attrArray = !attrs.some(attr => attr instanceof o.WrappedNodeExpr) ? + getConstantLiteralFromArray(constantPool, attrs) : + o.literalArr(attrs); + return [this._directiveExpr !, attrArray]; } }; }