refactor(core): incremental hydration TODO cleanup (#59032)

thePunderWoman · AndrewKushnir · commit c61ad65f646a · 2024-12-12T14:48:25.000-08:00
This adds a few helper functions and ensures we call complete fns when error state is rendered. It also eliminates serialized views from being copied. PR Close #59032
diff --git a/packages/core/src/defer/rendering.ts b/packages/core/src/defer/rendering.ts
@@ -223,17 +223,9 @@ function findMatchingDehydratedViewForDeferBlock(
   lContainer: LContainer,
   lDetails: LDeferBlockDetails,
 ): DehydratedContainerView | null {
-  // TODO(incremental-hydration): extract into a separate util function and use in relevant places.
-  const views = lContainer[DEHYDRATED_VIEWS];
-  if (views === null || views.length === 0) {
-    return null;
-  }
-
   // Find matching view based on serialized defer block state.
-  // TODO(incremental-hydration): reconcile this logic with the regular logic that looks up
-  // dehydrated views to see if there is anything missing in this function.
   return (
-    views.find(
+    lContainer[DEHYDRATED_VIEWS]?.find(
       (view: any) => view.data[SERIALIZED_DEFER_BLOCK_STATE] === lDetails[DEFER_BLOCK_STATE],
     ) ?? null
   );
@@ -279,41 +271,33 @@ function applyDeferBlockState(
         injector = createDeferBlockInjector(hostLView[INJECTOR], tDetails, providers);
       }
     }
-
     const dehydratedView = findMatchingDehydratedViewForDeferBlock(lContainer, lDetails);
-    // Render either when we don't have dehydrated views at all (e.g. client rendering)
-    // or when dehydrated view is found (in which case we hydrate).
-    // Otherwise, do nothing, since we'd end up erasing SSR'ed content.
-    // TODO(incremental-hydration): Use the util function for checking dehydrated views mentioned above
-    const isClientOnly =
-      lContainer[DEHYDRATED_VIEWS] === null || lContainer[DEHYDRATED_VIEWS].length === 0;
-    if (isClientOnly || dehydratedView) {
-      // Erase dehydrated view info, so that it's not removed later
-      // by post-hydration cleanup process.
-      // TODO(incremental-hydration): we need a better mechanism here.
-      lContainer[DEHYDRATED_VIEWS] = null;
-
-      const embeddedLView = createAndRenderEmbeddedLView(hostLView, activeBlockTNode, null, {
-        injector,
-        dehydratedView,
-      });
-      addLViewToLContainer(
-        lContainer,
-        embeddedLView,
-        viewIndex,
-        shouldAddViewToDom(activeBlockTNode, dehydratedView),
-      );
-      markViewDirty(embeddedLView, NotificationSource.DeferBlockStateUpdate);
-    }
+    // Erase dehydrated view info, so that it's not removed later
+    // by post-hydration cleanup process.
+    lContainer[DEHYDRATED_VIEWS] = null;
+
+    const embeddedLView = createAndRenderEmbeddedLView(hostLView, activeBlockTNode, null, {
+      injector,
+      dehydratedView,
+    });
+    addLViewToLContainer(
+      lContainer,
+      embeddedLView,
+      viewIndex,
+      shouldAddViewToDom(activeBlockTNode, dehydratedView),
+    );
+    markViewDirty(embeddedLView, NotificationSource.DeferBlockStateUpdate);
 
     // TODO(incremental-hydration):
     // - what if we had some views in `lContainer[DEHYDRATED_VIEWS]`, but
     //   we didn't find a view that matches the expected state?
     // - for example, handle a situation when a block was in the "completed" state
     //   on the server, but the loading failing on the client. How do we reconcile and cleanup?
 
-    // TODO(incremental-hydration): should we also invoke if newState === DeferBlockState.Error?
-    if (newState === DeferBlockState.Complete && Array.isArray(lDetails[ON_COMPLETE_FNS])) {
+    if (
+      (newState === DeferBlockState.Complete || newState === DeferBlockState.Error) &&
+      Array.isArray(lDetails[ON_COMPLETE_FNS])
+    ) {
       for (const callback of lDetails[ON_COMPLETE_FNS]) {
         callback();
       }
diff --git a/packages/core/src/hydration/annotate.ts b/packages/core/src/hydration/annotate.ts
@@ -303,7 +303,6 @@ export function annotateForHydration(appRef: ApplicationRef, doc: Document) {
 
   if (deferBlocks.size > 0) {
     const blocks: {[key: string]: SerializedDeferBlock} = {};
-    // TODO(incremental-hydration): we should probably have an object here instead of a Map?
     for (const [id, info] of deferBlocks.entries()) {
       blocks[id] = info;
     }
@@ -447,11 +446,10 @@ function serializeLContainer(
       }
 
       if (!isHydrateNeverBlock) {
-        // TODO(incremental-hydration): avoid copying of an object here
-        serializedView = {
-          ...serializedView,
-          ...serializeLView(lContainer[i] as LView, parentDeferBlockId, context),
-        };
+        Object.assign(
+          serializedView,
+          serializeLView(lContainer[i] as LView, parentDeferBlockId, context),
+        );
       }
     }
 

Original file line number	Diff line number	Diff line change
`@@ -303,7 +303,6 @@ export function annotateForHydration(appRef: ApplicationRef, doc: Document) {`
`303`	`303`
`304`	`304`	`if (deferBlocks.size > 0) {`
`305`	`305`	`const blocks: {[key: string]: SerializedDeferBlock} = {};`
`306`		`- // TODO(incremental-hydration): we should probably have an object here instead of a Map?`
`307`	`306`	`for (const [id, info] of deferBlocks.entries()) {`
`308`	`307`	`blocks[id] = info;`
`309`	`308`	`}`
`@@ -447,11 +446,10 @@ function serializeLContainer(`
`447`	`446`	`}`
`448`	`447`
`449`	`448`	`if (!isHydrateNeverBlock) {`
`450`		`- // TODO(incremental-hydration): avoid copying of an object here`
`451`		`- serializedView = {`
`452`		`- ...serializedView,`
`453`		`- ...serializeLView(lContainer[i] as LView, parentDeferBlockId, context),`
`454`		`- };`
	`449`	`+ Object.assign(`
	`450`	`+ serializedView,`
	`451`	`+ serializeLView(lContainer[i] as LView, parentDeferBlockId, context),`
	`452`	`+ );`
`455`	`453`	`}`
`456`	`454`	`}`
`457`	`455`