From 28516fd8528f277647353105d3cdcd1de2ab29c8 Mon Sep 17 00:00:00 2001 From: Jessica Janiuk Date: Wed, 23 Oct 2024 09:41:56 -0400 Subject: [PATCH] refactor(core): prevent annotating in hydrate never blocks (#58328) This fixes an issue with hydrate never where jsaction annotations would still hydrate even when they should not.' PR Close #58328 --- packages/core/src/hydration/annotate.ts | 66 +++++++++---------- .../test/incremental_hydration_spec.ts | 52 +++++++++++++++ 2 files changed, 83 insertions(+), 35 deletions(-) diff --git a/packages/core/src/hydration/annotate.ts b/packages/core/src/hydration/annotate.ts index 6333599b677..fee6d5b44e4 100644 --- a/packages/core/src/hydration/annotate.ts +++ b/packages/core/src/hydration/annotate.ts @@ -134,19 +134,6 @@ function getSsrId(tView: TView): string { return tView.ssrId; } -/** - * Global counter that is used to generate a unique id for Defer Blocks - * during the serialization process. - */ -let deferBlockSsrId = 0; - -/** - * Generates a unique id for a Defer Block. - */ -function getDeferBlockId(): string { - return `d${deferBlockSsrId++}`; -} - /** * Describes a context available during the serialization * process. The context is used to share and collect information @@ -407,6 +394,8 @@ function serializeLContainer( [NUM_ROOT_NODES]: numRootNodes, }; + let isHydrateNeverBlock = false; + // If this is a defer block, serialize extra info. if (isDeferBlock(lView[TVIEW], tNode)) { const lDetails = getLDeferBlockDetails(lView, tNode); @@ -414,11 +403,14 @@ function serializeLContainer( if (context.isIncrementalHydrationEnabled) { const deferBlockId = `d${context.deferBlocks.size}`; + const tDetails = getTDeferBlockDetails(lView[TVIEW], tNode); + if (tDetails.hydrateTriggers?.has(DeferBlockTrigger.Never)) { + isHydrateNeverBlock = true; + } + let rootNodes: any[] = []; collectNativeNodesInLContainer(lContainer, rootNodes); - const tDetails = getTDeferBlockDetails(lView[TVIEW], tNode); - // Add defer block into info context.deferBlocks const deferBlockInfo: SerializedDeferBlock = { [DEFER_PARENT_BLOCK_ID]: parentDeferBlockId, @@ -440,7 +432,10 @@ function serializeLContainer( for (let et of actionList) { context.eventTypesToReplay.regular.add(et); } - annotateDeferBlockRootNodesWithJsAction(actionList, rootNodes, deferBlockId); + + if (!isHydrateNeverBlock) { + annotateDeferBlockRootNodesWithJsAction(actionList, rootNodes, deferBlockId); + } // Use current block id as parent for nested routes. parentDeferBlockId = deferBlockId; @@ -453,11 +448,13 @@ function serializeLContainer( serializedView[DEFER_BLOCK_STATE] = lDetails[CURRENT_DEFER_BLOCK_STATE]; } - // TODO(incremental-hydration): avoid copying of an object here - serializedView = { - ...serializedView, - ...serializeLView(lContainer[i] as LView, parentDeferBlockId, context, injector), - }; + if (!isHydrateNeverBlock) { + // TODO(incremental-hydration): avoid copying of an object here + serializedView = { + ...serializedView, + ...serializeLView(lContainer[i] as LView, parentDeferBlockId, context, injector), + }; + } } // Check if the previous view has the same shape (for example, it was @@ -610,19 +607,6 @@ function serializeLView( continue; } - // Attach `jsaction` attribute to elements that have registered listeners, - // thus potentially having a need to do an event replay. - if (nativeElementsToEventTypes && tNode.type & TNodeType.Element) { - const nativeElement = unwrapRNode(lView[i]) as Element; - if (nativeElementsToEventTypes.has(nativeElement)) { - setJSActionAttributes( - nativeElement, - nativeElementsToEventTypes.get(nativeElement)!, - parentDeferBlockId, - ); - } - } - if (Array.isArray(tNode.projection)) { for (const projectionHeadTNode of tNode.projection) { // We may have `null`s in slots with no projected content. @@ -663,7 +647,6 @@ function serializeLView( } conditionallyAnnotateNodePath(ngh, tNode, lView, i18nChildren); - if (isLContainer(lView[i])) { // Serialize information about a template. const embeddedTView = tNode.tView; @@ -744,6 +727,19 @@ function serializeLView( processTextNodeBeforeSerialization(context, rNode); } } + + // Attach `jsaction` attribute to elements that have registered listeners, + // thus potentially having a need to do an event replay. + if (nativeElementsToEventTypes && tNode.type & TNodeType.Element) { + const nativeElement = unwrapRNode(lView[i]) as Element; + if (nativeElementsToEventTypes.has(nativeElement)) { + setJSActionAttributes( + nativeElement, + nativeElementsToEventTypes.get(nativeElement)!, + parentDeferBlockId, + ); + } + } } return ngh; } diff --git a/packages/platform-server/test/incremental_hydration_spec.ts b/packages/platform-server/test/incremental_hydration_spec.ts index 8788df8800e..0ce4fa6f2cd 100644 --- a/packages/platform-server/test/incremental_hydration_spec.ts +++ b/packages/platform-server/test/incremental_hydration_spec.ts @@ -1339,6 +1339,58 @@ describe('platform-server partial hydration integration', () => { expect(appHostNode.outerHTML).not.toContain('Outer block placeholder'); }, 100_000); + + it('should not annotate jsaction events for events inside a hydrate never block', async () => { + @Component({ + standalone: true, + selector: 'app', + template: ` +
+ @defer (on timer(1s); hydrate never) { +
+ defer block rendered! + {{value()}} + @defer(on immediate; hydrate on idle) { +

shouldn't be annotated

+ } @placeholder { +

blah de blah

+ } +
+ } @placeholder { + Outer block placeholder + } + @defer (on timer(1s); hydrate on viewport) { +
+ viewport section +

has a binding

+
+ } @placeholder { + another placeholder + } +
+ `, + }) + class SimpleComponent { + value = signal('start'); + fnA() {} + fnB() { + this.value.set('end'); + } + } + + const appId = 'custom-app-id'; + const providers = [{provide: APP_ID, useValue: appId}]; + const hydrationFeatures = [withIncrementalHydration()]; + + const html = await ssr(SimpleComponent, {envProviders: providers, hydrationFeatures}); + const ssrContents = getAppContents(html); + + expect(ssrContents).not.toContain('start'); + expect(ssrContents).toContain('

has a binding

'); + expect(ssrContents).not.toContain('

shouldn\'t be annotated

'); + }, 100_000); }); describe('cleanup', () => {