Skip to content

Commit

Permalink
refactor(ivy): remove TStylingContext locking in favor of firstUpdate…
Browse files Browse the repository at this point in the history 
…Pass flag

This patch removes the need to lock the style and class context
instances to track when bindings can be added. What happens now is
that the `tNode.firstUpdatePass` is used to track when bindings are
registered on the context instances.
  • Loading branch information
@matsko
matsko committed Nov 4, 2019
1 parent 9c13d6e commit a2527ec
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 182 deletions.
56 changes: 31 additions & 25 deletions packages/core/src/render3/instructions/styling.ts
View file
Expand Up @@ -13,14 +13,14 @@ import {AttributeMarker, TAttributes, TNode, TNodeFlags, TNodeType} from '../int
import {RElement} from '../interfaces/renderer';
import {StylingMapArray, StylingMapArrayIndex, TStylingConfig, TStylingContext} from '../interfaces/styling';
import {isDirectiveHost} from '../interfaces/type_checks';
import {LView, RENDERER} from '../interfaces/view';
import {LView, RENDERER, TVIEW, TView} from '../interfaces/view';
import {getActiveDirectiveId, getCheckNoChangesMode, getCurrentStyleSanitizer, getLView, getSelectedIndex, incrementBindingIndex, nextBindingIndex, resetCurrentStyleSanitizer, setCurrentStyleSanitizer, setElementExitFn} from '../state';
import {applyStylingMapDirectly, applyStylingValueDirectly, flushStyling, setClass, setStyle, updateClassViaContext, updateStyleViaContext} from '../styling/bindings';
import {activateStylingMapFeature} from '../styling/map_based_bindings';
import {attachStylingDebugObject} from '../styling/styling_debug';
import {NO_CHANGE} from '../tokens';
import {renderStringify} from '../util/misc_utils';
import {addItemToStylingMap, allocStylingMapArray, allocTStylingContext, allowDirectStyling, concatString, forceClassesAsString, forceStylesAsString, getInitialStylingValue, getStylingMapArray, getValue, hasClassInput, hasStyleInput, hasValueChanged, isContextLocked, isHostStylingActive, isStylingContext, normalizeIntoStylingMap, patchConfig, selectClassBasedInputName, setValue, stylingMapToString} from '../util/styling_utils';
import {addItemToStylingMap, allocStylingMapArray, allocTStylingContext, allowDirectStyling, concatString, forceClassesAsString, forceStylesAsString, getInitialStylingValue, getStylingMapArray, getValue, hasClassInput, hasStyleInput, hasValueChanged, isHostStylingActive, isStylingContext, isStylingValueDefined, normalizeIntoStylingMap, patchConfig, selectClassBasedInputName, setValue, stylingMapToString} from '../util/styling_utils';
import {getNativeByTNode, getTNode} from '../util/view_utils';


Expand Down Expand Up @@ -155,16 +155,16 @@ function stylingProp(

const lView = getLView();
const tNode = getTNode(elementIndex, lView);
const tView = lView[TVIEW];
const native = getNativeByTNode(tNode, lView) as RElement;

const hostBindingsMode = isHostStyling();
const firstUpdatePass = tView.firstUpdatePass;
const context = isClassBased ? getClassesContext(tNode) : getStylesContext(tNode);
const sanitizer = isClassBased ? null : getCurrentStyleSanitizer();

// we check for this in the instruction code so that the context can be notified
// about prop or map bindings so that the direct apply check can decide earlier
// if it allows for context resolution to be bypassed.
if (!isContextLocked(context, hostBindingsMode)) {
if (firstUpdatePass) {
patchConfig(context, TStylingConfig.HasPropBindings);
}

Expand All @@ -181,7 +181,7 @@ function stylingProp(

// Direct Apply Case: bypass context resolution and apply the
// style/class value directly to the element
if (allowDirectStyling(context, hostBindingsMode)) {
if (allowDirectStyling(context, firstUpdatePass)) {
const sanitizerToUse = isClassBased ? null : sanitizer;
const renderer = getRenderer(tNode, lView);
updated = applyStylingValueDirectly(
Expand All @@ -201,11 +201,11 @@ function stylingProp(
if (isClassBased) {
updated = updateClassViaContext(
context, lView, native, directiveIndex, prop, bindingIndex,
value as string | boolean | null);
value as string | boolean | null, false, firstUpdatePass);
} else {
updated = updateStyleViaContext(
context, lView, native, directiveIndex, prop, bindingIndex,
value as string | SafeValue | null, sanitizer);
value as string | SafeValue | null, sanitizer, false, firstUpdatePass);
}

setElementExitFn(stylingApply);
Expand Down Expand Up @@ -237,6 +237,7 @@ export function ɵɵstyleMap(styles: {[styleName: string]: any} | NO_CHANGE | nu
const index = getSelectedIndex();
const lView = getLView();
const tNode = getTNode(index, lView);
const tView = lView[TVIEW];
const context = getStylesContext(tNode);
const hasDirectiveInput = hasStyleInput(tNode);

Expand All @@ -250,11 +251,12 @@ export function ɵɵstyleMap(styles: {[styleName: string]: any} | NO_CHANGE | nu
// there should not be a situation where a directive host bindings function
// evaluates the inputs (this should only happen in the template function)
if (!isHostStyling() && hasDirectiveInput && styles !== NO_CHANGE) {
updateDirectiveInputValue(context, lView, tNode, bindingIndex, styles, false);
updateDirectiveInputValue(
context, lView, tNode, bindingIndex, styles, false, tView.firstUpdatePass);
styles = NO_CHANGE;
}

stylingMap(context, tNode, lView, bindingIndex, styles, false, hasDirectiveInput);
stylingMap(context, tNode, tView, lView, bindingIndex, styles, false, hasDirectiveInput);
}

/**
Expand Down Expand Up @@ -289,6 +291,7 @@ export function classMapInternal(
elementIndex: number, classes: {[className: string]: any} | NO_CHANGE | string | null): void {
const lView = getLView();
const tNode = getTNode(elementIndex, lView);
const tView = lView[TVIEW];
const context = getClassesContext(tNode);
const hasDirectiveInput = hasClassInput(tNode);

Expand All @@ -302,11 +305,12 @@ export function classMapInternal(
// there should not be a situation where a directive host bindings function
// evaluates the inputs (this should only happen in the template function)
if (!isHostStyling() && hasDirectiveInput && classes !== NO_CHANGE) {
updateDirectiveInputValue(context, lView, tNode, bindingIndex, classes, true);
updateDirectiveInputValue(
context, lView, tNode, bindingIndex, classes, true, tView.firstUpdatePass);
classes = NO_CHANGE;
}

stylingMap(context, tNode, lView, bindingIndex, classes, true, hasDirectiveInput);
stylingMap(context, tNode, tView, lView, bindingIndex, classes, true, hasDirectiveInput);
}

/**
Expand All @@ -316,15 +320,15 @@ export function classMapInternal(
* `[class]` bindings in Angular.
*/
function stylingMap(
context: TStylingContext, tNode: TNode, lView: LView, bindingIndex: number,
context: TStylingContext, tNode: TNode, tView: TView, lView: LView, bindingIndex: number,
value: {[key: string]: any} | string | null, isClassBased: boolean,
hasDirectiveInput: boolean): void {
const directiveIndex = getActiveDirectiveId();
const native = getNativeByTNode(tNode, lView) as RElement;
const oldValue = getValue(lView, bindingIndex);
const hostBindingsMode = isHostStyling();
const sanitizer = getCurrentStyleSanitizer();
const valueHasChanged = hasValueChanged(oldValue, value);
const firstUpdatePass = tView.firstUpdatePass;

// [style] and [class] bindings do not use `bind()` and will therefore
// manage accessing and updating the new value in the lView directly.
Expand All @@ -337,13 +341,13 @@ function stylingMap(
// we check for this in the instruction code so that the context can be notified
// about prop or map bindings so that the direct apply check can decide earlier
// if it allows for context resolution to be bypassed.
if (!isContextLocked(context, hostBindingsMode)) {
if (firstUpdatePass) {
patchConfig(context, TStylingConfig.HasMapBindings);
}

// Direct Apply Case: bypass context resolution and apply the
// style/class map values directly to the element
if (allowDirectStyling(context, hostBindingsMode)) {
if (allowDirectStyling(context, firstUpdatePass)) {
const sanitizerToUse = isClassBased ? null : sanitizer;
const renderer = getRenderer(tNode, lView);
applyStylingMapDirectly(
Expand All @@ -367,11 +371,11 @@ function stylingMap(
if (isClassBased) {
updateClassViaContext(
context, lView, native, directiveIndex, null, bindingIndex, stylingMapArr,
valueHasChanged);
valueHasChanged, firstUpdatePass);
} else {
updateStyleViaContext(
context, lView, native, directiveIndex, null, bindingIndex, stylingMapArr, sanitizer,
valueHasChanged);
valueHasChanged, firstUpdatePass);
}

setElementExitFn(stylingApply);
Expand All @@ -396,18 +400,17 @@ function stylingMap(
* depending on the following situations:
*
* - If `oldValue !== newValue`
* - If `newValue` is `null` (but this is skipped if it is during the first update pass--
* which is when the context is not locked yet)
* - If `newValue` is `null` (but this is skipped if it is during the first update pass)
*/
function updateDirectiveInputValue(
context: TStylingContext, lView: LView, tNode: TNode, bindingIndex: number, newValue: any,
isClassBased: boolean): void {
const oldValue = lView[bindingIndex];
if (oldValue !== newValue) {
isClassBased: boolean, firstUpdatePass: boolean): void {
const oldValue = getValue(lView, bindingIndex);
if (hasValueChanged(oldValue, newValue)) {
// even if the value has changed we may not want to emit it to the
// directive input(s) in the event that it is falsy during the
// first update pass.
if (newValue || isContextLocked(context, false)) {
if (isStylingValueDefined(newValue) || !firstUpdatePass) {
const inputName: string = isClassBased ? selectClassBasedInputName(tNode.inputs !) : 'style';
const inputs = tNode.inputs ![inputName] !;
const initialValue = getInitialStylingValue(context);
Expand Down Expand Up @@ -452,6 +455,7 @@ function normalizeStylingDirectiveInputValue(
*/
function stylingApply(): void {
const lView = getLView();
const tView = lView[TVIEW];
const elementIndex = getSelectedIndex();
const tNode = getTNode(elementIndex, lView);
const native = getNativeByTNode(tNode, lView) as RElement;
Expand All @@ -460,7 +464,9 @@ function stylingApply(): void {
const sanitizer = getCurrentStyleSanitizer();
const classesContext = isStylingContext(tNode.classes) ? tNode.classes as TStylingContext : null;
const stylesContext = isStylingContext(tNode.styles) ? tNode.styles as TStylingContext : null;
flushStyling(renderer, lView, classesContext, stylesContext, native, directiveIndex, sanitizer);
flushStyling(
renderer, lView, classesContext, stylesContext, native, directiveIndex, sanitizer,
tView.firstUpdatePass);
resetCurrentStyleSanitizer();
}

Expand Down
42 changes: 10 additions & 32 deletions packages/core/src/render3/interfaces/styling.ts
View file
Expand Up @@ -341,7 +341,7 @@ export const enum TStylingConfig {
/**
* The initial state of the styling context config.
*/
Initial = 0b00000000,
Initial = 0b000000,

/**
* Whether or not there are any directives on this element.
Expand All @@ -359,7 +359,7 @@ export const enum TStylingConfig {
* 3. `<comp>`
* 4. `<comp dir-one>`
*/
HasDirectives = 0b00000001,
HasDirectives = 0b000001,

/**
* Whether or not there are prop-based bindings present.
Expand All @@ -370,7 +370,7 @@ export const enum TStylingConfig {
* 3. `@HostBinding('style.prop') x`
* 4. `@HostBinding('class.prop') x`
*/
HasPropBindings = 0b00000010,
HasPropBindings = 0b000010,

/**
* Whether or not there are map-based bindings present.
Expand All @@ -381,7 +381,7 @@ export const enum TStylingConfig {
* 3. `@HostBinding('style') x`
* 4. `@HostBinding('class') x`
*/
HasMapBindings = 0b00000100,
HasMapBindings = 0b000100,

/**
* Whether or not there are map-based and prop-based bindings present.
Expand All @@ -402,7 +402,7 @@ export const enum TStylingConfig {
* 2. map + prop: `<div [style]="x" [style.prop]>`
* 3. map + map: `<div [style]="x" dir-that-sets-style>`
*/
HasCollisions = 0b00001000,
HasCollisions = 0b001000,

/**
* Whether or not the context contains initial styling values.
Expand All @@ -413,7 +413,7 @@ export const enum TStylingConfig {
* 3. `@Directive({ host: { 'style': 'width:200px' } })`
* 4. `@Directive({ host: { 'class': 'one two three' } })`
*/
HasInitialStyling = 0b000010000,
HasInitialStyling = 0b0010000,

/**
* Whether or not the context contains one or more template bindings.
Expand All @@ -424,7 +424,7 @@ export const enum TStylingConfig {
* 3. `<div [class]="x">`
* 4. `<div [class.name]="x">`
*/
HasTemplateBindings = 0b000100000,
HasTemplateBindings = 0b0100000,

/**
* Whether or not the context contains one or more host bindings.
Expand All @@ -435,35 +435,13 @@ export const enum TStylingConfig {
* 3. `@HostBinding('class') x`
* 4. `@HostBinding('class.name') x`
*/
HasHostBindings = 0b001000000,

/**
* Whether or not the template bindings are allowed to be registered in the context.
*
* This flag is after one or more template-based style/class bindings were
* set and processed for an element. Once the bindings are processed then a call
* to stylingApply is issued and the lock will be put into place.
*
* Note that this is only set once.
*/
TemplateBindingsLocked = 0b010000000,

/**
* Whether or not the host bindings are allowed to be registered in the context.
*
* This flag is after one or more host-based style/class bindings were
* set and processed for an element. Once the bindings are processed then a call
* to stylingApply is issued and the lock will be put into place.
*
* Note that this is only set once.
*/
HostBindingsLocked = 0b100000000,
HasHostBindings = 0b1000000,

/** A Mask of all the configurations */
Mask = 0b111111111,
Mask = 0b1111111,

/** Total amount of configuration bits used */
TotalBits = 9,
TotalBits = 7,
}

/**
Expand Down

0 comments on commit a2527ec

Please sign in to comment.