Skip to content

Commit

Permalink
refactor(ivy): move all styling configurations into TNodeFlags (#33540
Browse files Browse the repository at this point in the history 
)

This patch gets rid of the configuration settings present in the
`TStylingContext` array that is used within the styling algorithm
for `[style]`, `[style.prop]`, `[class]` and `[class.name]` bindings.
These configurations now all live inside of the `TNodeFlags`.

PR Close #33540
  • Loading branch information
@matsko @atscott
matsko authored and atscott committed Nov 6, 2019
1 parent e89c2dd commit 41560b4
Show file tree
Hide file tree
Showing 11 changed files with 368 additions and 291 deletions.
2 changes: 1 addition & 1 deletion integration/_payload-limits.json
View file
Expand Up @@ -39,7 +39,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 2289,
"main-es2015": 265613,
"main-es2015": 268331,
"polyfills-es2015": 36808,
"5-es2015": 751
}
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/render3/instructions/lview_debug.ts
View file
Expand Up @@ -375,10 +375,10 @@ export function buildDebugNode(tNode: TNode, lView: LView, nodeIndex: number): D
const native = unwrapRNode(rawValue);
const componentLViewDebug = toDebug(readLViewValue(rawValue));
const styles = isStylingContext(tNode.styles) ?
new NodeStylingDebug(tNode.styles as any as TStylingContext, lView, false) :
new NodeStylingDebug(tNode.styles as any as TStylingContext, tNode, lView, false) :
null;
const classes = isStylingContext(tNode.classes) ?
new NodeStylingDebug(tNode.classes as any as TStylingContext, lView, true) :
new NodeStylingDebug(tNode.classes as any as TStylingContext, tNode, lView, true) :
null;
return {
html: toHtml(native),
Expand Down
103 changes: 68 additions & 35 deletions packages/core/src/render3/instructions/styling.ts
View file
Expand Up @@ -11,7 +11,7 @@ import {throwErrorIfNoChangesMode} from '../errors';
import {setInputsForProperty} from '../instructions/shared';
import {AttributeMarker, TAttributes, TNode, TNodeFlags, TNodeType} from '../interfaces/node';
import {RElement} from '../interfaces/renderer';
import {StylingMapArray, StylingMapArrayIndex, TStylingConfig, TStylingContext} from '../interfaces/styling';
import {StylingMapArray, StylingMapArrayIndex, TStylingContext} from '../interfaces/styling';
import {isDirectiveHost} from '../interfaces/type_checks';
import {LView, RENDERER, TVIEW, TView} from '../interfaces/view';
import {getActiveDirectiveId, getCheckNoChangesMode, getCurrentStyleSanitizer, getLView, getSelectedIndex, incrementBindingIndex, nextBindingIndex, resetCurrentStyleSanitizer, setCurrentStyleSanitizer, setElementExitFn} from '../state';
Expand Down Expand Up @@ -95,9 +95,21 @@ export function stylePropInternal(
// still needs to be incremented because all styling binding values
// are stored inside of the lView.
const bindingIndex = nextBindingIndex();
const lView = getLView();
const tNode = getTNode(elementIndex, lView);
const firstUpdatePass = lView[TVIEW].firstUpdatePass;

const updated =
stylingProp(elementIndex, bindingIndex, prop, resolveStylePropValue(value, suffix), false);
// 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 (firstUpdatePass) {
patchConfig(tNode, TNodeFlags.hasStylePropBindings);
patchHostStylingFlag(tNode, isHostStyling(), false);
}

const updated = stylingProp(
tNode, firstUpdatePass, lView, bindingIndex, prop, resolveStylePropValue(value, suffix),
false);
if (ngDevMode) {
ngDevMode.styleProp++;
if (updated) {
Expand Down Expand Up @@ -127,8 +139,20 @@ export function ɵɵclassProp(className: string, value: boolean | null): void {
// still needs to be incremented because all styling binding values
// are stored inside of the lView.
const bindingIndex = nextBindingIndex();
const lView = getLView();
const elementIndex = getSelectedIndex();
const tNode = getTNode(elementIndex, lView);
const firstUpdatePass = lView[TVIEW].firstUpdatePass;

const updated = stylingProp(getSelectedIndex(), bindingIndex, className, value, true);
// 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 (firstUpdatePass) {
patchConfig(tNode, TNodeFlags.hasClassPropBindings);
patchHostStylingFlag(tNode, isHostStyling(), true);
}

const updated = stylingProp(tNode, firstUpdatePass, lView, bindingIndex, className, value, true);
if (ngDevMode) {
ngDevMode.classProp++;
if (updated) {
Expand All @@ -148,25 +172,15 @@ export function ɵɵclassProp(className: string, value: boolean | null): void {
* present together).
*/
function stylingProp(
elementIndex: number, bindingIndex: number, prop: string,
tNode: TNode, firstUpdatePass: boolean, lView: LView, bindingIndex: number, prop: string,
value: boolean | number | SafeValue | string | null | undefined | NO_CHANGE,
isClassBased: boolean): boolean {
let updated = false;

const lView = getLView();
const firstUpdatePass = lView[TVIEW].firstUpdatePass;
const tNode = getTNode(elementIndex, lView);
const native = getNativeByTNode(tNode, lView) as RElement;
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 (firstUpdatePass) {
patchConfig(context, TStylingConfig.HasPropBindings);
}

// [style.prop] and [class.name] bindings do not use `bind()` and will
// therefore manage accessing and updating the new value in the lView directly.
// For this reason, the checkNoChanges situation must also be handled here
Expand All @@ -180,11 +194,12 @@ function stylingProp(

// Direct Apply Case: bypass context resolution and apply the
// style/class value directly to the element
if (allowDirectStyling(context, firstUpdatePass)) {
if (allowDirectStyling(tNode, isClassBased, firstUpdatePass)) {
const sanitizerToUse = isClassBased ? null : sanitizer;
const renderer = getRenderer(tNode, lView);
updated = applyStylingValueDirectly(
renderer, context, native, lView, bindingIndex, prop, value, isClassBased, sanitizerToUse);
renderer, context, tNode, native, lView, bindingIndex, prop, value, isClassBased,
sanitizerToUse);

if (sanitizerToUse) {
// it's important we remove the current style sanitizer once the
Expand All @@ -199,11 +214,11 @@ function stylingProp(
const directiveIndex = getActiveDirectiveId();
if (isClassBased) {
updated = updateClassViaContext(
context, lView, native, directiveIndex, prop, bindingIndex,
context, tNode, lView, native, directiveIndex, prop, bindingIndex,
value as string | boolean | null, false, firstUpdatePass);
} else {
updated = updateStyleViaContext(
context, lView, native, directiveIndex, prop, bindingIndex,
context, tNode, lView, native, directiveIndex, prop, bindingIndex,
value as string | SafeValue | null, sanitizer, false, firstUpdatePass);
}

Expand Down Expand Up @@ -245,15 +260,24 @@ export function ɵɵstyleMap(styles: {[styleName: string]: any} | NO_CHANGE | nu
// still needs to be incremented because all styling binding values
// are stored inside of the lView.
const bindingIndex = incrementBindingIndex(2);
const hostBindingsMode = isHostStyling();

// inputs are only evaluated from a template binding into a directive, therefore,
// 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) {
if (!hostBindingsMode && hasDirectiveInput && styles !== NO_CHANGE) {
updateDirectiveInputValue(context, lView, tNode, bindingIndex, styles, false, firstUpdatePass);
styles = NO_CHANGE;
}

// 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 (firstUpdatePass) {
patchConfig(tNode, TNodeFlags.hasStyleMapBindings);
patchHostStylingFlag(tNode, isHostStyling(), false);
}

stylingMap(
context, tNode, firstUpdatePass, lView, bindingIndex, styles, false, hasDirectiveInput);
}
Expand Down Expand Up @@ -299,15 +323,24 @@ export function classMapInternal(
// still needs to be incremented because all styling binding values
// are stored inside of the lView.
const bindingIndex = incrementBindingIndex(2);
const hostBindingsMode = isHostStyling();

// inputs are only evaluated from a template binding into a directive, therefore,
// 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) {
if (!hostBindingsMode && hasDirectiveInput && classes !== NO_CHANGE) {
updateDirectiveInputValue(context, lView, tNode, bindingIndex, classes, true, firstUpdatePass);
classes = NO_CHANGE;
}

// 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 (firstUpdatePass) {
patchConfig(tNode, TNodeFlags.hasClassMapBindings);
patchHostStylingFlag(tNode, isHostStyling(), true);
}

stylingMap(
context, tNode, firstUpdatePass, lView, bindingIndex, classes, true, hasDirectiveInput);
}
Expand Down Expand Up @@ -336,20 +369,13 @@ function stylingMap(
throwErrorIfNoChangesMode(false, oldValue, value);
}

// 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 (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, firstUpdatePass)) {
if (allowDirectStyling(tNode, isClassBased, firstUpdatePass)) {
const sanitizerToUse = isClassBased ? null : sanitizer;
const renderer = getRenderer(tNode, lView);
applyStylingMapDirectly(
renderer, context, native, lView, bindingIndex, value, isClassBased, sanitizerToUse,
renderer, context, tNode, native, lView, bindingIndex, value, isClassBased, sanitizerToUse,
valueHasChanged, hasDirectiveInput);
if (sanitizerToUse) {
// it's important we remove the current style sanitizer once the
Expand All @@ -368,12 +394,12 @@ function stylingMap(
// value to the element.
if (isClassBased) {
updateClassViaContext(
context, lView, native, directiveIndex, null, bindingIndex, stylingMapArr,
context, tNode, lView, native, directiveIndex, null, bindingIndex, stylingMapArr,
valueHasChanged, firstUpdatePass);
} else {
updateStyleViaContext(
context, lView, native, directiveIndex, null, bindingIndex, stylingMapArr, sanitizer,
valueHasChanged, firstUpdatePass);
context, tNode, lView, native, directiveIndex, null, bindingIndex, stylingMapArr,
sanitizer, valueHasChanged, firstUpdatePass);
}

setElementExitFn(stylingApply);
Expand Down Expand Up @@ -463,7 +489,7 @@ function stylingApply(): void {
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,
renderer, lView, tNode, classesContext, stylesContext, native, directiveIndex, sanitizer,
tView.firstUpdatePass);
resetCurrentStyleSanitizer();
}
Expand Down Expand Up @@ -541,7 +567,7 @@ function getContext(tNode: TNode, isClassBased: boolean): TStylingContext {
const hasDirectives = isDirectiveHost(tNode);
context = allocTStylingContext(context as StylingMapArray | null, hasDirectives);
if (ngDevMode) {
attachStylingDebugObject(context as TStylingContext, isClassBased);
attachStylingDebugObject(context as TStylingContext, tNode, isClassBased);
}

if (isClassBased) {
Expand Down Expand Up @@ -582,3 +608,10 @@ function resolveStylePropValue(
function isHostStyling(): boolean {
return isHostStylingActive(getActiveDirectiveId());
}

function patchHostStylingFlag(tNode: TNode, hostBindingsMode: boolean, isClassBased: boolean) {
const flag = hostBindingsMode ?
isClassBased ? TNodeFlags.hasHostClassBindings : TNodeFlags.hasHostStyleBindings :
isClassBased ? TNodeFlags.hasTemplateClassBindings : TNodeFlags.hasTemplateStyleBindings;
patchConfig(tNode, flag);
}
116 changes: 109 additions & 7 deletions packages/core/src/render3/interfaces/node.ts
View file
Expand Up @@ -67,19 +67,121 @@ export const enum TNodeFlags {
/** Bit #6 - This bit is set if the node has any "style" inputs */
hasStyleInput = 0x20,

/** Bit #7 - This bit is set if the node has initial styling */
hasInitialStyling = 0x40,

/** Bit #8 - This bit is set if the node has been detached by i18n */
isDetached = 0x80,
/** Bit #7 This bit is set if the node has been detached by i18n */
isDetached = 0x40,

/**
* Bit #9 - This bit is set if the node has directives with host bindings.
* Bit #8 - This bit is set if the node has directives with host bindings.
*
* This flags allows us to guard host-binding logic and invoke it only on nodes
* that actually have directives with host bindings.
*/
hasHostBindings = 0x100,
hasHostBindings = 0x80,

/** Bit #9 - This bit is set if the node has initial styling */
hasInitialStyling = 0x100,

/**
* Bit #10 - Whether or not there are class-based map bindings present.
*
* Examples include:
* 1. `<div [class]="x">`
* 2. `@HostBinding('class') x`
*/
hasClassMapBindings = 0x200,

/**
* Bit #11 - Whether or not there are any class-based prop bindings present.
*
* Examples include:
* 1. `<div [class.name]="x">`
* 2. `@HostBinding('class.name') x`
*/
hasClassPropBindings = 0x400,

/**
* Bit #12 - whether or not there are any active [class] and [class.name] bindings
*/
hasClassPropAndMapBindings = hasClassMapBindings | hasClassPropBindings,

/**
* Bit #13 - Whether or not the context contains one or more class-based template bindings.
*
* Examples include:
* 1. `<div [class]="x">`
* 2. `<div [class.name]="x">`
*/
hasTemplateClassBindings = 0x800,

/**
* Bit #14 - Whether or not the context contains one or more class-based host bindings.
*
* Examples include:
* 1. `@HostBinding('class') x`
* 2. `@HostBinding('class.name') x`
*/
hasHostClassBindings = 0x1000,

/**
* Bit #15 - Whether or not there are two or more sources for a class property in the context.
*
* Examples include:
* 1. prop + prop: `<div [class.active]="x" dir-that-sets-active-class>`
* 2. map + prop: `<div [class]="x" [class.foo]>`
* 3. map + map: `<div [class]="x" dir-that-sets-class>`
*/
hasDuplicateClassBindings = 0x2000,

/**
* Bit #16 - Whether or not there are style-based map bindings present.
*
* Examples include:
* 1. `<div [style]="x">`
* 2. `@HostBinding('style') x`
*/
hasStyleMapBindings = 0x4000,

/**
* Bit #17 - Whether or not there are any style-based prop bindings present.
*
* Examples include:
* 1. `<div [style.prop]="x">`
* 2. `@HostBinding('style.prop') x`
*/
hasStylePropBindings = 0x8000,

/**
* Bit #18 - whether or not there are any active [style] and [style.prop] bindings
*/
hasStylePropAndMapBindings = hasStyleMapBindings | hasStylePropBindings,

/**
* Bit #19 - Whether or not the context contains one or more style-based template bindings.
*
* Examples include:
* 1. `<div [style]="x">`
* 2. `<div [style.prop]="x">`
*/
hasTemplateStyleBindings = 0x10000,

/**
* Bit #20 - Whether or not the context contains one or more style-based host bindings.
*
* Examples include:
* 1. `@HostBinding('style') x`
* 2. `@HostBinding('style.prop') x`
*/
hasHostStyleBindings = 0x20000,

/**
* Bit #21 - Whether or not there are two or more sources for a style property in the context.
*
* Examples include:
* 1. prop + prop: `<div [style.width]="x" dir-that-sets-width>`
* 2. map + prop: `<div [style]="x" [style.prop]>`
* 3. map + map: `<div [style]="x" dir-that-sets-style>`
*/
hasDuplicateStyleBindings = 0x40000,
}

/**
Expand Down

0 comments on commit 41560b4

Please sign in to comment.