Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor(ivy): move all styling configurations into TNodeFlags #33540

Closed
wants to merge 1 commit into from
Closed

refactor(ivy): move all styling configurations into TNodeFlags #33540

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion integration/_payload-limits.json
View file
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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