fix(ivy): avoid counting style/class bindings in component/directive `hostBindings` (#27388)

PR Close #27388
This commit is contained in:
Andrew Kushnir 2018-11-30 17:34:36 -08:00 committed by Alex Rickabaugh
parent 2a39425e48
commit 8eb102ab10
5 changed files with 128 additions and 14 deletions

View File

@ -773,7 +773,6 @@ describe('compiler compliance: styling', () => {
hostBindings: function MyComponent_HostBindings(rf, ctx, elIndex) { hostBindings: function MyComponent_HostBindings(rf, ctx, elIndex) {
if (rf & 1) { if (rf & 1) {
$r3$.ɵallocHostVars(4);
$r3$.ɵelementStyling(_c0, _c1, $r3$.ɵdefaultStyleSanitizer, ctx); $r3$.ɵelementStyling(_c0, _c1, $r3$.ɵdefaultStyleSanitizer, ctx);
} }
if (rf & 2) { if (rf & 2) {
@ -782,7 +781,9 @@ describe('compiler compliance: styling', () => {
$r3$.ɵelementClassProp(elIndex, 0, ctx.myFooClass, ctx); $r3$.ɵelementClassProp(elIndex, 0, ctx.myFooClass, ctx);
$r3$.ɵelementStylingApply(elIndex, ctx); $r3$.ɵelementStylingApply(elIndex, ctx);
} }
} },
consts: 0,
vars: 0,
`; `;
const result = compile(files, angularFiles); const result = compile(files, angularFiles);
@ -832,7 +833,6 @@ describe('compiler compliance: styling', () => {
hostBindings: function MyComponent_HostBindings(rf, ctx, elIndex) { hostBindings: function MyComponent_HostBindings(rf, ctx, elIndex) {
if (rf & 1) { if (rf & 1) {
$r3$.ɵallocHostVars(6);
$r3$.ɵelementStyling(_c0, _c1, $r3$.ɵdefaultStyleSanitizer, ctx); $r3$.ɵelementStyling(_c0, _c1, $r3$.ɵdefaultStyleSanitizer, ctx);
} }
if (rf & 2) { if (rf & 2) {
@ -843,7 +843,9 @@ describe('compiler compliance: styling', () => {
$r3$.ɵelementClassProp(elIndex, 1, ctx.myFooClass, ctx); $r3$.ɵelementClassProp(elIndex, 1, ctx.myFooClass, ctx);
$r3$.ɵelementStylingApply(elIndex, ctx); $r3$.ɵelementStylingApply(elIndex, ctx);
} }
} },
consts: 0,
vars: 0,
`; `;
const result = compile(files, angularFiles); const result = compile(files, angularFiles);
@ -898,7 +900,6 @@ describe('compiler compliance: styling', () => {
function WidthDirective_HostBindings(rf, ctx, elIndex) { function WidthDirective_HostBindings(rf, ctx, elIndex) {
if (rf & 1) { if (rf & 1) {
$r3$.ɵallocHostVars(2);
$r3$.ɵelementStyling(_c0, _c1, null, ctx); $r3$.ɵelementStyling(_c0, _c1, null, ctx);
} }
if (rf & 2) { if (rf & 2) {
@ -910,7 +911,6 @@ describe('compiler compliance: styling', () => {
function HeightDirective_HostBindings(rf, ctx, elIndex) { function HeightDirective_HostBindings(rf, ctx, elIndex) {
if (rf & 1) { if (rf & 1) {
$r3$.ɵallocHostVars(2);
$r3$.ɵelementStyling(_c2, _c3, null, ctx); $r3$.ɵelementStyling(_c2, _c3, null, ctx);
} }
if (rf & 2) { if (rf & 2) {
@ -926,4 +926,109 @@ describe('compiler compliance: styling', () => {
expectEmit(result.source, template, 'Incorrect template'); 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');
});
}); });

View File

@ -512,7 +512,7 @@ describe('ngtsc behavioral tests', () => {
const hostBindingsFn = ` const hostBindingsFn = `
hostBindings: function FooCmp_HostBindings(rf, ctx, elIndex) { hostBindings: function FooCmp_HostBindings(rf, ctx, elIndex) {
if (rf & 1) { if (rf & 1) {
i0.ɵallocHostVars(3); i0.ɵallocHostVars(2);
i0.ɵlistener("click", function FooCmp_click_HostBindingHandler($event) { return ctx.onClick($event); }); 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.ɵlistener("change", function FooCmp_change_HostBindingHandler($event) { return ctx.onChange(ctx.arg1, ctx.arg2, ctx.arg3); });
i0.ɵelementStyling(_c0, null, null, ctx); i0.ɵelementStyling(_c0, null, null, ctx);

View File

@ -37,6 +37,10 @@ const EMPTY_ARRAY: any[] = [];
// If there is a match, the first matching group will contain the attribute name to bind. // If there is a match, the first matching group will contain the attribute name to bind.
const ATTR_REGEX = /attr\.([^\]]+)/; const ATTR_REGEX = /attr\.([^\]]+)/;
function getStylingPrefix(propName: string): string {
return propName.substring(0, 5).toLowerCase();
}
function baseDirectiveFields( function baseDirectiveFields(
meta: R3DirectiveMetadata, constantPool: ConstantPool, meta: R3DirectiveMetadata, constantPool: ConstantPool,
bindingParser: BindingParser): {definitionMap: DefinitionMap, statements: o.Statement[]} { bindingParser: BindingParser): {definitionMap: DefinitionMap, statements: o.Statement[]} {
@ -62,8 +66,14 @@ function baseDirectiveFields(
definitionMap.set('contentQueriesRefresh', createContentQueriesRefreshFunction(meta)); definitionMap.set('contentQueriesRefresh', createContentQueriesRefreshFunction(meta));
// Initialize hostVarsCount to number of bound host properties (interpolations illegal) // Initialize hostVarsCount to number of bound host properties (interpolations illegal),
const hostVarsCount = Object.keys(meta.host.properties).length; // 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 elVarExp = o.variable('elIndex');
const contextVarExp = o.variable(CONTEXT_NAME); const contextVarExp = o.variable(CONTEXT_NAME);
@ -672,7 +682,7 @@ function createHostBindingsFunction(
for (const binding of bindings) { for (const binding of bindings) {
const name = binding.name; const name = binding.name;
const stylePrefix = name.substring(0, 5).toLowerCase(); const stylePrefix = getStylingPrefix(name);
if (stylePrefix === 'style') { if (stylePrefix === 'style') {
const {propertyName, unit} = parseNamedProperty(name); const {propertyName, unit} = parseNamedProperty(name);
styleBuilder.registerStyleInput(propertyName, binding.expression, unit, binding.sourceSpan); styleBuilder.registerStyleInput(propertyName, binding.expression, unit, binding.sourceSpan);

View File

@ -1515,9 +1515,10 @@ function invokeDirectivesHostBindings(tView: TView, viewData: LView, previousOrP
setCurrentDirectiveDef(null); setCurrentDirectiveDef(null);
// `hostBindings` function may or may not contain `allocHostVars` call // `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 // (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) { if (previousExpandoLength === expando.length && firstTemplatePass) {
expando.push(null); expando.push(def.hostBindings);
} }
} else if (firstTemplatePass) { } else if (firstTemplatePass) {
expando.push(null); expando.push(null);

View File

@ -999,7 +999,6 @@ describe('host bindings', () => {
vars: 0, vars: 0,
hostBindings: (rf: RenderFlags, ctx: HostBindingToStyles, elIndex: number) => { hostBindings: (rf: RenderFlags, ctx: HostBindingToStyles, elIndex: number) => {
if (rf & RenderFlags.Create) { if (rf & RenderFlags.Create) {
allocHostVars(0); // this is wrong, but necessary until FW-761 gets in
elementStyling(null, ['width'], null, ctx); elementStyling(null, ['width'], null, ctx);
} }
if (rf & RenderFlags.Update) { if (rf & RenderFlags.Update) {
@ -1043,7 +1042,6 @@ describe('host bindings', () => {
vars: 0, vars: 0,
hostBindings: (rf: RenderFlags, ctx: StaticHostClass, elIndex: number) => { hostBindings: (rf: RenderFlags, ctx: StaticHostClass, elIndex: number) => {
if (rf & RenderFlags.Create) { if (rf & RenderFlags.Create) {
allocHostVars(0); // this is wrong, but necessary until FW-761 gets in
elementStyling( elementStyling(
['mat-toolbar', InitialStylingFlags.VALUES_MODE, 'mat-toolbar', true], null, null, ['mat-toolbar', InitialStylingFlags.VALUES_MODE, 'mat-toolbar', true], null, null,
ctx); ctx);