fix(ivy): don't match directives against attribute bindings (#31541)

Fixes Ivy matching directives against attribute bindings (e.g. `[attr.some-directive]="foo"`). Works by excluding attribute bindings from the attributes array during compilation. This has the added benefit of generating less code.

**Note:** My initial approach to implementing this was to have a different marker for attribute bindings so that they can be ignored when matching directives, however as I was implementing it I realized that the attributes in that array were only used for directive matching (as far as I could tell). I decided to drop the attribute bindings completely, because it results in less generated code.

PR Close #31541
This commit is contained in:
crisbeto 2019-07-13 13:34:34 +02:00 committed by Matias Niemelä
parent 9e83822679
commit 12fd06916b
6 changed files with 70 additions and 9 deletions

View File

@ -612,6 +612,41 @@ describe('compiler compliance: bindings', () => {
expectEmit(result.source, template, 'Incorrect template'); expectEmit(result.source, template, 'Incorrect template');
}); });
it('should exclude attribute bindings from the attributes array', () => {
const files: MockDirectory = {
app: {
'example.ts': `
import {Component, NgModule} from '@angular/core';
@Component({
selector: 'my-app',
template: \`<a
target="_blank"
[title]="1"
[attr.foo]="'one'"
(customEvent)="doThings()"
[attr.bar]="'two'"
[id]="2"
aria-label="link"
[attr.baz]="three"></a>\`
})
export class MyComponent {
doThings() {}
}
@NgModule({declarations: [MyComponent]})
export class MyModule {}`
}
};
const template = `
const $e0_attrs$ = ["target", "_blank", "aria-label", "link", ${AttributeMarker.Bindings}, "title", "id", "customEvent"];
`;
const result = compile(files, angularFiles);
expectEmit(result.source, template, 'Incorrect attribute array');
});
}); });
describe('host bindings', () => { describe('host bindings', () => {

View File

@ -333,7 +333,7 @@ describe('i18n support in the view compiler', () => {
`; `;
const output = ` const output = `
const $_c0$ = [${AttributeMarker.Bindings}, "title", "label"]; const $_c0$ = [3, "title"];
template: function MyComponent_Template(rf, ctx) { template: function MyComponent_Template(rf, ctx) {
if (rf & 1) { if (rf & 1) {

View File

@ -520,7 +520,7 @@ describe('compiler compliance: styling', () => {
}; };
const template = ` const template = `
const $_c0$ = [${AttributeMarker.Styles}, "opacity", "1", ${AttributeMarker.Bindings}, "style"]; const $_c0$ = [${AttributeMarker.Styles}, "opacity", "1"];
const $_c1$ = ["width", "height"]; const $_c1$ = ["width", "height"];
MyComponent.ngComponentDef = $r3$.ɵɵdefineComponent({ MyComponent.ngComponentDef = $r3$.ɵɵdefineComponent({
@ -741,7 +741,7 @@ describe('compiler compliance: styling', () => {
}; };
const template = ` const template = `
const $e0_attrs$ = [${AttributeMarker.Classes}, "grape", ${AttributeMarker.Bindings}, "class"]; const $e0_attrs$ = [${AttributeMarker.Classes}, "grape"];
const $e0_bindings$ = ["apple", "orange"]; const $e0_bindings$ = ["apple", "orange"];
MyComponent.ngComponentDef = $r3$.ɵɵdefineComponent({ MyComponent.ngComponentDef = $r3$.ɵɵdefineComponent({
@ -797,7 +797,7 @@ describe('compiler compliance: styling', () => {
}; };
const template = ` const template = `
const $e0_attrs$ = [${AttributeMarker.Classes}, "foo", ${AttributeMarker.Styles}, "width", "100px", ${AttributeMarker.Bindings}, "class", "style"]; const $e0_attrs$ = [${AttributeMarker.Classes}, "foo", ${AttributeMarker.Styles}, "width", "100px"];
MyComponent.ngComponentDef = $r3$.ɵɵdefineComponent({ MyComponent.ngComponentDef = $r3$.ɵɵdefineComponent({
type: MyComponent, type: MyComponent,

View File

@ -1253,11 +1253,13 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
} }
if (inputs.length || outputs.length) { if (inputs.length || outputs.length) {
const attrsStartIndex = attrExprs.length; const attrsLengthBeforeInputs = attrExprs.length;
for (let i = 0; i < inputs.length; i++) { for (let i = 0; i < inputs.length; i++) {
const input = inputs[i]; const input = inputs[i];
if (input.type !== BindingType.Animation) { // We don't want the animation and attribute bindings in the
// attributes array since they aren't used for directive matching.
if (input.type !== BindingType.Animation && input.type !== BindingType.Attribute) {
addAttrExpr(input.name); addAttrExpr(input.name);
} }
} }
@ -1273,8 +1275,8 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
// values have been filtered (by not including the animation ones) and added // values have been filtered (by not including the animation ones) and added
// to the expressions. The marker is important because it tells the runtime // to the expressions. The marker is important because it tells the runtime
// code that this is where attributes without values start... // code that this is where attributes without values start...
if (attrExprs.length) { if (attrExprs.length !== attrsLengthBeforeInputs) {
attrExprs.splice(attrsStartIndex, 0, o.literal(core.AttributeMarker.Bindings)); attrExprs.splice(attrsLengthBeforeInputs, 0, o.literal(core.AttributeMarker.Bindings));
} }
} }

View File

@ -199,7 +199,7 @@ export const enum AttributeMarker {
* ``` * ```
* var _c1 = ['moo', 'car', AttributeMarker.I18n, 'foo', 'bar']; * var _c1 = ['moo', 'car', AttributeMarker.I18n, 'foo', 'bar'];
*/ */
I18n, I18n = 6,
} }
/** /**

View File

@ -202,6 +202,30 @@ describe('directives', () => {
expect(fixture.debugElement.query(By.directive(TestDir))).toBeTruthy(); expect(fixture.debugElement.query(By.directive(TestDir))).toBeTruthy();
}); });
it('should not match directives based on attribute bindings', () => {
const calls: string[] = [];
@Directive({selector: '[dir]'})
class MyDir {
ngOnInit() { calls.push('MyDir.ngOnInit'); }
}
@Component({
selector: `my-comp`,
template: `<p [attr.dir]="direction"></p><p dir="rtl"></p>`,
})
class MyComp {
direction = 'auto';
}
TestBed.configureTestingModule({declarations: [MyDir, MyComp]});
const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges();
// Expect only one directive to be instantiated.
expect(calls).toEqual(['MyDir.ngOnInit']);
});
}); });
describe('outputs', () => { describe('outputs', () => {