fix(ivy): ensure [style]/[class] bindings identity check the previous value (#26149)

PR Close #26149
This commit is contained in:
Matias Niemelä 2018-09-28 12:27:30 -07:00 committed by Alex Rickabaugh
parent c51331689f
commit 391c708d7e
2 changed files with 401 additions and 252 deletions

View File

@ -54,6 +54,7 @@ import {Renderer3, RendererStyleFlags3, isProceduralRenderer} from './interfaces
* ``` * ```
* context = [ * context = [
* element, * element,
* playerContext | null,
* styleSanitizer | null, * styleSanitizer | null,
* [null, '100px', '200px', true], // property names are not needed since they have already been * [null, '100px', '200px', true], // property names are not needed since they have already been
* written to DOM. * written to DOM.
@ -61,34 +62,35 @@ import {Renderer3, RendererStyleFlags3, isProceduralRenderer} from './interfaces
* configMasterVal, * configMasterVal,
* 1, // this instructs how many `style` values there are so that class index values can be * 1, // this instructs how many `style` values there are so that class index values can be
* offsetted * offsetted
* 'last class string applied', * { classOne: true, classTwo: false } | 'classOne classTwo' | null // last class value provided into updateStylingMap
* { styleOne: '100px', styleTwo: 0 } | null // last style value provided into updateStylingMap
* *
* // 6 * // 8
* 'width', * 'width',
* pointers(1, 15); // Point to static `width`: `100px` and multi `width`. * pointers(1, 15); // Point to static `width`: `100px` and multi `width`.
* null, * null,
* *
* // 9 * // 11
* 'height', * 'height',
* pointers(2, 18); // Point to static `height`: `200px` and multi `height`. * pointers(2, 18); // Point to static `height`: `200px` and multi `height`.
* null, * null,
* *
* // 12 * // 14
* 'foo', * 'foo',
* pointers(1, 21); // Point to static `foo`: `true` and multi `foo`. * pointers(1, 21); // Point to static `foo`: `true` and multi `foo`.
* null, * null,
* *
* // 15 * // 17
* 'width', * 'width',
* pointers(1, 6); // Point to static `width`: `100px` and single `width`. * pointers(1, 6); // Point to static `width`: `100px` and single `width`.
* null, * null,
* *
* // 18 * // 21
* 'height', * 'height',
* pointers(2, 9); // Point to static `height`: `200px` and single `height`. * pointers(2, 9); // Point to static `height`: `200px` and single `height`.
* null, * null,
* *
* // 21 * // 24
* 'foo', * 'foo',
* pointers(3, 12); // Point to static `foo`: `true` and single `foo`. * pointers(3, 12); // Point to static `foo`: `true` and single `foo`.
* null, * null,
@ -115,7 +117,8 @@ import {Renderer3, RendererStyleFlags3, isProceduralRenderer} from './interfaces
* `updateStylingMap` can include new CSS properties that will be added to the context). * `updateStylingMap` can include new CSS properties that will be added to the context).
*/ */
export interface StylingContext extends export interface StylingContext extends
Array<InitialStyles|number|string|boolean|LElementNode|StyleSanitizeFn|AnimationContext|null> { Array<InitialStyles|{[key: string]: any}|number|string|boolean|LElementNode|StyleSanitizeFn|
AnimationContext|null> {
/** /**
* Location of element that is used as a target for this context. * Location of element that is used as a target for this context.
*/ */
@ -152,10 +155,16 @@ export interface StylingContext extends
[StylingIndex.ClassOffsetPosition]: number; [StylingIndex.ClassOffsetPosition]: number;
/** /**
* The last CLASS STRING VALUE that was interpreted by elementStylingMap. This is cached * The last class value that was interpreted by elementStylingMap. This is cached
* So that the algorithm can exit early incase the string has not changed. * So that the algorithm can exit early incase the value has not changed.
*/ */
[StylingIndex.CachedCssClassString]: string|null; [StylingIndex.PreviousMultiClassValue]: {[key: string]: any}|string|null;
/**
* The last style value that was interpreted by elementStylingMap. This is cached
* So that the algorithm can exit early incase the value has not changed.
*/
[StylingIndex.PreviousMultiStyleValue]: {[key: string]: any}|null;
} }
/** /**
@ -202,9 +211,11 @@ export const enum StylingIndex {
// Index of location where the class index offset value is located // Index of location where the class index offset value is located
ClassOffsetPosition = 5, ClassOffsetPosition = 5,
// Position of where the last string-based CSS class value was stored // Position of where the last string-based CSS class value was stored
CachedCssClassString = 6, PreviousMultiClassValue = 6,
// Position of where the last string-based CSS class value was stored
PreviousMultiStyleValue = 7,
// Location of single (prop) value entries are stored within the context // Location of single (prop) value entries are stored within the context
SingleStylesStartPosition = 7, SingleStylesStartPosition = 8,
// Multi and single entries are stored in `StylingContext` as: Flag; PropertyName; PropertyValue // Multi and single entries are stored in `StylingContext` as: Flag; PropertyName; PropertyValue
FlagsOffset = 0, FlagsOffset = 0,
PropertyOffset = 1, PropertyOffset = 1,
@ -234,7 +245,9 @@ export function allocStylingContext(
export function createEmptyStylingContext( export function createEmptyStylingContext(
element?: LElementNode | null, sanitizer?: StyleSanitizeFn | null, element?: LElementNode | null, sanitizer?: StyleSanitizeFn | null,
initialStylingValues?: InitialStyles): StylingContext { initialStylingValues?: InitialStyles): StylingContext {
return [element || null, null, sanitizer || null, initialStylingValues || [null], 0, 0, null]; return [
element || null, null, sanitizer || null, initialStylingValues || [null], 0, 0, null, null
];
} }
/** /**
@ -382,30 +395,35 @@ const EMPTY_OBJ: {[key: string]: any} = {};
export function updateStylingMap( export function updateStylingMap(
context: StylingContext, classes: {[key: string]: any} | string | null, context: StylingContext, classes: {[key: string]: any} | string | null,
styles?: {[key: string]: any} | null): void { styles?: {[key: string]: any} | null): void {
styles = styles || null;
// early exit (this is what's done to avoid using ctx.bind() to cache the value)
const ignoreAllClassUpdates = classes === context[StylingIndex.PreviousMultiClassValue];
const ignoreAllStyleUpdates = styles === context[StylingIndex.PreviousMultiStyleValue];
if (ignoreAllClassUpdates && ignoreAllStyleUpdates) return;
let classNames: string[] = EMPTY_ARR; let classNames: string[] = EMPTY_ARR;
let applyAllClasses = false; let applyAllClasses = false;
let ignoreAllClassUpdates = false;
// each time a string-based value pops up then it shouldn't require a deep // each time a string-based value pops up then it shouldn't require a deep
// check of what's changed. // check of what's changed.
if (typeof classes == 'string') { if (!ignoreAllClassUpdates) {
const cachedClassString = context[StylingIndex.CachedCssClassString] as string | null; context[StylingIndex.PreviousMultiClassValue] = classes;
if (cachedClassString && cachedClassString === classes) { if (typeof classes == 'string') {
ignoreAllClassUpdates = true;
} else {
context[StylingIndex.CachedCssClassString] = classes;
classNames = classes.split(/\s+/); classNames = classes.split(/\s+/);
// this boolean is used to avoid having to create a key/value map of `true` values // this boolean is used to avoid having to create a key/value map of `true` values
// since a classname string implies that all those classes are added // since a classname string implies that all those classes are added
applyAllClasses = true; applyAllClasses = true;
} else {
classNames = classes ? Object.keys(classes) : EMPTY_ARR;
} }
} else {
classNames = classes ? Object.keys(classes) : EMPTY_ARR;
context[StylingIndex.CachedCssClassString] = null;
} }
classes = (classes || EMPTY_OBJ) as{[key: string]: any}; classes = (classes || EMPTY_OBJ) as{[key: string]: any};
if (!ignoreAllStyleUpdates) {
context[StylingIndex.PreviousMultiStyleValue] = styles;
}
const styleProps = styles ? Object.keys(styles) : EMPTY_ARR; const styleProps = styles ? Object.keys(styles) : EMPTY_ARR;
styles = styles || EMPTY_OBJ; styles = styles || EMPTY_OBJ;
@ -423,10 +441,12 @@ export function updateStylingMap(
// are off-balance then they will be dealt in another loop after this one // are off-balance then they will be dealt in another loop after this one
while (ctxIndex < context.length && propIndex < propLimit) { while (ctxIndex < context.length && propIndex < propLimit) {
const isClassBased = propIndex >= classesStartIndex; const isClassBased = propIndex >= classesStartIndex;
const processValue =
(!isClassBased && !ignoreAllStyleUpdates) || (isClassBased && !ignoreAllClassUpdates);
// when there is a cache-hit for a string-based class then we should // when there is a cache-hit for a string-based class then we should
// avoid doing any work diffing any of the changes // avoid doing any work diffing any of the changes
if (!ignoreAllClassUpdates || !isClassBased) { if (processValue) {
const adjustedPropIndex = isClassBased ? propIndex - classesStartIndex : propIndex; const adjustedPropIndex = isClassBased ? propIndex - classesStartIndex : propIndex;
const newProp: string = const newProp: string =
isClassBased ? classNames[adjustedPropIndex] : styleProps[adjustedPropIndex]; isClassBased ? classNames[adjustedPropIndex] : styleProps[adjustedPropIndex];
@ -483,14 +503,16 @@ export function updateStylingMap(
while (ctxIndex < context.length) { while (ctxIndex < context.length) {
const flag = getPointers(context, ctxIndex); const flag = getPointers(context, ctxIndex);
const isClassBased = (flag & StylingFlags.Class) === StylingFlags.Class; const isClassBased = (flag & StylingFlags.Class) === StylingFlags.Class;
if (ignoreAllClassUpdates && isClassBased) break; const processValue =
(!isClassBased && !ignoreAllStyleUpdates) || (isClassBased && !ignoreAllClassUpdates);
const value = getValue(context, ctxIndex); if (processValue) {
const doRemoveValue = valueExists(value, isClassBased); const value = getValue(context, ctxIndex);
if (doRemoveValue) { const doRemoveValue = valueExists(value, isClassBased);
setDirty(context, ctxIndex, true); if (doRemoveValue) {
setValue(context, ctxIndex, null); setDirty(context, ctxIndex, true);
dirty = true; setValue(context, ctxIndex, null);
dirty = true;
}
} }
ctxIndex += StylingIndex.Size; ctxIndex += StylingIndex.Size;
} }
@ -501,16 +523,18 @@ export function updateStylingMap(
const sanitizer = getStyleSanitizer(context); const sanitizer = getStyleSanitizer(context);
while (propIndex < propLimit) { while (propIndex < propLimit) {
const isClassBased = propIndex >= classesStartIndex; const isClassBased = propIndex >= classesStartIndex;
if (ignoreAllClassUpdates && isClassBased) break; const processValue =
(!isClassBased && !ignoreAllStyleUpdates) || (isClassBased && !ignoreAllClassUpdates);
const adjustedPropIndex = isClassBased ? propIndex - classesStartIndex : propIndex; if (processValue) {
const prop = isClassBased ? classNames[adjustedPropIndex] : styleProps[adjustedPropIndex]; const adjustedPropIndex = isClassBased ? propIndex - classesStartIndex : propIndex;
const value: string|boolean = const prop = isClassBased ? classNames[adjustedPropIndex] : styleProps[adjustedPropIndex];
isClassBased ? (applyAllClasses ? true : classes[prop]) : styles[prop]; const value: string|boolean =
const flag = prepareInitialFlag(prop, isClassBased, sanitizer) | StylingFlags.Dirty; isClassBased ? (applyAllClasses ? true : classes[prop]) : styles[prop];
context.push(flag, prop, value); const flag = prepareInitialFlag(prop, isClassBased, sanitizer) | StylingFlags.Dirty;
context.push(flag, prop, value);
dirty = true;
}
propIndex++; propIndex++;
dirty = true;
} }
if (dirty) { if (dirty) {

File diff suppressed because it is too large Load Diff