Skip to content

Commit

Permalink
perf(ivy): chain multiple attribute instructions (#31152)
Browse files Browse the repository at this point in the history 
Similarly to what we did in #31078, these changes implement chaining for the `attribute` instruction.

This PR resolves FW-1390.

PR Close #31152
  • Loading branch information
@crisbeto @kara
crisbeto authored and kara committed Jun 24, 2019
1 parent fcb03ab commit 23c0171
Show file tree
Hide file tree
Showing 6 changed files with 335 additions and 28 deletions.
208 changes: 207 additions & 1 deletion packages/compiler-cli/test/compliance/r3_view_compiler_binding_spec.ts
View file
Expand Up @@ -229,9 +229,9 @@ describe('compiler compliance: bindings', () => {
template: function MyComponent_Template(rf, ctx) {
…
if (rf & 2) {
$r3$.ɵɵattribute("id", 2);
$r3$.ɵɵpropertyInterpolate("aria-label", 1 + 3);
$r3$.ɵɵproperty("title", 1)("tabindex", 3);
$r3$.ɵɵattribute("id", 2);
}
}
`;
Expand Down Expand Up @@ -408,6 +408,212 @@ describe('compiler compliance: bindings', () => {

});

describe('attribute bindings', () => {
it('should chain multiple attribute bindings into a single instruction', () => {
const files = {
app: {
'example.ts': `
import {Component} from '@angular/core';
@Component({
template: \`
<button [attr.title]="myTitle" attr.id="{{buttonId}}" [attr.tabindex]="1"></button>
\`
})
export class MyComponent {
myTitle = 'hello';
buttonId = 'special-button';
}`
}
};

const result = compile(files, angularFiles);
const template = `
…
template: function MyComponent_Template(rf, ctx) {
…
if (rf & 2) {
$r3$.ɵɵattribute("title", ctx.myTitle)("id", ctx.buttonId)("tabindex", 1);
}
}
`;

expectEmit(result.source, template, 'Incorrect template');
});

it('should chain multiple single-interpolation attribute bindings into one instruction', () => {
const files = {
app: {
'example.ts': `
import {Component} from '@angular/core';
@Component({
template: \`
<button attr.title="{{myTitle}}" attr.id="{{buttonId}}" attr.tabindex="{{1}}"></button>
\`
})
export class MyComponent {
myTitle = 'hello';
buttonId = 'special-button';
}`
}
};

const result = compile(files, angularFiles);
const template = `
…
template: function MyComponent_Template(rf, ctx) {
…
if (rf & 2) {
$r3$.ɵɵattribute("title", ctx.myTitle)("id", ctx.buttonId)("tabindex", 1);
}
}
`;

expectEmit(result.source, template, 'Incorrect template');
});

it('should chain attribute bindings in the presence of other instructions', () => {
const files = {
app: {
'example.ts': `
import {Component} from '@angular/core';
@Component({
template: \`
<button [attr.title]="1" [id]="2" [attr.tabindex]="3" attr.aria-label="prefix-{{1 + 3}}">
</button>
\`
})
export class MyComponent {}`
}
};

const result = compile(files, angularFiles);
const template = `
…
template: function MyComponent_Template(rf, ctx) {
…
if (rf & 2) {
$r3$.ɵɵattributeInterpolate1("aria-label", "prefix-", 1 + 3, "");
$r3$.ɵɵproperty("id", 2);
$r3$.ɵɵattribute("title", 1)("tabindex", 3);
}
}
`;

expectEmit(result.source, template, 'Incorrect template');
});

it('should not add interpolated attributes to the attribute instruction chain', () => {
const files = {
app: {
'example.ts': `
import {Component} from '@angular/core';
@Component({
template: \`
<button
[attr.title]="1"
[attr.id]="2"
attr.tabindex="prefix-{{0 + 3}}"
attr.aria-label="hello-{{1 + 3}}-{{2 + 3}}"></button>\`
})
export class MyComponent {}`
}
};

const result = compile(files, angularFiles);
const template = `
…
template: function MyComponent_Template(rf, ctx) {
…
if (rf & 2) {
$r3$.ɵɵattributeInterpolate1("tabindex", "prefix-", 0 + 3, "");
$r3$.ɵɵattributeInterpolate2("aria-label", "hello-", 1 + 3, "-", 2 + 3, "");
$r3$.ɵɵattribute("title", 1)("id", 2);
}
}
`;

expectEmit(result.source, template, 'Incorrect template');
});

it('should chain multiple attribute bindings when there are multiple elements', () => {
const files = {
app: {
'example.ts': `
import {Component} from '@angular/core';
@Component({
template: \`
<button [attr.title]="myTitle" [attr.id]="buttonId" [attr.tabindex]="1"></button>
<span [attr.id]="1" [attr.title]="'hello'" [attr.some-attr]="1 + 2"></span>
<custom-element [attr.some-attr]="'one'" [attr.some-other-attr]="2"></custom-element>
\`
})
export class MyComponent {
myTitle = 'hello';
buttonId = 'special-button';
}`
}
};

const result = compile(files, angularFiles);
const template = `
…
template: function MyComponent_Template(rf, ctx) {
…
if (rf & 2) {
$r3$.ɵɵattribute("title", ctx.myTitle)("id", ctx.buttonId)("tabindex", 1);
$r3$.ɵɵselect(1);
$r3$.ɵɵattribute("id", 1)("title", "hello")("some-attr", 1 + 2);
$r3$.ɵɵselect(2);
$r3$.ɵɵattribute("some-attr", "one")("some-other-attr", 2);
}
}
`;

expectEmit(result.source, template, 'Incorrect template');
});

it('should chain multiple attribute bindings when there are child elements', () => {
const files = {
app: {
'example.ts': `
import {Component} from '@angular/core';
@Component({
template: \`
<button [attr.title]="myTitle" [attr.id]="buttonId" [attr.tabindex]="1">
<span [attr.id]="1" [attr.title]="'hello'" [attr.some-attr]="1 + 2"></span>
</button>\`
})
export class MyComponent {
myTitle = 'hello';
buttonId = 'special-button';
}`
}
};

const result = compile(files, angularFiles);
const template = `
…
template: function MyComponent_Template(rf, ctx) {
…
if (rf & 2) {
$r3$.ɵɵattribute("title", ctx.myTitle)("id", ctx.buttonId)("tabindex", 1);
$r3$.ɵɵselect(1);
$r3$.ɵɵattribute("id", 1)("title", "hello")("some-attr", 1 + 2);
}
}
`;

expectEmit(result.source, template, 'Incorrect template');
});

});

describe('host bindings', () => {
it('should support host bindings', () => {
const files = {
Expand Down
3 changes: 1 addition & 2 deletions packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts
View file
Expand Up @@ -812,8 +812,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelement(0, "div", $e0_attrs$);
}
if (rf & 2) {
$r3$.ɵɵattribute("class", "round");
$r3$.ɵɵattribute("style", "height:100px", $r3$.ɵɵsanitizeStyle);
$r3$.ɵɵattribute("class", "round")("style", "height:100px", $r3$.ɵɵsanitizeStyle);
}
},
encapsulation: 2
Expand Down
44 changes: 23 additions & 21 deletions packages/compiler/src/render3/view/template.ts
View file
Expand Up @@ -711,7 +711,8 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
// special value to symbolize that there is no RHS to this binding
// TODO (matsko): revisit this once FW-959 is approached
const emptyValueBindInstruction = o.literal(undefined);
const propertyBindings: ChainablePropertyBinding[] = [];
const propertyBindings: ChainableBindingInstruction[] = [];
const attributeBindings: ChainableBindingInstruction[] = [];

// Generate element input bindings
allOtherInputs.forEach((input: t.BoundAttribute) => {
Expand Down Expand Up @@ -781,8 +782,12 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
} else {
const boundValue = value instanceof Interpolation ? value.expressions[0] : value;
// [attr.name]="value" or attr.name="{{value}}"
this.boundUpdateInstruction(
R3.attribute, elementIndex, attrName, input, boundValue, params);
// Collect the attribute bindings so that they can be chained at the end.
attributeBindings.push({
name: attrName,
input,
value: () => this.convertPropertyBinding(boundValue), params
});
}
} else {
// class prop
Expand All @@ -798,7 +803,11 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
});

if (propertyBindings.length > 0) {
this.propertyInstructionChain(elementIndex, propertyBindings);
this.updateInstructionChain(elementIndex, R3.property, propertyBindings);
}

if (attributeBindings.length > 0) {
this.updateInstructionChain(elementIndex, R3.attribute, attributeBindings);
}

// Traverse element child nodes
Expand Down Expand Up @@ -1025,7 +1034,7 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver

private templatePropertyBindings(
templateIndex: number, attrs: (t.BoundAttribute|t.TextAttribute)[]) {
const propertyBindings: ChainablePropertyBinding[] = [];
const propertyBindings: ChainableBindingInstruction[] = [];
attrs.forEach(input => {
if (input instanceof t.BoundAttribute) {
const value = input.value.visit(this._valueConverter);
Expand All @@ -1039,7 +1048,7 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
});

if (propertyBindings.length > 0) {
this.propertyInstructionChain(templateIndex, propertyBindings);
this.updateInstructionChain(templateIndex, R3.property, propertyBindings);
}
}

Expand Down Expand Up @@ -1083,23 +1092,16 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
}

private updateInstructionChain(
nodeIndex: number, span: ParseSourceSpan|null, reference: o.ExternalReference,
callsOrFn?: o.Expression[][]|(() => o.Expression[][])) {
nodeIndex: number, reference: o.ExternalReference, bindings: ChainableBindingInstruction[]) {
const span = bindings.length ? bindings[0].input.sourceSpan : null;

this.addSelectInstructionIfNecessary(nodeIndex, span);
this._updateCodeFns.push(() => {
const calls = typeof callsOrFn === 'function' ? callsOrFn() : callsOrFn;
return chainedInstruction(span, reference, calls || []).toStmt();
});
}
const calls = bindings.map(
property => [o.literal(property.name), property.value(), ...(property.params || [])]);

private propertyInstructionChain(
nodeIndex: number, propertyBindings: ChainablePropertyBinding[]) {
this.updateInstructionChain(
nodeIndex, propertyBindings.length ? propertyBindings[0].input.sourceSpan : null,
R3.property, () => {
return propertyBindings.map(
property => [o.literal(property.name), property.value(), ...(property.params || [])]);
});
return chainedInstruction(span, reference, calls).toStmt();
});
}

private addSelectInstructionIfNecessary(nodeIndex: number, span: ParseSourceSpan|null) {
Expand Down Expand Up @@ -2024,7 +2026,7 @@ function hasTextChildrenOnly(children: t.Node[]): boolean {
return children.every(isTextNode);
}

interface ChainablePropertyBinding {
interface ChainableBindingInstruction {
name: string;
input: t.BoundAttribute;
value: () => o.Expression;
Expand Down
8 changes: 5 additions & 3 deletions packages/core/src/render3/instructions/attribute.ts
View file
Expand Up @@ -10,7 +10,7 @@ import {getLView, getSelectedIndex} from '../state';
import {NO_CHANGE} from '../tokens';

import {bind} from './property';
import {elementAttributeInternal} from './shared';
import {TsickleIssue1009, elementAttributeInternal} from './shared';



Expand All @@ -28,12 +28,14 @@ import {elementAttributeInternal} from './shared';
* @codeGenApi
*/
export function ɵɵattribute(
name: string, value: any, sanitizer?: SanitizerFn | null, namespace?: string) {
name: string, value: any, sanitizer?: SanitizerFn | null,
namespace?: string): TsickleIssue1009 {
const index = getSelectedIndex();
const lView = getLView();
// TODO(FW-1340): Refactor to remove the use of other instructions here.
const bound = bind(lView, value);
if (bound !== NO_CHANGE) {
return elementAttributeInternal(index, name, bound, lView, sanitizer, namespace);
elementAttributeInternal(index, name, bound, lView, sanitizer, namespace);
}
return ɵɵattribute;
}

0 comments on commit 23c0171

Please sign in to comment.