fix(ivy): classes should not mess up matching for bound dir attributes (#30002)

Previously, we had a bug where directive matching could fail if the directive
attribute was bound and followed a certain number of classes. This is because
in the matching logic, we were treating classes like normal attributes. We
should instead be skipping classes in the attribute matching logic. Otherwise
classes will match for directives with attribute selectors, and as we are
iterating through them in twos (when they are stored as name-only, not in
name-value pairs), it may throw off directive matching for any bound attributes
that come after. This commit changes the directive matching logic to skip
classes altogether.

PR Close #30002
This commit is contained in:
Kara Erickson 2019-04-19 18:30:54 -07:00 committed by Ben Lesh
parent 96a828993f
commit f53d0fd2d0
3 changed files with 57 additions and 1 deletions

View File

@ -205,6 +205,14 @@ function findAttrIndexInNode(
return i;
} else if (maybeAttrName === AttributeMarker.Bindings) {
bindingsMode = true;
} else if (maybeAttrName === AttributeMarker.Classes) {
let value = attrs[++i];
// We should skip classes here because we have a separate mechanism for
// matching classes in projection mode.
while (typeof value === 'string') {
value = attrs[++i];
}
continue;
} else if (maybeAttrName === AttributeMarker.Template) {
// We do not care about Template attributes in this scenario.
break;

View File

@ -58,6 +58,30 @@ describe('directives', () => {
expect(nodesWithDirective.length).toBe(1);
});
it('should match a mix of bound directives and classes', () => {
TestBed.configureTestingModule({declarations: [TestComponent, TitleDirective]});
TestBed.overrideTemplate(TestComponent, `
<div class="one two" [id]="someId" [title]="title"></div>
`);
const fixture = TestBed.createComponent(TestComponent);
const nodesWithDirective = fixture.debugElement.queryAllNodes(By.directive(TitleDirective));
expect(nodesWithDirective.length).toBe(1);
});
it('should NOT match classes to directive selectors', () => {
TestBed.configureTestingModule({declarations: [TestComponent, TitleDirective]});
TestBed.overrideTemplate(TestComponent, `
<div class="title" [id]="someId"></div>
`);
const fixture = TestBed.createComponent(TestComponent);
const nodesWithDirective = fixture.debugElement.queryAllNodes(By.directive(TitleDirective));
expect(nodesWithDirective.length).toBe(0);
});
});
});
});

View File

@ -201,6 +201,30 @@ describe('css selector matching', () => {
.toBeFalsy(
`Selector '[directive=value]' should not match <span [directive]="exp" [value]="otherExp">`);
});
it('should match bound attributes that come after classes', () => {
expect(isMatching(
'span',
[
AttributeMarker.Classes, 'my-class', 'other-class', AttributeMarker.Bindings,
'title', 'directive'
],
['', 'directive', '']))
.toBeTruthy(
`Selector '[directive]' should match <span class="my-class other-class" [title]="title" [directive]="exp">`);
});
it('should match NOT match classes when looking for directives', () => {
expect(isMatching(
'span',
[
AttributeMarker.Classes, 'directive', 'other-class', AttributeMarker.Bindings,
'title'
],
['', 'directive', '']))
.toBeFalsy(
`Selector '[directive]' should NOT match <span class="directive other-class" [title]="title">`);
});
});
describe('class matching', () => {