fix(ivy): host-styling throws assert exception inside *ngFor (#35133)

Inside `*ngFor` the second run of the styling instructions can get into situation where it tries to read a value from a binding which has not yet executed. As a result the read is `NO_CHANGE` value and subsequent property read cause an exception as it is of wrong type.

Fix #35118

PR Close #35133
This commit is contained in:
Miško Hevery 2020-02-03 12:23:00 -08:00
parent a8609ba0ad
commit ab931cf872
3 changed files with 63 additions and 2 deletions

View File

@ -834,8 +834,20 @@ function findStylingValue(
const containsStatics = Array.isArray(rawKey); const containsStatics = Array.isArray(rawKey);
// Unwrap the key if we contain static values. // Unwrap the key if we contain static values.
const key = containsStatics ? (rawKey as string[])[1] : rawKey; const key = containsStatics ? (rawKey as string[])[1] : rawKey;
let currentValue = key === null ? keyValueArrayGet(lView[index + 1], prop) : const isStylingMap = key === null;
key === prop ? lView[index + 1] : undefined; let valueAtLViewIndex = lView[index + 1];
if (valueAtLViewIndex === NO_CHANGE) {
// In firstUpdatePass the styling instructions create a linked list of styling.
// On subsequent passes it is possible for a styling instruction to try to read a binding
// which
// has not yet executed. In that case we will find `NO_CHANGE` and we should assume that
// we have `undefined` (or empty array in case of styling-map instruction) instead. This
// allows the resolution to apply the value (which may later be overwritten when the
// binding actually executes.)
valueAtLViewIndex = isStylingMap ? EMPTY_ARRAY : undefined;
}
let currentValue = isStylingMap ? keyValueArrayGet(valueAtLViewIndex, prop) :
key === prop ? valueAtLViewIndex : undefined;
if (containsStatics && !isStylingValuePresent(currentValue)) { if (containsStatics && !isStylingValuePresent(currentValue)) {
currentValue = keyValueArrayGet(rawKey as KeyValueArray<any>, prop); currentValue = keyValueArrayGet(rawKey as KeyValueArray<any>, prop);
} }

View File

@ -3273,6 +3273,52 @@ describe('styling', () => {
expect(logs).toEqual([]); expect(logs).toEqual([]);
}); });
describe('regression', () => {
onlyInIvy('styling priority resolution is Ivy only feature.')
.it('should allow lookahead binding on second pass #35118', () => {
@Component({
selector: 'my-cmp',
template: ``,
host: {
'[class.foo]': 'hostClass',
}
})
class MyCmp {
hostClass = true;
}
@Directive({
selector: '[host-styling]',
host: {
'[class]': 'hostClass',
}
})
class HostStylingsDir {
hostClass = {'bar': true};
}
@Component({template: `<my-cmp *ngFor="let i of [1,2]" host-styling></my-cmp>`})
class MyApp {
// When the first view in the list gets CD-ed, everything works.
// When the second view gets CD-ed, the styling has already created the data structures
// in the `TView`. As a result when
// `[class.foo]` runs it already knows that `[class]` is a duplicate and hence it
// should check with it. While the resolution is happening it reads the value of the
// `[class]`, however `[class]` has not yet executed and therefore it does not have
// normalized value in its `LView`. The result is that the assertions fails as it
// expects an
// `KeyValueArray`.
}
TestBed.configureTestingModule({declarations: [MyApp, MyCmp, HostStylingsDir]});
const fixture = TestBed.createComponent(MyApp);
expect(() => fixture.detectChanges()).not.toThrow();
const [cmp1, cmp2] = fixture.nativeElement.querySelectorAll('my-cmp');
expectClass(cmp1).toEqual({foo: true, bar: true});
expectClass(cmp2).toEqual({foo: true, bar: true});
});
});
}); });
function assertStyleCounters(countForSet: number, countForRemove: number) { function assertStyleCounters(countForSet: number, countForRemove: number) {

View File

@ -47,6 +47,9 @@
{ {
"name": "EMPTY_ARRAY" "name": "EMPTY_ARRAY"
}, },
{
"name": "EMPTY_ARRAY"
},
{ {
"name": "EMPTY_OBJ" "name": "EMPTY_OBJ"
}, },