refactor(core): cleanup incremental hydration code (angular#58363)

This cleans up some minor issues with the incremental hydration implementation and should make maintaining it a little easier.

PR Close angular#58363

Author: thePunderWoman

packages/core/src/defer/cleanup.ts

@@ -13,7 +13,7 @@ import {
   PREFETCH_TRIGGER_CLEANUP_FNS,
   TRIGGER_CLEANUP_FNS,
   TriggerType,
-  UNIQUE_SSR_ID,
+  SSR_UNIQUE_ID,
 } from './interfaces';
 import {DeferBlockRegistry} from './registry';
 
@@ -57,7 +57,7 @@ export function invokeAllTriggerCleanupFns(
   // TODO(incremental-hydration): cleanup functions are invoked in multiple places
   // should we centralize where cleanup functions are invoked to this registry?
   if (registry !== null) {
-    registry.invokeCleanupFns(lDetails[UNIQUE_SSR_ID]!);
+    registry.invokeCleanupFns(lDetails[SSR_UNIQUE_ID]!);
   }
 
   invokeTriggerCleanupFns(TriggerType.Prefetch, lDetails);

packages/core/src/defer/instructions.ts

@@ -79,12 +79,12 @@ import {
   LOADING_AFTER_CLEANUP_FN,
   NEXT_DEFER_BLOCK_STATE,
   ON_COMPLETE_FNS,
-  SSR_STATE,
+  SSR_BLOCK_STATE,
   STATE_IS_FROZEN_UNTIL,
   TDeferBlockDetails,
   TriggerType,
   DeferBlock,
-  UNIQUE_SSR_ID,
+  SSR_UNIQUE_ID,
 } from './interfaces';
 import {onTimer, scheduleTimerTrigger} from './timer_scheduler';
 import {
@@ -144,7 +144,8 @@ function shouldTriggerWhenOnClient(
   if (!isPlatformBrowser(injector)) {
     return false;
   }
-  const isServerRendered = lDetails[SSR_STATE] && lDetails[SSR_STATE] === DeferBlockState.Complete;
+  const isServerRendered =
+    lDetails[SSR_BLOCK_STATE] && lDetails[SSR_BLOCK_STATE] === DeferBlockState.Complete;
   const hasHydrateTriggers = tDetails.hydrateTriggers && tDetails.hydrateTriggers.size > 0;
   if (hasHydrateTriggers && isServerRendered && isIncrementalHydrationEnabled(injector)) {
     return false;
@@ -280,13 +281,12 @@ export function ɵɵdefer(
   // In client-only mode, this function is a noop.
   populateDehydratedViewsInLContainer(lContainer, tNode, lView);
 
-  let ssrState = null;
-  let uniqueId: string | null = null;
+  let ssrBlockState = null;
+  let ssrUniqueId: string | null = null;
   if (lContainer[DEHYDRATED_VIEWS]?.length > 0) {
-    // TODO(incremental-hydration): this is a hack, we should serialize defer
     const info = lContainer[DEHYDRATED_VIEWS][0].data;
-    uniqueId = info[DEFER_BLOCK_ID] ?? null;
-    ssrState = info[SERIALIZED_DEFER_BLOCK_STATE];
+    ssrUniqueId = info[DEFER_BLOCK_ID] ?? null;
+    ssrBlockState = info[SERIALIZED_DEFER_BLOCK_STATE];
   }
 
   // Init instance-specific defer details and store it.
@@ -297,21 +297,21 @@ export function ɵɵdefer(
     null, // LOADING_AFTER_CLEANUP_FN
     null, // TRIGGER_CLEANUP_FNS
     null, // PREFETCH_TRIGGER_CLEANUP_FNS
-    uniqueId, // UNIQUE_ID
-    ssrState, // SSR_STATE
+    ssrUniqueId, // SSR_UNIQUE_ID
+    ssrBlockState, // SSR_BLOCK_STATE
     null, // ON_COMPLETE_FNS
     null, // HYDRATE_TRIGGER_CLEANUP_FNS
   ];
   setLDeferBlockDetails(lView, adjustedIndex, lDetails);
 
   let registry: DeferBlockRegistry | null = null;
-  if (uniqueId !== null) {
+  if (ssrUniqueId !== null) {
     // TODO(incremental-hydration): explore how we can make
     // `DeferBlockRegistry` tree-shakable for client-only cases.
     registry = injector.get(DeferBlockRegistry);
 
     // Also store this defer block in the registry.
-    registry.add(uniqueId, {lView, tNode, lContainer});
+    registry.add(ssrUniqueId, {lView, tNode, lContainer});
   }
 
   const cleanupTriggersFn = () => invokeAllTriggerCleanupFns(lDetails, registry);
@@ -415,7 +415,7 @@ export function ɵɵdeferHydrateWhen(rawValue: unknown) {
           // state.
           incrementallyHydrateFromBlockName(
             injector,
-            getLDeferBlockDetails(lView, tNode)[UNIQUE_SSR_ID]!,
+            getLDeferBlockDetails(lView, tNode)[SSR_UNIQUE_ID]!,
             (deferBlock: DeferBlock) => triggerAndWaitForCompletion(deferBlock),
           );
         }
@@ -534,7 +534,7 @@ export function ɵɵdeferHydrateOnImmediate() {
   } else {
     incrementallyHydrateFromBlockName(
       injector,
-      lDetails[UNIQUE_SSR_ID]!,
+      lDetails[SSR_UNIQUE_ID]!,
       (deferBlock: DeferBlock) => triggerAndWaitForCompletion(deferBlock),
     );
   }
@@ -643,6 +643,8 @@ export function ɵɵdeferHydrateOnHover() {
     // We are on the server and SSR for defer blocks is enabled.
     triggerDeferBlock(lView, tNode);
   }
+  // The actual triggering of hydration on hover is handled by JSAction in
+  // event_replay.ts.
 }
 
 /**
@@ -711,6 +713,8 @@ export function ɵɵdeferHydrateOnInteraction() {
     // We are on the server and SSR for defer blocks is enabled.
     triggerDeferBlock(lView, tNode);
   }
+  // The actual triggering of hydration on interaction is handled by JSAction in
+  // event_replay.ts.
 }
 
 /**
@@ -780,6 +784,8 @@ export function ɵɵdeferHydrateOnViewport() {
     // We are on the server and SSR for defer blocks is enabled.
     triggerDeferBlock(lView, tNode);
   }
+  // The actual triggering of hydration on viewport happens in incremental.ts,
+  // since these instructions won't exist for dehydrated content.
 }
 
 /********** Helper functions **********/
@@ -853,7 +859,7 @@ export function scheduleDelayedHydrating(
       () =>
         incrementallyHydrateFromBlockName(
           injector,
-          lDetails[UNIQUE_SSR_ID]!,
+          lDetails[SSR_UNIQUE_ID]!,
           (deferBlock: DeferBlock) => triggerAndWaitForCompletion(deferBlock),
         ),
       injector,
@@ -896,7 +902,7 @@ export function renderDeferBlockState(
 
   const currentState = lDetails[DEFER_BLOCK_STATE];
 
-  const ssrState = lDetails[SSR_STATE];
+  const ssrState = lDetails[SSR_BLOCK_STATE];
   if (ssrState !== null && newState < ssrState) {
     return; // trying to render a previous state, exit
   }

packages/core/src/defer/interfaces.ts

@@ -210,8 +210,8 @@ export const STATE_IS_FROZEN_UNTIL = 2;
 export const LOADING_AFTER_CLEANUP_FN = 3;
 export const TRIGGER_CLEANUP_FNS = 4;
 export const PREFETCH_TRIGGER_CLEANUP_FNS = 5;
-export const UNIQUE_SSR_ID = 6;
-export const SSR_STATE = 7;
+export const SSR_UNIQUE_ID = 6;
+export const SSR_BLOCK_STATE = 7;
 export const ON_COMPLETE_FNS = 8;
 export const HYDRATE_TRIGGER_CLEANUP_FNS = 9;
 
@@ -259,12 +259,12 @@ export interface LDeferBlockDetails extends Array<unknown> {
   /**
    * Unique id of this defer block assigned during SSR.
    */
-  [UNIQUE_SSR_ID]: string | null;
+  [SSR_UNIQUE_ID]: string | null;
 
   /**
    * Defer block state after SSR.
    */
-  [SSR_STATE]: number | null;
+  [SSR_BLOCK_STATE]: number | null;
 
   /**
    * A set of callbacks to be invoked once the main content is rendered.

packages/core/src/defer/registry.ts

@@ -9,13 +9,19 @@ import {ɵɵdefineInjectable} from '../di';
 import {DeferBlock} from './interfaces';
 
 // TODO(incremental-hydration): refactor this so that it's not used in CSR cases
+/**
+ * The DeferBlockRegistry is used for incremental hydration purposes. It keeps
+ * track of the Defer Blocks that need hydration so we can effectively
+ * navigate up to the top dehydrated defer block and fire appropriate cleanup
+ * functions post hydration.
+ */
 export class DeferBlockRegistry {
   private registry = new Map<string, DeferBlock>();
   private cleanupFns = new Map<string, Function[]>();
-  add(blockId: string, info: any) {
+  add(blockId: string, info: DeferBlock) {
     this.registry.set(blockId, info);
   }
-  get(blockId: string) {
+  get(blockId: string): DeferBlock | null {
     return this.registry.get(blockId) ?? null;
   }
   // TODO(incremental-hydration): we need to determine when this should be invoked
@@ -38,14 +44,17 @@ export class DeferBlockRegistry {
 
   invokeCleanupFns(blockId: string) {
     // TODO(incremental-hydration): determine if we can safely remove entries from
-    // the cleanupFns after they've been invoked
+    // the cleanupFns after they've been invoked. Can we reset
+    // `this.cleanupFns.get(blockId)`?
     const fns = this.cleanupFns.get(blockId) ?? [];
     for (let fn of fns) {
       fn();
     }
   }
 
   // Blocks that are being hydrated.
+  // TODO(incremental-hydration): cleanup task - we currently retain ids post hydration
+  // and need to determine when we can remove them.
   hydrating = new Set();
 
   /** @nocollapse */

packages/core/src/event_delegation_utils.ts

@@ -11,14 +11,9 @@ import {EventContract} from '@angular/core/primitives/event-dispatch';
 import {Attribute} from '@angular/core/primitives/event-dispatch';
 import {InjectionToken, Injector} from './di';
 import {RElement} from './render3/interfaces/renderer_dom';
-import {
-  BLOCK_ELEMENT_MAP,
-  EVENT_REPLAY_ENABLED_DEFAULT,
-  IS_EVENT_REPLAY_ENABLED,
-} from './hydration/tokens';
-import {OnDestroy} from './interface/lifecycle_hooks';
+import {BLOCK_ELEMENT_MAP} from './hydration/tokens';
 
-export const BLOCKNAME_ATTRIBUTE = 'ngb';
+export const DEFER_BLOCK_SSR_ID_ATTRIBUTE = 'ngb';
 
 declare global {
   interface Element {
@@ -41,7 +36,9 @@ export function setJSActionAttributes(
   eventTypes: string[],
   parentDeferBlockId: string | null = null,
 ) {
-  if (!eventTypes.length || nativeElement.nodeType !== Node.ELEMENT_NODE) {
+  // jsaction attributes specifically should be applied to elements and not comment nodes.
+  // Comment nodes also have no setAttribute function. So this avoids errors.
+  if (eventTypes.length === 0 || nativeElement.nodeType !== Node.ELEMENT_NODE) {
     return;
   }
   const existingAttr = nativeElement.getAttribute(Attribute.JSACTION);
@@ -57,7 +54,7 @@ export function setJSActionAttributes(
 
   const blockName = parentDeferBlockId ?? '';
   if (blockName !== '' && parts.length > 0) {
-    nativeElement.setAttribute(BLOCKNAME_ATTRIBUTE, blockName);
+    nativeElement.setAttribute(DEFER_BLOCK_SSR_ID_ATTRIBUTE, blockName);
   }
 }
 
@@ -71,7 +68,7 @@ export const sharedStashFunction = (rEl: RElement, eventType: string, listenerFn
 };
 
 export const sharedMapFunction = (rEl: RElement, jsActionMap: Map<string, Set<Element>>) => {
-  let blockName = rEl.getAttribute(BLOCKNAME_ATTRIBUTE) ?? '';
+  let blockName = rEl.getAttribute(DEFER_BLOCK_SSR_ID_ATTRIBUTE) ?? '';
   const el = rEl as unknown as Element;
   const blockSet = jsActionMap.get(blockName) ?? new Set<Element>();
   if (!blockSet.has(el)) {
@@ -94,7 +91,7 @@ export function removeListenersFromBlocks(blockNames: string[], injector: Inject
 
 export const removeListeners = (el: Element) => {
   el.removeAttribute(Attribute.JSACTION);
-  el.removeAttribute(BLOCKNAME_ATTRIBUTE);
+  el.removeAttribute(DEFER_BLOCK_SSR_ID_ATTRIBUTE);
   el.__jsaction_fns = undefined;
 };