honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(ivy): ensure binding ordering doesn't mess up when a `NO_CHANGE` value is encountered (#32143) 

Prior to this fix if a `NO_CHANGE` value was assigned to a binding, or
an interpolation value rendererd a `NO_CHANGE` value, then the presence
of that value would cause the internal counter index values to not
increment properly. This patch ensures that this doesn't happen and
that the counter/bitmask values update accordingly.

PR Close #32143
This commit is contained in:
Matias Niemelä 2019-08-14 14:16:09 -07:00 committed by Miško Hevery
parent f6e88cd659
commit 7cc4225eb9
5 changed files with 103 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
integration/_payload-limits.json
View File

@ -3,7 +3,7 @@
"master": { "master": {
"uncompressed": { "uncompressed": {
"runtime": 1497, "runtime": 1497,
"main": 157490, "main": 157021,
"polyfills": 45399 "polyfills": 45399
} }
} }
@ -39,7 +39,7 @@
"master": { "master": {
"uncompressed": { "uncompressed": {
"bundle": "TODO(i): temporarily increase the payload size limit from 105779 - this is due to a closure issue related to ESM reexports that still needs to be investigated", "bundle": "TODO(i): temporarily increase the payload size limit from 105779 - this is due to a closure issue related to ESM reexports that still needs to be investigated",
"bundle": 177447 "bundle": 177137
} }
} }
} }

59
packages/core/src/render3/styling_next/bindings.ts
View File

@ -8,6 +8,7 @@
import {SafeValue} from '../../sanitization/bypass'; import {SafeValue} from '../../sanitization/bypass';
import {StyleSanitizeFn, StyleSanitizeMode} from '../../sanitization/style_sanitizer'; import {StyleSanitizeFn, StyleSanitizeMode} from '../../sanitization/style_sanitizer';
import {ProceduralRenderer3, RElement, Renderer3, RendererStyleFlags3, isProceduralRenderer} from '../interfaces/renderer'; import {ProceduralRenderer3, RElement, Renderer3, RendererStyleFlags3, isProceduralRenderer} from '../interfaces/renderer';
import {NO_CHANGE} from '../tokens';
import {ApplyStylingFn, LStylingData, StylingMapArray, StylingMapArrayIndex, StylingMapsSyncMode, SyncStylingMapsFn, TStylingContext, TStylingContextIndex, TStylingContextPropConfigFlags} from './interfaces'; import {ApplyStylingFn, LStylingData, StylingMapArray, StylingMapArrayIndex, StylingMapsSyncMode, SyncStylingMapsFn, TStylingContext, TStylingContextIndex, TStylingContextPropConfigFlags} from './interfaces';
import {BIT_MASK_START_VALUE, deleteStylingStateFromStorage, getStylingState, resetStylingState, storeStylingState} from './state'; import {BIT_MASK_START_VALUE, deleteStylingStateFromStorage, getStylingState, resetStylingState, storeStylingState} from './state';
@ -80,21 +81,23 @@ let deferredBindingQueue: (TStylingContext | number | string | null | boolean)[]
*/ */
export function updateClassBinding( export function updateClassBinding(
context: TStylingContext, data: LStylingData, element: RElement, prop: string | null, context: TStylingContext, data: LStylingData, element: RElement, prop: string | null,
bindingIndex: number, value: boolean | string | null | undefined | StylingMapArray, bindingIndex: number, value: boolean | string | null | undefined | StylingMapArray | NO_CHANGE,
deferRegistration: boolean, forceUpdate: boolean): boolean { deferRegistration: boolean, forceUpdate: boolean): boolean {
const isMapBased = !prop; const isMapBased = !prop;
const state = getStylingState(element, stateIsPersisted(context)); const state = getStylingState(element, stateIsPersisted(context));
const index = isMapBased ? STYLING_INDEX_FOR_MAP_BINDING : state.classesIndex++; const index = isMapBased ? STYLING_INDEX_FOR_MAP_BINDING : state.classesIndex++;
const updated = updateBindingData( if (value !== NO_CHANGE) {
context, data, index, prop, bindingIndex, value, deferRegistration, forceUpdate, false); const updated = updateBindingData(
if (updated || forceUpdate) { context, data, index, prop, bindingIndex, value, deferRegistration, forceUpdate, false);
// We flip the bit in the bitMask to reflect that the binding if (updated || forceUpdate) {
// at the `index` slot has changed. This identifies to the flushing // We flip the bit in the bitMask to reflect that the binding
// phase that the bindings for this particular CSS class need to be // at the `index` slot has changed. This identifies to the flushing
// applied again because on or more of the bindings for the CSS // phase that the bindings for this particular CSS class need to be
// class have changed. // applied again because on or more of the bindings for the CSS
state.classesBitMask |= 1 << index; // class have changed.
return true; state.classesBitMask |= 1 << index;
return true;
}
} }
return false; return false;
} }
@ -111,25 +114,27 @@ export function updateClassBinding(
*/ */
export function updateStyleBinding( export function updateStyleBinding(
context: TStylingContext, data: LStylingData, element: RElement, prop: string | null, context: TStylingContext, data: LStylingData, element: RElement, prop: string | null,
bindingIndex: number, value: string | number | SafeValue | null | undefined | StylingMapArray, bindingIndex: number,
value: string | number | SafeValue | null | undefined | StylingMapArray | NO_CHANGE,
sanitizer: StyleSanitizeFn | null, deferRegistration: boolean, forceUpdate: boolean): boolean { sanitizer: StyleSanitizeFn | null, deferRegistration: boolean, forceUpdate: boolean): boolean {
const isMapBased = !prop; const isMapBased = !prop;
const state = getStylingState(element, stateIsPersisted(context)); const state = getStylingState(element, stateIsPersisted(context));
const index = isMapBased ? STYLING_INDEX_FOR_MAP_BINDING : state.stylesIndex++; const index = isMapBased ? STYLING_INDEX_FOR_MAP_BINDING : state.stylesIndex++;
const sanitizationRequired = isMapBased ? if (value !== NO_CHANGE) {
true : const sanitizationRequired = isMapBased ||
(sanitizer ? sanitizer(prop !, null, StyleSanitizeMode.ValidateProperty) : false); (sanitizer ? sanitizer(prop !, null, StyleSanitizeMode.ValidateProperty) : false);
const updated = updateBindingData( const updated = updateBindingData(
context, data, index, prop, bindingIndex, value, deferRegistration, forceUpdate, context, data, index, prop, bindingIndex, value, deferRegistration, forceUpdate,
sanitizationRequired); sanitizationRequired);
if (updated || forceUpdate) { if (updated || forceUpdate) {
// We flip the bit in the bitMask to reflect that the binding // We flip the bit in the bitMask to reflect that the binding
// at the `index` slot has changed. This identifies to the flushing // at the `index` slot has changed. This identifies to the flushing
// phase that the bindings for this particular property need to be // phase that the bindings for this particular property need to be
// applied again because on or more of the bindings for the CSS // applied again because on or more of the bindings for the CSS
// property have changed. // property have changed.
state.stylesBitMask |= 1 << index; state.stylesBitMask |= 1 << index;
return true; return true;
}
} }
return false; return false;
} }
@ -150,7 +155,7 @@ export function updateStyleBinding(
function updateBindingData( function updateBindingData(
context: TStylingContext, data: LStylingData, counterIndex: number, prop: string | null, context: TStylingContext, data: LStylingData, counterIndex: number, prop: string | null,
bindingIndex: number, bindingIndex: number,
value: string | SafeValue | number | boolean | null | undefined | StylingMapArray, value: string | SafeValue | number | boolean | null | undefined | StylingMapArray | NO_CHANGE,
deferRegistration: boolean, forceUpdate: boolean, sanitizationRequired: boolean): boolean { deferRegistration: boolean, forceUpdate: boolean, sanitizationRequired: boolean): boolean {
if (!isContextLocked(context)) { if (!isContextLocked(context)) {
if (deferRegistration) { if (deferRegistration) {

54
packages/core/src/render3/styling_next/instructions.ts
View File

@ -168,21 +168,19 @@ function _stylingProp(
const tNode = getTNode(elementIndex, lView); const tNode = getTNode(elementIndex, lView);
const native = getNativeByTNode(tNode, lView) as RElement; const native = getNativeByTNode(tNode, lView) as RElement;
let updated = false; let valueHasChanged = false;
if (value !== NO_CHANGE) { if (isClassBased) {
if (isClassBased) { valueHasChanged = updateClassBinding(
updated = updateClassBinding( getClassesContext(tNode), lView, native, prop, bindingIndex,
getClassesContext(tNode), lView, native, prop, bindingIndex, value as string | boolean | null, defer, false);
value as string | boolean | null, defer, false); } else {
} else { const sanitizer = getCurrentStyleSanitizer();
const sanitizer = getCurrentStyleSanitizer(); valueHasChanged = updateStyleBinding(
updated = updateStyleBinding( getStylesContext(tNode), lView, native, prop, bindingIndex,
getStylesContext(tNode), lView, native, prop, bindingIndex, value as string | SafeValue | null, sanitizer, defer, false);
value as string | SafeValue | null, sanitizer, defer, false);
}
} }
return updated; return valueHasChanged;
} }
/** /**
@ -299,22 +297,20 @@ function _stylingMap(
activateStylingMapFeature(); activateStylingMapFeature();
const lView = getLView(); const lView = getLView();
let valueHasChanged = false; const tNode = getTNode(elementIndex, lView);
if (value !== NO_CHANGE) { const native = getNativeByTNode(tNode, lView) as RElement;
const tNode = getTNode(elementIndex, lView); const oldValue = lView[bindingIndex];
const native = getNativeByTNode(tNode, lView) as RElement; const valueHasChanged = hasValueChanged(oldValue, value);
const oldValue = lView[bindingIndex]; const stylingMapArr =
valueHasChanged = hasValueChanged(oldValue, value); value === NO_CHANGE ? NO_CHANGE : normalizeIntoStylingMap(oldValue, value, !isClassBased);
const stylingMapArr = normalizeIntoStylingMap(oldValue, value, !isClassBased); if (isClassBased) {
if (isClassBased) { updateClassBinding(
updateClassBinding( context, lView, native, null, bindingIndex, stylingMapArr, defer, valueHasChanged);
context, lView, native, null, bindingIndex, stylingMapArr, defer, valueHasChanged); } else {
} else { const sanitizer = getCurrentStyleSanitizer();
const sanitizer = getCurrentStyleSanitizer(); updateStyleBinding(
updateStyleBinding( context, lView, native, null, bindingIndex, stylingMapArr, sanitizer, defer,
context, lView, native, null, bindingIndex, stylingMapArr, sanitizer, defer, valueHasChanged);
valueHasChanged);
}
} }
return valueHasChanged; return valueHasChanged;

9
packages/core/src/render3/styling_next/util.ts
View File

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license * found in the LICENSE file at https://angular.io/license
*/ */
import {TNode, TNodeFlags} from '../interfaces/node'; import {TNode, TNodeFlags} from '../interfaces/node';
import {NO_CHANGE} from '../tokens';
import {StylingMapArray, StylingMapArrayIndex, TStylingConfigFlags, TStylingContext, TStylingContextIndex, TStylingContextPropConfigFlags} from './interfaces'; import {StylingMapArray, StylingMapArrayIndex, TStylingConfigFlags, TStylingContext, TStylingContextIndex, TStylingContextPropConfigFlags} from './interfaces';
const MAP_BASED_ENTRY_PROP_NAME = '--MAP--'; const MAP_BASED_ENTRY_PROP_NAME = '--MAP--';
@ -146,8 +146,11 @@ export function isMapBased(prop: string) {
} }
export function hasValueChanged( export function hasValueChanged(
a: StylingMapArray | number | String | string | null | boolean | undefined | {}, a: NO_CHANGE | StylingMapArray | number | String | string | null | boolean | undefined | {},
b: StylingMapArray | number | String | string | null | boolean | undefined | {}): boolean { b: NO_CHANGE | StylingMapArray | number | String | string | null | boolean | undefined |
{}): boolean {
if (b === NO_CHANGE) return false;
let compareValueA = Array.isArray(a) ? a[StylingMapArrayIndex.RawValuePosition] : a; let compareValueA = Array.isArray(a) ? a[StylingMapArrayIndex.RawValuePosition] : a;
let compareValueB = Array.isArray(b) ? b[StylingMapArrayIndex.RawValuePosition] : b; let compareValueB = Array.isArray(b) ? b[StylingMapArrayIndex.RawValuePosition] : b;

40
packages/core/test/acceptance/styling_next_spec.ts
View File

@ -6,9 +6,7 @@
* found in the LICENSE file at https://angular.io/license * found in the LICENSE file at https://angular.io/license
*/ */
import {Component, Directive, HostBinding, Input, ViewChild} from '@angular/core'; import {Component, Directive, HostBinding, Input, ViewChild} from '@angular/core';
import {SecurityContext} from '@angular/core/src/core';
import {DebugNode, LViewDebug, toDebug} from '@angular/core/src/render3/instructions/lview_debug'; import {DebugNode, LViewDebug, toDebug} from '@angular/core/src/render3/instructions/lview_debug';
import {getCheckNoChangesMode} from '@angular/core/src/render3/state';
import {loadLContextFromNode} from '@angular/core/src/render3/util/discovery_utils'; import {loadLContextFromNode} from '@angular/core/src/render3/util/discovery_utils';
import {ngDevModeResetPerfCounters as resetStylingCounters} from '@angular/core/src/util/ng_dev_mode'; import {ngDevModeResetPerfCounters as resetStylingCounters} from '@angular/core/src/util/ng_dev_mode';
import {TestBed} from '@angular/core/testing'; import {TestBed} from '@angular/core/testing';
@ -1056,6 +1054,44 @@ describe('new styling integration', () => {
expect(div.style.width).toEqual('100px'); expect(div.style.width).toEqual('100px');
expect(div.style.height).toEqual('300px'); expect(div.style.height).toEqual('300px');
}); });
it('should work with NO_CHANGE values if they are applied to bindings ', () => {
@Component({
template: `
<div
[style.width]="w"
style.height="{{ h }}"
[style.opacity]="o"></div>
`
})
class Cmp {
w: any = null;
h: any = null;
o: any = null;
}
TestBed.configureTestingModule({declarations: [Cmp]});
const fixture = TestBed.createComponent(Cmp);
const comp = fixture.componentInstance;
comp.w = '100px';
comp.h = '200px';
comp.o = '0.5';
fixture.detectChanges();
const div = fixture.nativeElement.querySelector('div');
expect(div.style.width).toEqual('100px');
expect(div.style.height).toEqual('200px');
expect(div.style.opacity).toEqual('0.5');
comp.w = '500px';
comp.o = '1';
fixture.detectChanges();
expect(div.style.width).toEqual('500px');
expect(div.style.height).toEqual('200px');
expect(div.style.opacity).toEqual('1');
});
}); });
function assertStyleCounters(countForSet: number, countForRemove: number) { function assertStyleCounters(countForSet: number, countForRemove: number) {