Skip to content

Commit

Permalink
fix(core): handle projection of hydrated containters into components …
Browse files Browse the repository at this point in the history
…that skip hydration (#50199)

This commit updates hydration logic to support a scenario where a view container that was hydrated and later on projected to a component that skips hydration. Currently, such projected content is extracted from the DOM (since a component that skips hydration needs to be re-created), but never added back, since the current logic treats such content as "already inserted".

Closes #50175.

PR Close #50199
  • Loading branch information
AndrewKushnir authored and alxhub committed May 9, 2023
1 parent bb8fb10 commit f751ce6
Show file tree
Hide file tree
Showing 12 changed files with 328 additions and 11 deletions.
2 changes: 1 addition & 1 deletion packages/core/src/hydration/annotate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {TransferState} from '../transfer_state';
import {unsupportedProjectionOfDomNodes} from './error_handling';
import {CONTAINERS, DISCONNECTED_NODES, ELEMENT_CONTAINERS, MULTIPLIER, NODES, NUM_ROOT_NODES, SerializedContainerView, SerializedView, TEMPLATE_ID, TEMPLATES} from './interfaces';
import {calcPathForNode} from './node_lookup_utils';
import {isInSkipHydrationBlock, SKIP_HYDRATION_ATTR_NAME} from './skip_hydration';
import {hasInSkipHydrationBlockFlag, isInSkipHydrationBlock, SKIP_HYDRATION_ATTR_NAME} from './skip_hydration';
import {getComponentLViewForHydration, NGH_ATTR_NAME, NGH_DATA_KEY, TextNodeMarker} from './utils';

/**
Expand Down
14 changes: 13 additions & 1 deletion packages/core/src/hydration/skip_hydration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {TNode} from '../render3/interfaces/node';
import {TNode, TNodeFlags} from '../render3/interfaces/node';
import {LView} from '../render3/interfaces/view';

/**
Expand Down Expand Up @@ -36,10 +36,22 @@ export function hasNgSkipHydrationAttr(tNode: TNode): boolean {
return false;
}

/**
* Checks whether a TNode has a flag to indicate that it's a part of
* a skip hydration block.
*/
export function hasInSkipHydrationBlockFlag(tNode: TNode): boolean {
return (tNode.flags & TNodeFlags.inSkipHydrationBlock) === TNodeFlags.inSkipHydrationBlock;
}

/**
* Helper function that determines if a given node is within a skip hydration block
* by navigating up the TNode tree to see if any parent nodes have skip hydration
* attribute.
*
* TODO(akushnir): this function should contain the logic of `hasInSkipHydrationBlockFlag`,
* there is no need to traverse parent nodes when we have a TNode flag (which would also
* make this lookup O(1)).
*/
export function isInSkipHydrationBlock(tNode: TNode): boolean {
let currentTNode: TNode|null = tNode.parent;
Expand Down
23 changes: 18 additions & 5 deletions packages/core/src/linker/view_container_ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {Injector} from '../di/injector';
import {EnvironmentInjector} from '../di/r3_injector';
import {validateMatchingNode} from '../hydration/error_handling';
import {CONTAINERS} from '../hydration/interfaces';
import {isInSkipHydrationBlock} from '../hydration/skip_hydration';
import {hasInSkipHydrationBlockFlag, isInSkipHydrationBlock} from '../hydration/skip_hydration';
import {getSegmentHead, isDisconnectedNode, markRNodeAsClaimedByHydration} from '../hydration/utils';
import {findMatchingDehydratedView, locateDehydratedViewsInContainer} from '../hydration/views';
import {isType, Type} from '../interface/type';
Expand Down Expand Up @@ -311,7 +311,11 @@ const R3ViewContainerRef = class ViewContainerRef extends VE_ViewContainerRef {

const hydrationInfo = findMatchingDehydratedView(this._lContainer, templateRef.ssrId);
const viewRef = templateRef.createEmbeddedViewImpl(context || <any>{}, injector, hydrationInfo);
this.insertImpl(viewRef, index, !!hydrationInfo);
// If there is a matching dehydrated view, but the host TNode is located in the skip
// hydration block, this means that the content was detached (as a part of the skip
// hydration logic) and it needs to be appended into the DOM.
const skipDomInsertion = !!hydrationInfo && !hasInSkipHydrationBlockFlag(this._hostTNode);
this.insertImpl(viewRef, index, skipDomInsertion);
return viewRef;
}

Expand Down Expand Up @@ -428,7 +432,11 @@ const R3ViewContainerRef = class ViewContainerRef extends VE_ViewContainerRef {
const rNode = dehydratedView?.firstChild ?? null;
const componentRef =
componentFactory.create(contextInjector, projectableNodes, rNode, environmentInjector);
this.insertImpl(componentRef.hostView, index, !!dehydratedView);
// If there is a matching dehydrated view, but the host TNode is located in the skip
// hydration block, this means that the content was detached (as a part of the skip
// hydration logic) and it needs to be appended into the DOM.
const skipDomInsertion = !!dehydratedView && !hasInSkipHydrationBlockFlag(this._hostTNode);
this.insertImpl(componentRef.hostView, index, skipDomInsertion);
return componentRef;
}

Expand Down Expand Up @@ -638,8 +646,13 @@ function locateOrCreateAnchorNode(

const hydrationInfo = hostLView[HYDRATION];
const noOffsetIndex = hostTNode.index - HEADER_OFFSET;
const isNodeCreationMode = !hydrationInfo || isInSkipHydrationBlock(hostTNode) ||
isDisconnectedNode(hydrationInfo, noOffsetIndex);

// TODO(akushnir): this should really be a single condition, refactor the code
// to use `hasInSkipHydrationBlockFlag` logic inside `isInSkipHydrationBlock`.
const skipHydration = isInSkipHydrationBlock(hostTNode) || hasInSkipHydrationBlockFlag(hostTNode);

const isNodeCreationMode =
!hydrationInfo || skipHydration || isDisconnectedNode(hydrationInfo, noOffsetIndex);

// Regular creation mode.
if (isNodeCreationMode) {
Expand Down
10 changes: 7 additions & 3 deletions packages/core/src/render3/instructions/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {Injector} from '../../di/injector';
import {ErrorHandler} from '../../error_handler';
import {RuntimeError, RuntimeErrorCode} from '../../errors';
import {DehydratedView} from '../../hydration/interfaces';
import {SKIP_HYDRATION_ATTR_NAME} from '../../hydration/skip_hydration';
import {hasInSkipHydrationBlockFlag, SKIP_HYDRATION_ATTR_NAME} from '../../hydration/skip_hydration';
import {PRESERVE_HOST_CONTENT, PRESERVE_HOST_CONTENT_DEFAULT} from '../../hydration/tokens';
import {processTextNodeMarkersBeforeHydration} from '../../hydration/utils';
import {DoCheck, OnChanges, OnInit} from '../../interface/lifecycle_hooks';
Expand Down Expand Up @@ -42,7 +42,7 @@ import {clearElementContents, updateTextNode} from '../node_manipulation';
import {isInlineTemplate, isNodeMatchingSelectorList} from '../node_selector_matcher';
import {profiler, ProfilerEvent} from '../profiler';
import {commitLViewConsumerIfHasProducers, getReactiveLViewConsumer} from '../reactive_lview_consumer';
import {getBindingsEnabled, getCurrentDirectiveIndex, getCurrentParentTNode, getCurrentTNodePlaceholderOk, getSelectedIndex, isCurrentTNodeParent, isInCheckNoChangesMode, isInI18nBlock, leaveView, setBindingRootForHostBindings, setCurrentDirectiveIndex, setCurrentQueryIndex, setCurrentTNode, setSelectedIndex} from '../state';
import {getBindingsEnabled, getCurrentDirectiveIndex, getCurrentParentTNode, getCurrentTNodePlaceholderOk, getSelectedIndex, isCurrentTNodeParent, isInCheckNoChangesMode, isInI18nBlock, isInSkipHydrationBlock, leaveView, setBindingRootForHostBindings, setCurrentDirectiveIndex, setCurrentQueryIndex, setCurrentTNode, setSelectedIndex} from '../state';
import {NO_CHANGE} from '../tokens';
import {mergeHostAttrs} from '../util/attrs_utils';
import {INTERPOLATION_DELIMITER} from '../util/misc_utils';
Expand Down Expand Up @@ -584,6 +584,10 @@ export function createTNode(
ngDevMode && ngDevMode.tNode++;
ngDevMode && tParent && assertTNodeForTView(tParent, tView);
let injectorIndex = tParent ? tParent.injectorIndex : -1;
let flags = 0;
if (isInSkipHydrationBlock()) {
flags |= TNodeFlags.inSkipHydrationBlock;
}
const tNode = {
type,
index,
Expand All @@ -594,7 +598,7 @@ export function createTNode(
directiveStylingLast: -1,
componentOffset: -1,
propertyBindings: null,
flags: 0,
flags,
providerIndexes: 0,
value: value,
attrs: attrs,
Expand Down
7 changes: 6 additions & 1 deletion packages/core/src/render3/interfaces/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export const enum TNodeFlags {
/** Bit #5 - This bit is set if the node has any "style" inputs */
hasStyleInput = 0x10,

/** Bit #6 This bit is set if the node has been detached by i18n */
/** Bit #6 - This bit is set if the node has been detached by i18n */
isDetached = 0x20,

/**
Expand All @@ -125,6 +125,11 @@ export const enum TNodeFlags {
* that actually have directives with host bindings.
*/
hasHostBindings = 0x40,

/**
* Bit #8 - This bit is set if the node is a located inside skip hydration block.
*/
inSkipHydrationBlock = 0x80,
}

/**
Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/render3/node_manipulation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {hasInSkipHydrationBlockFlag} from '../hydration/skip_hydration';
import {ViewEncapsulation} from '../metadata/view';
import {RendererStyleFlags2} from '../render/api_flags';
import {addToArray, removeFromArray} from '../util/array_utils';
Expand Down Expand Up @@ -964,6 +965,11 @@ function applyProjectionRecursive(
} else {
let nodeToProject: TNode|null = nodeToProjectOrRNodes;
const projectedComponentLView = componentLView[PARENT] as LView;
// If a parent <ng-content> is located within a skip hydration block,
// annotate an actual node that is being projected with the same flag too.
if (hasInSkipHydrationBlockFlag(tProjectionNode)) {
nodeToProject.flags |= TNodeFlags.inSkipHydrationBlock;
}
applyNodes(
renderer, action, nodeToProject, projectedComponentLView, parentRElement, beforeNode, true);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1145,6 +1145,9 @@
{
"name": "handleError"
},
{
"name": "hasInSkipHydrationBlockFlag"
},
{
"name": "hasParentInjector"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1109,6 +1109,9 @@
{
"name": "handleError"
},
{
"name": "hasInSkipHydrationBlockFlag"
},
{
"name": "hasParentInjector"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -905,6 +905,9 @@
{
"name": "getTNodeFromLView"
},
{
"name": "hasInSkipHydrationBlockFlag"
},
{
"name": "hostReportError"
},
Expand Down
3 changes: 3 additions & 0 deletions packages/core/test/bundling/router/bundle.golden_symbols.json
Original file line number Diff line number Diff line change
Expand Up @@ -1442,6 +1442,9 @@
{
"name": "hasEmptyPathConfig"
},
{
"name": "hasInSkipHydrationBlockFlag"
},
{
"name": "hasParentInjector"
},
Expand Down
3 changes: 3 additions & 0 deletions packages/core/test/bundling/todo/bundle.golden_symbols.json
Original file line number Diff line number Diff line change
Expand Up @@ -1004,6 +1004,9 @@
{
"name": "handleError"
},
{
"name": "hasInSkipHydrationBlockFlag"
},
{
"name": "hasParentInjector"
},
Expand Down
Loading

0 comments on commit f751ce6

Please sign in to comment.