fix(ivy): ensure multiple map-based bindings do not skip intermediate values (#32774)

This patch fixes a bug where the map-based cursor moves too far and
skips intermediate values when the correct combination of single-prop
bindings and map-based bindings are used together.

PR Close #32774
This commit is contained in:
Matias Niemelä 2019-09-19 11:23:19 -07:00 committed by Andrew Kushnir
parent 32f4544f34
commit 86fd5719b5
3 changed files with 79 additions and 15 deletions

View File

@ -152,7 +152,7 @@ function updateBindingData(
const doSetValuesAsStale = (getConfig(context) & TStylingConfig.HasHostBindings) && const doSetValuesAsStale = (getConfig(context) & TStylingConfig.HasHostBindings) &&
!hostBindingsMode && (prop ? !value : true); !hostBindingsMode && (prop ? !value : true);
if (doSetValuesAsStale) { if (doSetValuesAsStale) {
renderHostBindingsAsStale(context, data, prop, !prop); renderHostBindingsAsStale(context, data, prop);
} }
} }
return changed; return changed;
@ -170,28 +170,32 @@ function updateBindingData(
* binding changes. * binding changes.
*/ */
function renderHostBindingsAsStale( function renderHostBindingsAsStale(
context: TStylingContext, data: LStylingData, prop: string | null, isMapBased: boolean): void { context: TStylingContext, data: LStylingData, prop: string | null): void {
const valuesCount = getValuesCount(context); const valuesCount = getValuesCount(context);
if (hasConfig(context, TStylingConfig.HasPropBindings)) { if (prop !== null && hasConfig(context, TStylingConfig.HasPropBindings)) {
const itemsPerRow = TStylingContextIndex.BindingsStartOffset + valuesCount; const itemsPerRow = TStylingContextIndex.BindingsStartOffset + valuesCount;
let i = TStylingContextIndex.ValuesStartPosition; let i = TStylingContextIndex.ValuesStartPosition;
let found = false;
while (i < context.length) { while (i < context.length) {
if (getProp(context, i) === prop) { if (getProp(context, i) === prop) {
found = true;
break; break;
} }
i += itemsPerRow; i += itemsPerRow;
} }
const bindingsStart = i + TStylingContextIndex.BindingsStartOffset; if (found) {
const valuesStart = bindingsStart + 1; // the first column is template bindings const bindingsStart = i + TStylingContextIndex.BindingsStartOffset;
const valuesEnd = bindingsStart + valuesCount - 1; const valuesStart = bindingsStart + 1; // the first column is template bindings
const valuesEnd = bindingsStart + valuesCount - 1;
for (let i = valuesStart; i < valuesEnd; i++) { for (let i = valuesStart; i < valuesEnd; i++) {
const bindingIndex = context[i] as number; const bindingIndex = context[i] as number;
if (bindingIndex !== 0) { if (bindingIndex !== 0) {
setValue(data, bindingIndex, null); setValue(data, bindingIndex, null);
}
} }
} }
} }
@ -575,9 +579,18 @@ export function applyStylingViaContext(
// determine whether or not to apply the target property or to skip it // determine whether or not to apply the target property or to skip it
let mode = mapsMode | (valueApplied ? StylingMapsSyncMode.SkipTargetProp : let mode = mapsMode | (valueApplied ? StylingMapsSyncMode.SkipTargetProp :
StylingMapsSyncMode.ApplyTargetProp); StylingMapsSyncMode.ApplyTargetProp);
// the first column in the context (when `j == 0`) is special-cased for
// template bindings. If and when host bindings are being processed then
// the first column will still be iterated over, but the values will only
// be checked against (not applied). If and when this happens we need to
// notify the map-based syncing code to know not to apply the values it
// comes across in the very first map-based binding (which is also located
// in column zero).
if (hostBindingsMode && j === 0) { if (hostBindingsMode && j === 0) {
mode |= StylingMapsSyncMode.CheckValuesOnly; mode |= StylingMapsSyncMode.CheckValuesOnly;
} }
const valueAppliedWithinMap = stylingMapsSyncFn( const valueAppliedWithinMap = stylingMapsSyncFn(
context, renderer, element, bindingData, j, applyStylingFn, sanitizer, mode, prop, context, renderer, element, bindingData, j, applyStylingFn, sanitizer, mode, prop,
defaultValue); defaultValue);

View File

@ -14,7 +14,6 @@ import {getBindingValue, getMapProp, getMapValue, getValue, getValuesCount, isSt
import {setStylingMapsSyncFn} from './bindings'; import {setStylingMapsSyncFn} from './bindings';
/** /**
* -------- * --------
* *
@ -267,19 +266,31 @@ function innerSyncStylingMap(
* *
* - value is being applied: * - value is being applied:
* if the value is being applied from this current styling * if the value is being applied from this current styling
* map then there is no need to apply it in a deeper map. * map then there is no need to apply it in a deeper map
* (i.e. the `SkipTargetProp` flag is set)
* *
* - value is being not applied: * - value is being not applied:
* apply the value if it is found in a deeper map. * apply the value if it is found in a deeper map.
* (i.e. the `SkipTargetProp` flag is unset)
* *
* When these reasons are encountered the flags will for the * When these reasons are encountered the flags will for the
* inner map mode will be configured. * inner map mode will be configured.
*/ */
function resolveInnerMapMode( function resolveInnerMapMode(
currentMode: number, valueIsDefined: boolean, isExactMatch: boolean): number { currentMode: number, valueIsDefined: boolean, isTargetPropMatched: boolean): number {
let innerMode = currentMode; let innerMode = currentMode;
if (!valueIsDefined && !(currentMode & StylingMapsSyncMode.SkipTargetProp) &&
(isExactMatch || (currentMode & StylingMapsSyncMode.ApplyAllValues))) { // the statements below figures out whether or not an inner styling map
// is allowed to apply its value or not. The main thing to keep note
// of is that if the target prop isn't matched then its expected that
// all values before it are allowed to be applied so long as "apply all values"
// is set to true.
const applyAllValues = currentMode & StylingMapsSyncMode.ApplyAllValues;
const applyTargetProp = currentMode & StylingMapsSyncMode.ApplyTargetProp;
const allowInnerApply =
!valueIsDefined && (isTargetPropMatched ? applyTargetProp : applyAllValues);
if (allowInnerApply) {
// case 1: set the mode to apply the targeted prop value if it // case 1: set the mode to apply the targeted prop value if it
// ends up being encountered in another map value // ends up being encountered in another map value
innerMode |= StylingMapsSyncMode.ApplyTargetProp; innerMode |= StylingMapsSyncMode.ApplyTargetProp;

View File

@ -1995,6 +1995,46 @@ describe('styling', () => {
expect(readyHost).toBeTruthy(); expect(readyHost).toBeTruthy();
expect(readyChild).toBeTruthy(); expect(readyChild).toBeTruthy();
}); });
onlyInIvy('only ivy allows for multiple styles/classes to be balanaced across directives')
.it('should allow various duplicate properties to be defined in various styling maps within the template and directive styling bindings',
() => {
@Component({
template: `
<div [style.width]="w"
[style.height]="h"
[style]="s1"
[dir-with-styling]="s2">
`
})
class Cmp {
h = '100px';
w = '100px';
s1: any = {border: '10px solid black', width: '200px'};
s2: any = {border: '10px solid red', width: '300px'};
}
@Directive({selector: '[dir-with-styling]'})
class DirectiveExpectingStyling {
@Input('dir-with-styling') @HostBinding('style') public styles: any = null;
}
TestBed.configureTestingModule({declarations: [Cmp, DirectiveExpectingStyling]});
const fixture = TestBed.createComponent(Cmp);
fixture.detectChanges();
const element = fixture.nativeElement.querySelector('div');
expect(element.style.border).toEqual('10px solid black');
expect(element.style.width).toEqual('100px');
expect(element.style.height).toEqual('100px');
fixture.componentInstance.s1 = null;
fixture.detectChanges();
expect(element.style.border).toEqual('10px solid red');
expect(element.style.width).toEqual('100px');
expect(element.style.height).toEqual('100px');
});
}); });
function getDebugNode(element: Node): DebugNode|null { function getDebugNode(element: Node): DebugNode|null {