From 049fe82de68efaf6e07d5ba7e7f87323c40582b9 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Fri, 16 May 2025 14:20:20 +0200 Subject: [PATCH] refactor(core): consolidate element start logic (#61409) The logic for the `elementStart` and `elementContainerStart` instructions was identical. These changes consolidate it into a single base instruction. PR Close #61409 --- .../core/src/render3/instructions/element.ts | 103 +++++------------- .../render3/instructions/element_container.ts | 73 ++++--------- .../core/src/render3/instructions/shared.ts | 87 ++++++++++++++- packages/core/src/render3/view/elements.ts | 2 +- .../bundle.golden_symbols.json | 3 +- .../bundling/defer/bundle.golden_symbols.json | 2 +- .../forms_reactive/bundle.golden_symbols.json | 2 +- .../bundle.golden_symbols.json | 2 +- .../router/bundle.golden_symbols.json | 2 +- 9 files changed, 141 insertions(+), 135 deletions(-) diff --git a/packages/core/src/render3/instructions/element.ts b/packages/core/src/render3/instructions/element.ts index 8faec764541..28b3c1c3670 100644 --- a/packages/core/src/render3/instructions/element.ts +++ b/packages/core/src/render3/instructions/element.ts @@ -24,33 +24,23 @@ import { setSegmentHead, } from '../../hydration/utils'; import {isDetachedByI18n} from '../../i18n/utils'; -import {assertDefined, assertEqual, assertIndexInRange} from '../../util/assert'; +import {assertDefined, assertEqual} from '../../util/assert'; import {assertHasParent} from '../assert'; -import {attachPatchData} from '../context_discovery'; -import { - clearElementContents, - createElementNode, - setupStaticAttributes, -} from '../dom_node_manipulation'; -import {hasClassInput, hasStyleInput, TElementNode, TNode, TNodeType} from '../interfaces/node'; -import {Renderer} from '../interfaces/renderer'; +import {clearElementContents, createElementNode} from '../dom_node_manipulation'; +import {hasClassInput, hasStyleInput, TNode, TNodeType} from '../interfaces/node'; import {RElement} from '../interfaces/renderer_dom'; import {isComponentHost, isDirectiveHost} from '../interfaces/type_checks'; -import {HEADER_OFFSET, HYDRATION, LView, RENDERER, TView} from '../interfaces/view'; +import {HYDRATION, LView, RENDERER, TView} from '../interfaces/view'; import {assertTNodeType} from '../node_assert'; -import {appendChild} from '../node_manipulation'; -import {executeContentQueries} from '../queries/query_execution'; import { decreaseElementDepthCount, enterSkipHydrationBlock, getBindingIndex, getBindingsEnabled, getCurrentTNode, - getElementDepthCount, getLView, getNamespace, getTView, - increaseElementDepthCount, isCurrentTNodeParent, isInSkipHydrationBlock, isSkipHydrationRootTNode, @@ -58,17 +48,12 @@ import { leaveSkipHydrationBlock, setCurrentTNode, setCurrentTNodeAsNotParent, - wasLastNodeCreated, } from '../state'; -import {elementEndFirstCreatePass, elementLikeStartFirstCreatePass} from '../view/elements'; +import {elementEndFirstCreatePass} from '../view/elements'; import {validateElementIsKnown} from './element_validation'; import {setDirectiveInputsWhichShadowsStyling} from './property'; -import { - createDirectivesInstances, - findDirectiveDefMatches, - saveResolvedLocalsInData, -} from './shared'; +import {elementLikeStartShared} from './shared'; /** * Create DOM element. The instruction must later be followed by `elementEnd()` call. @@ -93,64 +78,36 @@ export function ɵɵelementStart( ): typeof ɵɵelementStart { const lView = getLView(); const tView = getTView(); - const adjustedIndex = HEADER_OFFSET + index; - + const bindingsEnabled = getBindingsEnabled(); ngDevMode && assertEqual( getBindingIndex(), tView.bindingStartIndex, 'elements should be created before any bindings', ); - ngDevMode && assertIndexInRange(lView, adjustedIndex); - - const renderer = lView[RENDERER]; - const tNode = tView.firstCreatePass - ? elementLikeStartFirstCreatePass( - adjustedIndex, - tView, - lView, - TNodeType.Element, - name, - findDirectiveDefMatches, - getBindingsEnabled(), - attrsIndex, - localRefsIndex, - ) - : (tView.data[adjustedIndex] as TElementNode); - - const native = _locateOrCreateElementNode(tView, lView, tNode, renderer, name, index); - lView[adjustedIndex] = native; - - const hasDirectives = isDirectiveHost(tNode); + const tNode = elementLikeStartShared( + lView, + tView, + index, + TNodeType.Element, + name, + _locateOrCreateElementNode, + bindingsEnabled, + attrsIndex, + localRefsIndex, + ); if (ngDevMode && tView.firstCreatePass) { - validateElementIsKnown(native, lView, tNode.value, tView.schemas, hasDirectives); + const lView = getLView(); + validateElementIsKnown( + lView[tNode.index], + lView, + tNode.value, + tView.schemas, + isDirectiveHost(tNode), + ); } - setCurrentTNode(tNode, true); - setupStaticAttributes(renderer, native, tNode); - - if (!isDetachedByI18n(tNode) && wasLastNodeCreated()) { - // In the i18n case, the translation may have removed this element, so only add it if it is not - // detached. See `TNodeType.Placeholder` and `LFrame.inI18n` for more context. - appendChild(tView, lView, native, tNode); - } - - // any immediate children of a component or template container must be pre-emptively - // monkey-patched with the component view data so that the element can be inspected - // later on using any element discovery utility methods (see `element_discovery.ts`) - if (getElementDepthCount() === 0 || hasDirectives) { - attachPatchData(native, lView); - } - increaseElementDepthCount(); - - if (hasDirectives) { - createDirectivesInstances(tView, lView, tNode); - executeContentQueries(tView, tNode, lView); - } - if (localRefsIndex !== null) { - saveResolvedLocalsInData(lView, tNode); - } return ɵɵelementStart; } @@ -162,6 +119,7 @@ export function ɵɵelementStart( */ export function ɵɵelementEnd(): typeof ɵɵelementEnd { let currentTNode = getCurrentTNode()!; + const tView = getTView(); ngDevMode && assertDefined(currentTNode, 'No parent node to close.'); if (isCurrentTNodeParent()) { setCurrentTNodeAsNotParent(); @@ -180,7 +138,6 @@ export function ɵɵelementEnd(): typeof ɵɵelementEnd { decreaseElementDepthCount(); - const tView = getTView(); if (tView.firstCreatePass) { elementEndFirstCreatePass(tView, tNode); } @@ -221,12 +178,11 @@ let _locateOrCreateElementNode: typeof locateOrCreateElementNodeImpl = ( tView: TView, lView: LView, tNode: TNode, - renderer: Renderer, name: string, index: number, ) => { lastNodeWasCreated(true); - return createElementNode(renderer, name, getNamespace()); + return createElementNode(lView[RENDERER], name, getNamespace()); }; /** @@ -237,7 +193,6 @@ function locateOrCreateElementNodeImpl( tView: TView, lView: LView, tNode: TNode, - renderer: Renderer, name: string, index: number, ): RElement { @@ -251,7 +206,7 @@ function locateOrCreateElementNodeImpl( // Regular creation mode. if (isNodeCreationMode) { - return createElementNode(renderer, name, getNamespace()); + return createElementNode(lView[RENDERER], name, getNamespace()); } // Hydration mode, looking up an existing element in DOM. diff --git a/packages/core/src/render3/instructions/element_container.ts b/packages/core/src/render3/instructions/element_container.ts index 7268d225110..72d6208c5ee 100644 --- a/packages/core/src/render3/instructions/element_container.ts +++ b/packages/core/src/render3/instructions/element_container.ts @@ -14,18 +14,15 @@ import { setSegmentHead, } from '../../hydration/utils'; import {isDetachedByI18n} from '../../i18n/utils'; -import {assertEqual, assertIndexInRange, assertNumber} from '../../util/assert'; +import {assertEqual, assertNumber} from '../../util/assert'; import {assertHasParent} from '../assert'; -import {attachPatchData} from '../context_discovery'; import {createCommentNode} from '../dom_node_manipulation'; import {registerPostOrderHooks} from '../hooks'; -import {TElementContainerNode, TNode, TNodeType} from '../interfaces/node'; +import {TNode, TNodeType} from '../interfaces/node'; import {RComment} from '../interfaces/renderer_dom'; -import {isContentQueryHost, isDirectiveHost} from '../interfaces/type_checks'; -import {HEADER_OFFSET, HYDRATION, LView, RENDERER, TView} from '../interfaces/view'; +import {isContentQueryHost} from '../interfaces/type_checks'; +import {HYDRATION, LView, RENDERER, TView} from '../interfaces/view'; import {assertTNodeType} from '../node_assert'; -import {appendChild} from '../node_manipulation'; -import {executeContentQueries} from '../queries/query_execution'; import { getBindingIndex, getBindingsEnabled, @@ -37,14 +34,8 @@ import { lastNodeWasCreated, setCurrentTNode, setCurrentTNodeAsNotParent, - wasLastNodeCreated, } from '../state'; -import { - createDirectivesInstances, - findDirectiveDefMatches, - saveResolvedLocalsInData, -} from './shared'; -import {elementLikeStartFirstCreatePass} from '../view/elements'; +import {elementLikeStartShared} from './shared'; /** * Creates a logical container for other nodes () backed by a comment node in the DOM. @@ -68,48 +59,24 @@ export function ɵɵelementContainerStart( ): typeof ɵɵelementContainerStart { const lView = getLView(); const tView = getTView(); - const adjustedIndex = HEADER_OFFSET + index; - - ngDevMode && assertIndexInRange(lView, adjustedIndex); + const bindingsEnabled = getBindingsEnabled(); ngDevMode && assertEqual( getBindingIndex(), tView.bindingStartIndex, 'element containers should be created before any bindings', ); - - const tNode = tView.firstCreatePass - ? elementLikeStartFirstCreatePass( - adjustedIndex, - tView, - lView, - TNodeType.ElementContainer, - 'ng-container', - findDirectiveDefMatches, - getBindingsEnabled(), - attrsIndex, - localRefsIndex, - ) - : (tView.data[adjustedIndex] as TElementContainerNode); - setCurrentTNode(tNode, true); - - const comment = _locateOrCreateElementContainerNode(tView, lView, tNode, index); - lView[adjustedIndex] = comment; - - if (wasLastNodeCreated()) { - appendChild(tView, lView, comment, tNode); - } - attachPatchData(comment, lView); - - if (isDirectiveHost(tNode)) { - createDirectivesInstances(tView, lView, tNode); - executeContentQueries(tView, tNode, lView); - } - - if (localRefsIndex != null) { - saveResolvedLocalsInData(lView, tNode); - } - + elementLikeStartShared( + lView, + tView, + index, + TNodeType.ElementContainer, + 'ng-container', + _locateOrCreateElementContainerNode, + bindingsEnabled, + attrsIndex, + localRefsIndex, + ); return ɵɵelementContainerStart; } @@ -166,10 +133,11 @@ let _locateOrCreateElementContainerNode: typeof locateOrCreateElementContainerNo tView: TView, lView: LView, tNode: TNode, + commentText: string, index: number, ) => { lastNodeWasCreated(true); - return createCommentNode(lView[RENDERER], ngDevMode ? 'ng-container' : ''); + return createCommentNode(lView[RENDERER], ngDevMode ? commentText : ''); }; /** @@ -181,6 +149,7 @@ function locateOrCreateElementContainerNode( tView: TView, lView: LView, tNode: TNode, + commentText: string, index: number, ): RComment { let comment: RComment; @@ -195,7 +164,7 @@ function locateOrCreateElementContainerNode( // Regular creation mode. if (isNodeCreationMode) { - return createCommentNode(lView[RENDERER], ngDevMode ? 'ng-container' : ''); + return createCommentNode(lView[RENDERER], ngDevMode ? commentText : ''); } // Hydration mode, looking up existing elements in DOM. diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index 01f0813a600..3ce6162ff12 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -38,9 +38,9 @@ import { TNodeType, } from '../interfaces/node'; import {Renderer} from '../interfaces/renderer'; -import {RComment, RElement} from '../interfaces/renderer_dom'; +import {RComment, RElement, RNode} from '../interfaces/renderer_dom'; import {SanitizerFn} from '../interfaces/sanitization'; -import {isComponentDef, isComponentHost} from '../interfaces/type_checks'; +import {isComponentDef, isComponentHost, isDirectiveHost} from '../interfaces/type_checks'; import { CONTEXT, ENVIRONMENT, @@ -60,21 +60,29 @@ import {profiler} from '../profiler'; import {ProfilerEvent} from '../profiler_types'; import { getCurrentDirectiveIndex, + getElementDepthCount, getSelectedIndex, + increaseElementDepthCount, isInCheckNoChangesMode, setCurrentDirectiveIndex, + setCurrentTNode, setSelectedIndex, + wasLastNodeCreated, } from '../state'; import {NO_CHANGE} from '../tokens'; import {INTERPOLATION_DELIMITER} from '../util/misc_utils'; import {renderStringify} from '../util/stringify_utils'; import {getComponentLViewByIndex, getNativeByTNode, unwrapLView} from '../util/view_utils'; -import {clearElementContents} from '../dom_node_manipulation'; +import {clearElementContents, setupStaticAttributes} from '../dom_node_manipulation'; import {createComponentLView} from '../view/construction'; import {selectIndexInternal} from './advance'; import {handleUnknownPropertyError, isPropertyValid, matchingSchemas} from './element_validation'; import {writeToDirectiveInput} from './write_to_directive_input'; +import {elementLikeStartFirstCreatePass} from '../view/elements'; +import {isDetachedByI18n} from '../../i18n/utils'; +import {appendChild} from '../node_manipulation'; +import {executeContentQueries} from '../queries/query_execution'; export function executeTemplate( tView: TView, @@ -554,6 +562,79 @@ function setInputsFromAttrs( } } +/** Shared code between instructions that indicate the start of an element. */ +export function elementLikeStartShared( + lView: LView, + tView: TView, + index: number, + type: TNodeType.Element | TNodeType.ElementContainer, + name: string, + locateOrCreateNativeNode: ( + tView: TView, + lView: LView, + tNode: TNode, + name: string, + index: number, + ) => RNode, + bindingsEnabled: boolean, + attrsIndex: number | null | undefined, + localRefsIndex: number | undefined, +) { + const adjustedIndex = HEADER_OFFSET + index; + const isElement = type === TNodeType.Element; + ngDevMode && assertIndexInRange(lView, adjustedIndex); + + const tNode = tView.firstCreatePass + ? elementLikeStartFirstCreatePass( + adjustedIndex, + tView, + lView, + type, + name, + findDirectiveDefMatches, + bindingsEnabled, + attrsIndex, + localRefsIndex, + ) + : (tView.data[adjustedIndex] as TElementNode | TElementContainerNode); + + const native = locateOrCreateNativeNode(tView, lView, tNode, name, index); + lView[adjustedIndex] = native; + setCurrentTNode(tNode, true); + + // It's important that this runs before we've instantiated the directives. + if (isElement) { + setupStaticAttributes(lView[RENDERER], native as RElement, tNode); + + // any immediate children of a component or template container must be pre-emptively + // monkey-patched with the component view data so that the element can be inspected + // later on using any element discovery utility methods (see `element_discovery.ts`) + if (getElementDepthCount() === 0 || isDirectiveHost(tNode)) { + attachPatchData(native, lView); + } + increaseElementDepthCount(); + } else { + attachPatchData(native, lView); + } + + if (wasLastNodeCreated() && (!isElement || !isDetachedByI18n(tNode))) { + // In the i18n case, the translation may have removed this element, so only add it if it is not + // detached. See `TNodeType.Placeholder` and `LFrame.inI18n` for more context. + appendChild(tView, lView, native, tNode); + } + + if (isDirectiveHost(tNode)) { + createDirectivesInstances(tView, lView, tNode); + executeContentQueries(tView, tNode, lView); + } + + if (localRefsIndex != null) { + saveResolvedLocalsInData(lView, tNode); + } + + return tNode; +} + /////////////////////////////// //// Bindings & interpolations /////////////////////////////// diff --git a/packages/core/src/render3/view/elements.ts b/packages/core/src/render3/view/elements.ts index ad1cd44bf4c..e6d0ee267d4 100644 --- a/packages/core/src/render3/view/elements.ts +++ b/packages/core/src/render3/view/elements.ts @@ -16,7 +16,7 @@ import { type TElementNode, } from '../interfaces/node'; import {isContentQueryHost} from '../interfaces/type_checks'; -import type {LView, TView} from '../interfaces/view'; +import {type LView, type TView} from '../interfaces/view'; import {computeStaticStyling} from '../styling/static_styling'; import {getOrCreateTNode} from '../tnode_manipulation'; import {mergeHostAttrs} from '../util/attrs_utils'; diff --git a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json index b483468db21..f557b6c3afd 100644 --- a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json @@ -274,7 +274,7 @@ "diPublicInInjector", "documentElement", "elementEndFirstCreatePass", - "elementStartFirstCreatePass", + "elementLikeStartFirstCreatePass", "enterDI", "enterView", "eraseStyles", @@ -366,6 +366,7 @@ "isCurrentTNodeParent", "isDestroyed", "isDetachedByI18n", + "isDirectiveHost", "isElementNode", "isEnvironmentProviders", "isFunction", diff --git a/packages/core/test/bundling/defer/bundle.golden_symbols.json b/packages/core/test/bundling/defer/bundle.golden_symbols.json index 9353cc9bbcc..b88a82b8e9e 100644 --- a/packages/core/test/bundling/defer/bundle.golden_symbols.json +++ b/packages/core/test/bundling/defer/bundle.golden_symbols.json @@ -276,7 +276,7 @@ "detectChangesInternal", "diPublicInInjector", "elementEndFirstCreatePass", - "elementStartFirstCreatePass", + "elementLikeStartFirstCreatePass", "enterDI", "enterView", "epoch", diff --git a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json index a219b346257..37a136d0a87 100644 --- a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json @@ -327,7 +327,7 @@ "detectChangesInternal", "diPublicInInjector", "elementEndFirstCreatePass", - "elementStartFirstCreatePass", + "elementLikeStartFirstCreatePass", "enterDI", "enterView", "epoch", diff --git a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json index 229dc61404d..4d2df872be0 100644 --- a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json @@ -318,7 +318,7 @@ "detectChangesInternal", "diPublicInInjector", "elementEndFirstCreatePass", - "elementStartFirstCreatePass", + "elementLikeStartFirstCreatePass", "enterDI", "enterView", "epoch", diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index e8f84d38c74..005fc943a80 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -385,7 +385,7 @@ "detectChangesInternal", "diPublicInInjector", "elementEndFirstCreatePass", - "elementStartFirstCreatePass", + "elementLikeStartFirstCreatePass", "emptyPathMatch", "encodeUriQuery", "encodeUriSegment",