From 8eb102ab10aeda1c5c9dd0fac3052b6b7b3a32a5 Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Fri, 30 Nov 2018 17:34:36 -0800 Subject: [PATCH] fix(ivy): avoid counting style/class bindings in component/directive `hostBindings` (#27388) PR Close #27388 --- .../r3_view_compiler_styling_spec.ts | 117 +++++++++++++++++- .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 2 +- .../compiler/src/render3/view/compiler.ts | 16 ++- packages/core/src/render3/instructions.ts | 5 +- .../core/test/render3/host_binding_spec.ts | 2 - 5 files changed, 128 insertions(+), 14 deletions(-) diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts index d6c11c46bb..6bac852cc2 100644 --- a/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts @@ -773,7 +773,6 @@ describe('compiler compliance: styling', () => { … hostBindings: function MyComponent_HostBindings(rf, ctx, elIndex) { if (rf & 1) { - $r3$.ɵallocHostVars(4); $r3$.ɵelementStyling(_c0, _c1, $r3$.ɵdefaultStyleSanitizer, ctx); } if (rf & 2) { @@ -782,7 +781,9 @@ describe('compiler compliance: styling', () => { $r3$.ɵelementClassProp(elIndex, 0, ctx.myFooClass, ctx); $r3$.ɵelementStylingApply(elIndex, ctx); } - } + }, + consts: 0, + vars: 0, `; const result = compile(files, angularFiles); @@ -832,7 +833,6 @@ describe('compiler compliance: styling', () => { … hostBindings: function MyComponent_HostBindings(rf, ctx, elIndex) { if (rf & 1) { - $r3$.ɵallocHostVars(6); $r3$.ɵelementStyling(_c0, _c1, $r3$.ɵdefaultStyleSanitizer, ctx); } if (rf & 2) { @@ -843,7 +843,9 @@ describe('compiler compliance: styling', () => { $r3$.ɵelementClassProp(elIndex, 1, ctx.myFooClass, ctx); $r3$.ɵelementStylingApply(elIndex, ctx); } - } + }, + consts: 0, + vars: 0, `; const result = compile(files, angularFiles); @@ -898,7 +900,6 @@ describe('compiler compliance: styling', () => { … function WidthDirective_HostBindings(rf, ctx, elIndex) { if (rf & 1) { - $r3$.ɵallocHostVars(2); $r3$.ɵelementStyling(_c0, _c1, null, ctx); } if (rf & 2) { @@ -910,7 +911,6 @@ describe('compiler compliance: styling', () => { … function HeightDirective_HostBindings(rf, ctx, elIndex) { if (rf & 1) { - $r3$.ɵallocHostVars(2); $r3$.ɵelementStyling(_c2, _c3, null, ctx); } if (rf & 2) { @@ -926,4 +926,109 @@ describe('compiler compliance: styling', () => { expectEmit(result.source, template, 'Incorrect template'); }); }); + + it('should count only non-style and non-class host bindings on Components', () => { + const files = { + app: { + 'spec.ts': ` + import {Component, NgModule, HostBinding} from '@angular/core'; + + @Component({ + selector: 'my-component', + template: '', + host: { + 'style': 'width:200px; height:500px', + 'class': 'foo baz' + } + }) + export class MyComponent { + @HostBinding('style') + myStyle = {width:'100px'}; + + @HostBinding('class') + myClass = {bar:false}; + + @HostBinding('id') + id = 'some id'; + + @HostBinding('title') + title = 'some title'; + } + + @NgModule({declarations: [MyComponent]}) + export class MyModule {} + ` + } + }; + + const template = ` + const $_c0$ = ["foo", "baz", ${InitialStylingFlags.VALUES_MODE}, "foo", true, "baz", true]; + const $_c1$ = ["width", "height", ${InitialStylingFlags.VALUES_MODE}, "width", "200px", "height", "500px"]; + … + hostBindings: function MyComponent_HostBindings(rf, ctx, elIndex) { + if (rf & 1) { + $r3$.ɵallocHostVars(2); + $r3$.ɵelementStyling($_c0$, $_c1$, $r3$.ɵdefaultStyleSanitizer, ctx); + } + if (rf & 2) { + $r3$.ɵelementProperty(elIndex, "id", $r3$.ɵbind(ctx.id)); + $r3$.ɵelementProperty(elIndex, "title", $r3$.ɵbind(ctx.title)); + $r3$.ɵelementStylingMap(elIndex, ctx.myClass, ctx.myStyle, ctx); + $r3$.ɵelementStylingApply(elIndex, ctx); + } + }, + consts: 0, + vars: 0, + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, template, 'Incorrect template'); + }); + + it('should count only non-style and non-class host bindings on Directives', () => { + const files = { + app: { + 'spec.ts': ` + import {Directive, Component, NgModule, HostBinding} from '@angular/core'; + + @Directive({selector: '[myWidthDir]'}) + export class WidthDirective { + @HostBinding('style.width') + myWidth = 200; + + @HostBinding('class.foo') + myFooClass = true; + + @HostBinding('id') + id = 'some id'; + + @HostBinding('title') + title = 'some title'; + } + ` + } + }; + + const template = ` + const $_c0$ = ["foo"]; + const $_c1$ = ["width"]; + … + hostBindings: function WidthDirective_HostBindings(rf, ctx, elIndex) { + if (rf & 1) { + $r3$.ɵallocHostVars(2); + $r3$.ɵelementStyling($_c0$, $_c1$, null, ctx); + } + if (rf & 2) { + $r3$.ɵelementProperty(elIndex, "id", $r3$.ɵbind(ctx.id)); + $r3$.ɵelementProperty(elIndex, "title", $r3$.ɵbind(ctx.title)); + $r3$.ɵelementStyleProp(elIndex, 0, ctx.myWidth, null, ctx); + $r3$.ɵelementClassProp(elIndex, 0, ctx.myFooClass, ctx); + $r3$.ɵelementStylingApply(elIndex, ctx); + } + } + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, template, 'Incorrect template'); + }); }); diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index 20491ba23f..210abe4cc5 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -512,7 +512,7 @@ describe('ngtsc behavioral tests', () => { const hostBindingsFn = ` hostBindings: function FooCmp_HostBindings(rf, ctx, elIndex) { if (rf & 1) { - i0.ɵallocHostVars(3); + i0.ɵallocHostVars(2); i0.ɵlistener("click", function FooCmp_click_HostBindingHandler($event) { return ctx.onClick($event); }); i0.ɵlistener("change", function FooCmp_change_HostBindingHandler($event) { return ctx.onChange(ctx.arg1, ctx.arg2, ctx.arg3); }); i0.ɵelementStyling(_c0, null, null, ctx); diff --git a/packages/compiler/src/render3/view/compiler.ts b/packages/compiler/src/render3/view/compiler.ts index 011049245c..9aa5fd9330 100644 --- a/packages/compiler/src/render3/view/compiler.ts +++ b/packages/compiler/src/render3/view/compiler.ts @@ -37,6 +37,10 @@ const EMPTY_ARRAY: any[] = []; // If there is a match, the first matching group will contain the attribute name to bind. const ATTR_REGEX = /attr\.([^\]]+)/; +function getStylingPrefix(propName: string): string { + return propName.substring(0, 5).toLowerCase(); +} + function baseDirectiveFields( meta: R3DirectiveMetadata, constantPool: ConstantPool, bindingParser: BindingParser): {definitionMap: DefinitionMap, statements: o.Statement[]} { @@ -62,8 +66,14 @@ function baseDirectiveFields( definitionMap.set('contentQueriesRefresh', createContentQueriesRefreshFunction(meta)); - // Initialize hostVarsCount to number of bound host properties (interpolations illegal) - const hostVarsCount = Object.keys(meta.host.properties).length; + // Initialize hostVarsCount to number of bound host properties (interpolations illegal), + // except 'style' and 'class' properties, since they should *not* allocate host var slots + const hostVarsCount = Object.keys(meta.host.properties) + .filter(name => { + const prefix = getStylingPrefix(name); + return prefix !== 'style' && prefix !== 'class'; + }) + .length; const elVarExp = o.variable('elIndex'); const contextVarExp = o.variable(CONTEXT_NAME); @@ -672,7 +682,7 @@ function createHostBindingsFunction( for (const binding of bindings) { const name = binding.name; - const stylePrefix = name.substring(0, 5).toLowerCase(); + const stylePrefix = getStylingPrefix(name); if (stylePrefix === 'style') { const {propertyName, unit} = parseNamedProperty(name); styleBuilder.registerStyleInput(propertyName, binding.expression, unit, binding.sourceSpan); diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index 69922a84a8..d5de0dc541 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -1515,9 +1515,10 @@ function invokeDirectivesHostBindings(tView: TView, viewData: LView, previousOrP setCurrentDirectiveDef(null); // `hostBindings` function may or may not contain `allocHostVars` call // (e.g. it may not if it only contains host listeners), so we need to check whether - // `expandoInstructions` has changed and if not - we push `null` to keep indices in sync + // `expandoInstructions` has changed and if not - we still push `hostBindings` to + // expando block, to make sure we execute it for DI cycle if (previousExpandoLength === expando.length && firstTemplatePass) { - expando.push(null); + expando.push(def.hostBindings); } } else if (firstTemplatePass) { expando.push(null); diff --git a/packages/core/test/render3/host_binding_spec.ts b/packages/core/test/render3/host_binding_spec.ts index efc11b7fb4..d00fc12ea6 100644 --- a/packages/core/test/render3/host_binding_spec.ts +++ b/packages/core/test/render3/host_binding_spec.ts @@ -999,7 +999,6 @@ describe('host bindings', () => { vars: 0, hostBindings: (rf: RenderFlags, ctx: HostBindingToStyles, elIndex: number) => { if (rf & RenderFlags.Create) { - allocHostVars(0); // this is wrong, but necessary until FW-761 gets in elementStyling(null, ['width'], null, ctx); } if (rf & RenderFlags.Update) { @@ -1043,7 +1042,6 @@ describe('host bindings', () => { vars: 0, hostBindings: (rf: RenderFlags, ctx: StaticHostClass, elIndex: number) => { if (rf & RenderFlags.Create) { - allocHostVars(0); // this is wrong, but necessary until FW-761 gets in elementStyling( ['mat-toolbar', InitialStylingFlags.VALUES_MODE, 'mat-toolbar', true], null, null, ctx);