fix(ivy): ensure `select(n)` instructions are always generated around style/class bindings (#30311)

Prior to this patch, the `select(n)` instruction would only be generated
when property bindings are encountered which meant that styling-related
bindings were skipped. This patch ensures that all styling-related bindings
(i.e. class and style bindings) are always prepended with a `select()`
instruction prior to being generated in AOT.

PR Close #30311
This commit is contained in:
Matias Niemelä 2019-05-07 13:18:27 -07:00 committed by Kara Erickson
parent bd37622050
commit 345a3cd9aa
3 changed files with 78 additions and 9 deletions

View File

@ -506,6 +506,7 @@ describe('compiler compliance', () => {
$r3$.ɵɵelementEnd(); $r3$.ɵɵelementEnd();
} }
if (rf & 2) { if (rf & 2) {
$r3$.ɵɵselect(0);
$r3$.ɵɵelementStyleProp(0, 0, ctx.color); $r3$.ɵɵelementStyleProp(0, 0, ctx.color);
$r3$.ɵɵelementClassProp(0, 0, ctx.error); $r3$.ɵɵelementClassProp(0, 0, ctx.error);
$r3$.ɵɵelementStylingApply(0); $r3$.ɵɵelementStylingApply(0);

View File

@ -389,6 +389,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd(); $r3$.ɵɵelementEnd();
} }
if (rf & 2) { if (rf & 2) {
$r3$.ɵɵselect(0);
$r3$.ɵɵelementStyleMap(0, $ctx$.myStyleExp); $r3$.ɵɵelementStyleMap(0, $ctx$.myStyleExp);
$r3$.ɵɵelementStylingApply(0); $r3$.ɵɵelementStylingApply(0);
} }
@ -454,6 +455,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd(); $r3$.ɵɵelementEnd();
} }
if (rf & 2) { if (rf & 2) {
$r3$.ɵɵselect(0);
$r3$.ɵɵelementClassMap(0, $r3$.ɵɵinterpolation1("foo foo-", $ctx$.fooId, "")); $r3$.ɵɵelementClassMap(0, $r3$.ɵɵinterpolation1("foo foo-", $ctx$.fooId, ""));
$r3$.ɵɵelementStylingApply(0); $r3$.ɵɵelementStylingApply(0);
} }
@ -468,6 +470,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd(); $r3$.ɵɵelementEnd();
} }
if (rf & 2) { if (rf & 2) {
$r3$.ɵɵselect(0);
$r3$.ɵɵelementClassMap(0, $r3$.ɵɵinterpolation2("foo foo-", $ctx$.fooId, "-", $ctx$.fooUsername, "")); $r3$.ɵɵelementClassMap(0, $r3$.ɵɵinterpolation2("foo foo-", $ctx$.fooId, "-", $ctx$.fooUsername, ""));
$r3$.ɵɵelementStylingApply(0); $r3$.ɵɵelementStylingApply(0);
} }
@ -482,6 +485,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd(); $r3$.ɵɵelementEnd();
} }
if (rf & 2) { if (rf & 2) {
$r3$.ɵɵselect(0);
$r3$.ɵɵelementClassMap(0, $ctx$.exp); $r3$.ɵɵelementClassMap(0, $ctx$.exp);
$r3$.ɵɵelementStylingApply(0); $r3$.ɵɵelementStylingApply(0);
} }
@ -538,11 +542,11 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd(); $r3$.ɵɵelementEnd();
} }
if (rf & 2) { if (rf & 2) {
$r3$.ɵɵselect(0);
$r3$.ɵɵelementStyleMap(0, $ctx$.myStyleExp); $r3$.ɵɵelementStyleMap(0, $ctx$.myStyleExp);
$r3$.ɵɵelementStyleProp(0, 0, $ctx$.myWidth); $r3$.ɵɵelementStyleProp(0, 0, $ctx$.myWidth);
$r3$.ɵɵelementStyleProp(0, 1, $ctx$.myHeight); $r3$.ɵɵelementStyleProp(0, 1, $ctx$.myHeight);
$r3$.ɵɵelementStylingApply(0); $r3$.ɵɵelementStylingApply(0);
$r3$.ɵɵselect(0);
$r3$.ɵɵelementAttribute(0, "style", $r3$.ɵɵbind("border-width: 10px"), $r3$.ɵɵsanitizeStyle); $r3$.ɵɵelementAttribute(0, "style", $r3$.ɵɵbind("border-width: 10px"), $r3$.ɵɵsanitizeStyle);
} }
}, },
@ -598,6 +602,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd(); $r3$.ɵɵelementEnd();
} }
if (rf & 2) { if (rf & 2) {
$r3$.ɵɵselect(0);
$r3$.ɵɵelementStyleProp(0, 0, ctx.myImage); $r3$.ɵɵelementStyleProp(0, 0, ctx.myImage);
$r3$.ɵɵelementStylingApply(0); $r3$.ɵɵelementStylingApply(0);
} }
@ -639,6 +644,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd(); $r3$.ɵɵelementEnd();
} }
if (rf & 2) { if (rf & 2) {
$r3$.ɵɵselect(0);
$r3$.ɵɵelementStyleProp(0, 0, 12, "px"); $r3$.ɵɵelementStyleProp(0, 0, 12, "px");
$r3$.ɵɵelementStylingApply(0); $r3$.ɵɵelementStylingApply(0);
} }
@ -704,6 +710,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd(); $r3$.ɵɵelementEnd();
} }
if (rf & 2) { if (rf & 2) {
$r3$.ɵɵselect(0);
$r3$.ɵɵelementClassMap(0,$ctx$.myClassExp); $r3$.ɵɵelementClassMap(0,$ctx$.myClassExp);
$r3$.ɵɵelementStylingApply(0); $r3$.ɵɵelementStylingApply(0);
} }
@ -760,11 +767,11 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd(); $r3$.ɵɵelementEnd();
} }
if (rf & 2) { if (rf & 2) {
$r3$.ɵɵselect(0);
$r3$.ɵɵelementClassMap(0, $ctx$.myClassExp); $r3$.ɵɵelementClassMap(0, $ctx$.myClassExp);
$r3$.ɵɵelementClassProp(0, 0, $ctx$.yesToApple); $r3$.ɵɵelementClassProp(0, 0, $ctx$.yesToApple);
$r3$.ɵɵelementClassProp(0, 1, $ctx$.yesToOrange); $r3$.ɵɵelementClassProp(0, 1, $ctx$.yesToOrange);
$r3$.ɵɵelementStylingApply(0); $r3$.ɵɵelementStylingApply(0);
$r3$.ɵɵselect(0);
$r3$.ɵɵelementAttribute(0, "class", $r3$.ɵɵbind("banana")); $r3$.ɵɵelementAttribute(0, "class", $r3$.ɵɵbind("banana"));
} }
}, },
@ -853,7 +860,7 @@ describe('compiler compliance: styling', () => {
}); });
describe('[style] mixed with [class]', () => { describe('[style] mixed with [class]', () => {
it('should combine [style] and [class] bindings into a single instruction', () => { it('should split [style] and [class] bindings into a separate instructions', () => {
const files = { const files = {
app: { app: {
'spec.ts': ` 'spec.ts': `
@ -882,6 +889,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd(); $r3$.ɵɵelementEnd();
} }
if (rf & 2) { if (rf & 2) {
$r3$.ɵɵselect(0);
$r3$.ɵɵelementStyleMap(0, $ctx$.myStyleExp); $r3$.ɵɵelementStyleMap(0, $ctx$.myStyleExp);
$r3$.ɵɵelementClassMap(0, $ctx$.myClassExp); $r3$.ɵɵelementClassMap(0, $ctx$.myClassExp);
$r3$.ɵɵelementStylingApply(0); $r3$.ɵɵelementStylingApply(0);
@ -925,6 +933,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd(); $r3$.ɵɵelementEnd();
} }
if (rf & 2) { if (rf & 2) {
$r3$.ɵɵselect(0);
$r3$.ɵɵelementStyleMap(0, $r3$.ɵɵpipeBind1(1, 0, $ctx$.myStyleExp)); $r3$.ɵɵelementStyleMap(0, $r3$.ɵɵpipeBind1(1, 0, $ctx$.myStyleExp));
$r3$.ɵɵelementClassMap(0, $r3$.ɵɵpipeBind1(2, 2, $ctx$.myClassExp)); $r3$.ɵɵelementClassMap(0, $r3$.ɵɵpipeBind1(2, 2, $ctx$.myClassExp));
$r3$.ɵɵelementStylingApply(0); $r3$.ɵɵelementStylingApply(0);
@ -982,6 +991,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd(); $r3$.ɵɵelementEnd();
} }
if (rf & 2) { if (rf & 2) {
$r3$.ɵɵselect(0);
$r3$.ɵɵelementStyleMap(0, $r3$.ɵɵpipeBind2(1, 1, $ctx$.myStyleExp, 1000)); $r3$.ɵɵelementStyleMap(0, $r3$.ɵɵpipeBind2(1, 1, $ctx$.myStyleExp, 1000));
$r3$.ɵɵelementClassMap(0, $e2_styling$); $r3$.ɵɵelementClassMap(0, $e2_styling$);
$r3$.ɵɵelementStyleProp(0, 0, $r3$.ɵɵpipeBind2(2, 4, $ctx$.barExp, 3000)); $r3$.ɵɵelementStyleProp(0, 0, $r3$.ɵɵpipeBind2(2, 4, $ctx$.barExp, 3000));
@ -997,6 +1007,59 @@ describe('compiler compliance: styling', () => {
const result = compile(files, angularFiles); const result = compile(files, angularFiles);
expectEmit(result.source, template, 'Incorrect template'); expectEmit(result.source, template, 'Incorrect template');
}); });
it('should always generate select() statements before any styling instructions', () => {
const files = {
app: {
'spec.ts': `
import {Component, NgModule} from '@angular/core';
@Component({
selector: 'my-component',
template: \`
<div [style.width]="w1"></div>
<div [style.height]="h1"></div>
<div [class.active]="a1"></div>
<div [class.removed]="r1"></div>
\`
})
export class MyComponent {
w1 = '100px';
h1 = '100px';
a1 = true;
r1 = true;
}
@NgModule({declarations: [MyComponent]})
export class MyModule {}
`
}
};
const template = `
template: function MyComponent_Template(rf, $ctx$) {
if (rf & 2) {
$r3$.ɵɵselect(0);
$r3$.ɵɵelementStyleProp(0, 0, $ctx$.w1);
$r3$.ɵɵelementStylingApply(0);
$r3$.ɵɵselect(1);
$r3$.ɵɵelementStyleProp(1, 0, $ctx$.h1);
$r3$.ɵɵelementStylingApply(1);
$r3$.ɵɵselect(2);
$r3$.ɵɵelementClassProp(2, 0, $ctx$.a1);
$r3$.ɵɵelementStylingApply(2);
$r3$.ɵɵselect(3);
$r3$.ɵɵelementClassProp(3, 0, $ctx$.r1);
$r3$.ɵɵelementStylingApply(3);
}
}
`;
const result = compile(files, angularFiles);
expectEmit(result.source, template, 'Incorrect template');
});
}); });
describe('@Component host styles/classes', () => { describe('@Component host styles/classes', () => {
@ -1171,6 +1234,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd(); $r3$.ɵɵelementEnd();
} }
if (rf & 2) { if (rf & 2) {
$r3$.ɵɵselect(0);
$r3$.ɵɵelementStyleMap(0, ctx.myStyleExp); $r3$.ɵɵelementStyleMap(0, ctx.myStyleExp);
$r3$.ɵɵelementClassMap(0, ctx.myClassExp); $r3$.ɵɵelementClassMap(0, ctx.myClassExp);
$r3$.ɵɵelementStyleProp(0, 0, ctx.myHeightExp, null, true); $r3$.ɵɵelementStyleProp(0, 0, ctx.myHeightExp, null, true);

View File

@ -666,7 +666,7 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
// (things like `elementStyleProp`, `elementClassProp`, etc..) are applied later on in this // (things like `elementStyleProp`, `elementClassProp`, etc..) are applied later on in this
// file // file
this.processStylingInstruction( this.processStylingInstruction(
implicit, elementIndex, implicit,
stylingBuilder.buildElementStylingInstruction(element.sourceSpan, this.constantPool), stylingBuilder.buildElementStylingInstruction(element.sourceSpan, this.constantPool),
true); true);
@ -688,10 +688,13 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
// update block of the template function AOT code. Instructions like `elementStyleProp`, // update block of the template function AOT code. Instructions like `elementStyleProp`,
// `elementStyleMap`, `elementClassMap`, `elementClassProp` and `elementStylingApply` // `elementStyleMap`, `elementClassMap`, `elementClassProp` and `elementStylingApply`
// are all generated and assigned in the code below. // are all generated and assigned in the code below.
stylingBuilder.buildUpdateLevelInstructions(this._valueConverter).forEach(instruction => { const stylingInstructions = stylingBuilder.buildUpdateLevelInstructions(this._valueConverter);
const limit = stylingInstructions.length - 1;
for (let i = 0; i <= limit; i++) {
const instruction = stylingInstructions[i];
this._bindingSlots += instruction.allocateBindingSlots; this._bindingSlots += instruction.allocateBindingSlots;
this.processStylingInstruction(implicit, instruction, false); this.processStylingInstruction(elementIndex, implicit, instruction, false);
}); }
// the reason why `undefined` is used is because the renderer understands this as a // the reason why `undefined` is used is because the renderer understands this as a
// special value to symbolize that there is no RHS to this binding // special value to symbolize that there is no RHS to this binding
@ -1007,14 +1010,15 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
} }
private processStylingInstruction( private processStylingInstruction(
implicit: any, instruction: Instruction|null, createMode: boolean) { elementIndex: number, implicit: any, instruction: Instruction|null, createMode: boolean) {
if (instruction) { if (instruction) {
const paramsFn = () => const paramsFn = () =>
instruction.buildParams(value => this.convertPropertyBinding(implicit, value, true)); instruction.buildParams(value => this.convertPropertyBinding(implicit, value, true));
if (createMode) { if (createMode) {
this.creationInstruction(instruction.sourceSpan, instruction.reference, paramsFn); this.creationInstruction(instruction.sourceSpan, instruction.reference, paramsFn);
} else { } else {
this.updateInstruction(-1, instruction.sourceSpan, instruction.reference, paramsFn); this.updateInstruction(
elementIndex, instruction.sourceSpan, instruction.reference, paramsFn);
} }
} }
} }