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.

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

perf(ivy): apply static styles/classes directly to an element's style/className properties #33364

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions packages/core/src/render3/instructions/element.ts
Expand Up @@ -65,7 +65,7 @@ export function ɵɵelementStart(
}

if ((tNode.flags & TNodeFlags.hasInitialStyling) === TNodeFlags.hasInitialStyling) {
renderInitialStyling(renderer, native, tNode);
renderInitialStyling(renderer, native, tNode, false);
}

appendChild(native, tNode, lView);
Expand Down Expand Up @@ -222,7 +222,7 @@ export function ɵɵelementHostAttrs(attrs: TAttributes) {
// attribute values to the element.
if (stylingNeedsToBeRendered) {
const renderer = lView[RENDERER];
renderInitialStyling(renderer, native, tNode);
renderInitialStyling(renderer, native, tNode, true);
}
}
}
Expand Down
24 changes: 20 additions & 4 deletions packages/core/src/render3/instructions/shared.ts
Expand Up @@ -31,10 +31,11 @@ import {BINDING_INDEX, CHILD_HEAD, CHILD_TAIL, CLEANUP, CONTEXT, DECLARATION_VIE
import {assertNodeOfPossibleTypes} from '../node_assert';
import {isNodeMatchingSelectorList} from '../node_selector_matcher';
import {ActiveElementFlags, enterView, executeElementExitFn, getBindingsEnabled, getCheckNoChangesMode, getIsParent, getPreviousOrParentTNode, getSelectedIndex, hasActiveElementFlag, incrementActiveDirectiveId, leaveView, leaveViewProcessExit, setActiveHostElement, setBindingRoot, setCheckNoChangesMode, setCurrentDirectiveDef, setCurrentQueryIndex, setPreviousOrParentTNode, setSelectedIndex} from '../state';
import {renderStylingMap} from '../styling/bindings';
import {renderStylingMap, writeStylingValueDirectly} from '../styling/bindings';
import {NO_CHANGE} from '../tokens';
import {isAnimationProp} from '../util/attrs_utils';
import {INTERPOLATION_DELIMITER, renderStringify, stringifyForError} from '../util/misc_utils';
import {getInitialStylingValue} from '../util/styling_utils';
import {getLViewParent} from '../util/view_traversal_utils';
import {getComponentLViewByIndex, getNativeByIndex, getNativeByTNode, getTNode, isCreationMode, readPatchedLView, resetPreOrderHookFlags, unwrapRNode, viewAttachedToChangeDetector} from '../util/view_utils';

Expand Down Expand Up @@ -1832,7 +1833,22 @@ export function textBindingInternal(lView: LView, index: number, value: string):
* applied once the element is instantiated. This function applies each of the static
* style and class entries to the element.
*/
export function renderInitialStyling(renderer: Renderer3, native: RElement, tNode: TNode) {
renderStylingMap(renderer, native, tNode.classes, true);
renderStylingMap(renderer, native, tNode.styles, false);
export function renderInitialStyling(
renderer: Renderer3, native: RElement, tNode: TNode, append: boolean) {
if (tNode.classes !== null) {
if (append) {
renderStylingMap(renderer, native, tNode.classes, true);
} else {
const classes = getInitialStylingValue(tNode.classes);
writeStylingValueDirectly(renderer, native, classes, true, null);
}
}
if (tNode.styles !== null) {
if (append) {
renderStylingMap(renderer, native, tNode.styles, false);
} else {
const styles = getInitialStylingValue(tNode.styles);
writeStylingValueDirectly(renderer, native, styles, false, null);
}
}
}
4 changes: 1 addition & 3 deletions packages/core/src/render3/instructions/styling.ts
Expand Up @@ -441,9 +441,7 @@ function normalizeStylingDirectiveInputValue(
if (isClassBased) {
value = concatString(initialValue, forceClassesAsString(bindingValue));
} else {
value = concatString(
initialValue,
forceStylesAsString(bindingValue as{[key: string]: any} | null | undefined, true), ';');
value = concatString(initialValue, forceStylesAsString(bindingValue, true), ';');
}
}
return value;
Expand Down
38 changes: 24 additions & 14 deletions packages/core/src/render3/styling/bindings.ts
Expand Up @@ -700,20 +700,10 @@ export function applyStylingMapDirectly(
}

if (writeToAttrDirectly) {
let valueToApply: string;
if (isClassBased) {
valueToApply = typeof value === 'string' ? value : objectToClassName(value);
if (initialValue !== null) {
valueToApply = concatString(initialValue, valueToApply, ' ');
}
setClassName(renderer, element, valueToApply);
} else {
valueToApply = forceStylesAsString(value as{[key: string]: any}, true);
if (initialValue !== null) {
valueToApply = initialValue + ';' + valueToApply;
}
setStyleAttr(renderer, element, valueToApply);
}
const initialValue =
hasInitial && !bindingValueContainsInitial ? getInitialStylingValue(context) : null;
const valueToApply =
writeStylingValueDirectly(renderer, element, value, isClassBased, initialValue);
setValue(data, cachedValueIndex, valueToApply || null);
} else {
const applyFn = isClassBased ? setClass : setStyle;
Expand Down Expand Up @@ -751,6 +741,26 @@ export function applyStylingMapDirectly(
}
}

export function writeStylingValueDirectly(
renderer: any, element: RElement, value: {[key: string]: any} | string | null,
isClassBased: boolean, initialValue: string | null): string {
let valueToApply: string;
if (isClassBased) {
valueToApply = typeof value === 'string' ? value : objectToClassName(value);
if (initialValue !== null) {
valueToApply = concatString(initialValue, valueToApply, ' ');
}
setClassName(renderer, element, valueToApply);
} else {
valueToApply = forceStylesAsString(value, true);
if (initialValue !== null) {
valueToApply = initialValue + ';' + valueToApply;
}
setStyleAttr(renderer, element, valueToApply);
}
return valueToApply;
}

/**
* Applies the provided styling prop/value to the element directly (without context resolution).
*
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/render3/util/styling_utils.ts
Expand Up @@ -296,7 +296,8 @@ export function forceClassesAsString(classes: string | {[key: string]: any} | nu
}

export function forceStylesAsString(
styles: {[key: string]: any} | null | undefined, hyphenateProps: boolean): string {
styles: {[key: string]: any} | string | null | undefined, hyphenateProps: boolean): string {
if (typeof styles == 'string') return styles;
let str = '';
if (styles) {
const props = Object.keys(styles);
Expand Down
Expand Up @@ -269,6 +269,9 @@
{
"name": "findDirectiveMatches"
},
{
"name": "forceStylesAsString"
},
{
"name": "generateExpandoInstructionBlock"
},
Expand Down Expand Up @@ -404,6 +407,9 @@
{
"name": "hasTagAndTypeMatch"
},
{
"name": "hyphenate"
},
{
"name": "includeViewProviders"
},
Expand Down Expand Up @@ -527,6 +533,9 @@
{
"name": "noSideEffects"
},
{
"name": "objectToClassName"
},
{
"name": "postProcessBaseDirective"
},
Expand Down Expand Up @@ -605,6 +614,9 @@
{
"name": "setClass"
},
{
"name": "setClassName"
},
{
"name": "setCurrentDirectiveDef"
},
Expand Down Expand Up @@ -644,6 +656,9 @@
{
"name": "setStyle"
},
{
"name": "setStyleAttr"
},
{
"name": "setUpAttributes"
},
Expand All @@ -668,6 +683,9 @@
{
"name": "viewAttachedToChangeDetector"
},
{
"name": "writeStylingValueDirectly"
},
{
"name": "ɵɵdefineComponent"
},
Expand Down
18 changes: 18 additions & 0 deletions packages/core/test/bundling/todo/bundle.golden_symbols.json
Expand Up @@ -593,6 +593,9 @@
{
"name": "flushStyling"
},
{
"name": "forceStylesAsString"
},
{
"name": "forwardRef"
},
Expand Down Expand Up @@ -851,6 +854,9 @@
{
"name": "hasValueChanged"
},
{
"name": "hyphenate"
},
{
"name": "includeViewProviders"
},
Expand Down Expand Up @@ -1079,6 +1085,9 @@
{
"name": "normalizeBitMaskValue"
},
{
"name": "objectToClassName"
},
{
"name": "patchConfig"
},
Expand Down Expand Up @@ -1208,6 +1217,9 @@
{
"name": "setClass"
},
{
"name": "setClassName"
},
{
"name": "setCurrentDirectiveDef"
},
Expand Down Expand Up @@ -1262,6 +1274,9 @@
{
"name": "setStyle"
},
{
"name": "setStyleAttr"
},
{
"name": "setUpAttributes"
},
Expand Down Expand Up @@ -1340,6 +1355,9 @@
{
"name": "wrapListener"
},
{
"name": "writeStylingValueDirectly"
},
{
"name": "ɵɵadvance"
},
Expand Down