perf(ivy): chain multiple attribute instructions (#31152)

Similarly to what we did in #31078, these changes implement chaining for the `attribute` instruction.

This PR resolves FW-1390.

PR Close #31152
This commit is contained in:
crisbeto 2019-06-24 20:24:32 +02:00 committed by Kara Erickson
parent fcb03abc72
commit 23c017121e
6 changed files with 335 additions and 28 deletions

View File

@ -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);
}
}
`;
@ -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 = {

View File

@ -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

View File

@ -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) => {
@ -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
@ -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
@ -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);
@ -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);
}
}
@ -1083,22 +1092,15 @@ 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();
});
}
private propertyInstructionChain(
nodeIndex: number, propertyBindings: ChainablePropertyBinding[]) {
this.updateInstructionChain(
nodeIndex, propertyBindings.length ? propertyBindings[0].input.sourceSpan : null,
R3.property, () => {
return propertyBindings.map(
const calls = bindings.map(
property => [o.literal(property.name), property.value(), ...(property.params || [])]);
return chainedInstruction(span, reference, calls).toStmt();
});
}
@ -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;

View File

@ -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';
@ -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;
}

View File

@ -73,6 +73,104 @@ describe('attribute binding', () => {
expect(a.href).toEqual('https://angular.io/robots.txt');
});
it('should be able to bind multiple attribute values per element', () => {
@Component({
template: `<a [attr.id]="id" [attr.href]="url" [attr.tabindex]="'-1'"></a>`,
})
class Comp {
url = 'https://angular.io/robots.txt';
id = 'my-link';
}
TestBed.configureTestingModule({declarations: [Comp]});
const fixture = TestBed.createComponent(Comp);
fixture.detectChanges();
const a = fixture.debugElement.query(By.css('a')).nativeElement;
// NOTE: different browsers will add `//` into the URI.
expect(a.getAttribute('href')).toBe('https://angular.io/robots.txt');
expect(a.getAttribute('id')).toBe('my-link');
expect(a.getAttribute('tabindex')).toBe('-1');
});
it('should be able to bind multiple attributes in the presence of other bindings', () => {
@Component({
template: `<a [id]="id" [attr.href]="url" [title]="'hello'"></a>`,
})
class Comp {
url = 'https://angular.io/robots.txt';
id = 'my-link';
}
TestBed.configureTestingModule({declarations: [Comp]});
const fixture = TestBed.createComponent(Comp);
fixture.detectChanges();
const a = fixture.debugElement.query(By.css('a')).nativeElement;
// NOTE: different browsers will add `//` into the URI.
expect(a.getAttribute('href')).toBe('https://angular.io/robots.txt');
expect(a.id).toBe('my-link');
expect(a.getAttribute('title')).toBe('hello');
});
it('should be able to bind attributes with interpolations', () => {
@Component({
template: `
<button
attr.id="my-{{id}}-button"
[attr.title]="title"
attr.tabindex="{{1 + 3 + 7}}"></button>`,
})
class Comp {
title = 'hello';
id = 'custom';
}
TestBed.configureTestingModule({declarations: [Comp]});
const fixture = TestBed.createComponent(Comp);
fixture.detectChanges();
const button = fixture.debugElement.query(By.css('button')).nativeElement;
expect(button.getAttribute('id')).toBe('my-custom-button');
expect(button.getAttribute('tabindex')).toBe('11');
expect(button.getAttribute('title')).toBe('hello');
});
it('should be able to bind attributes both to parent and child nodes', () => {
@Component({
template: `
<button
attr.id="my-{{id}}-button"
[attr.title]="title"
attr.tabindex="{{1 + 3 + 7}}">
<span attr.title="span-{{title}}" id="custom-span" [attr.tabindex]="-1"></span>
</button>
`,
})
class Comp {
title = 'hello';
id = 'custom';
}
TestBed.configureTestingModule({declarations: [Comp]});
const fixture = TestBed.createComponent(Comp);
fixture.detectChanges();
const button = fixture.debugElement.query(By.css('button')).nativeElement;
const span = fixture.debugElement.query(By.css('span')).nativeElement;
expect(button.getAttribute('id')).toBe('my-custom-button');
expect(button.getAttribute('tabindex')).toBe('11');
expect(button.getAttribute('title')).toBe('hello');
expect(span.getAttribute('id')).toBe('custom-span');
expect(span.getAttribute('tabindex')).toBe('-1');
expect(span.getAttribute('title')).toBe('span-hello');
});
it('should sanitize attribute values', () => {
@Component({
template: `<a [attr.href]="badUrl"></a>`,

View File

@ -665,7 +665,7 @@ export interface OutputDecorator {
export declare function ɵɵallocHostVars(count: number): void;
export declare function ɵɵattribute(name: string, value: any, sanitizer?: SanitizerFn | null, namespace?: string): void;
export declare function ɵɵattribute(name: string, value: any, sanitizer?: SanitizerFn | null, namespace?: string): TsickleIssue1009;
export declare function ɵɵattributeInterpolate1(attrName: string, prefix: string, v0: any, suffix: string, sanitizer?: SanitizerFn, namespace?: string): TsickleIssue1009;