fix(core): support swapping hydrated views in @for loops (#53274)

AndrewKushnir · dylhunn · commit 8e366e891143 · 2023-12-01T10:35:37.000-08:00
This commit fixes an issue where swapping hydrated views was not possible in the new control flow repeater. The problem was caused by the fact that an internal representation of a view had no indication that hydration is completed and further detaching/attaching should work in a regular (non-hydration) mode. This commit adds a logic that resets a pointer to a dehydrated content and we use this as an indication that the view is swtiched to a regular mode. Resolves #53163. PR Close #53274
diff --git a/packages/core/src/hydration/interfaces.ts b/packages/core/src/hydration/interfaces.ts
@@ -139,6 +139,11 @@ export interface DehydratedView {
   /**
    * A reference to the first child in a DOM segment associated
    * with a given hydration boundary.
+   *
+   * Once a view becomes hydrated, the value is set to `null`, which
+   * indicates that further detaching/attaching view actions should result
+   * in invoking corresponding DOM actions (attaching DOM nodes action is
+   * skipped when we hydrate, since nodes are already in the DOM).
    */
   firstChild: RNode|null;
 
diff --git a/packages/core/src/render3/view_manipulation.ts b/packages/core/src/render3/view_manipulation.ts
@@ -17,7 +17,7 @@ import {createLView} from './instructions/shared';
 import {CONTAINER_HEADER_OFFSET, LContainer, NATIVE} from './interfaces/container';
 import {TNode} from './interfaces/node';
 import {RComment, RElement} from './interfaces/renderer_dom';
-import {DECLARATION_LCONTAINER, FLAGS, LView, LViewFlags, QUERIES, RENDERER, T_HOST, TVIEW} from './interfaces/view';
+import {DECLARATION_LCONTAINER, FLAGS, HYDRATION, LView, LViewFlags, QUERIES, RENDERER, T_HOST, TVIEW} from './interfaces/view';
 import {addViewToDOM, destroyLView, detachView, getBeforeNodeForView, insertView, nativeParentNode} from './node_manipulation';
 
 export function createAndRenderEmbeddedLView<T>(
@@ -70,17 +70,18 @@ export function getLViewFromLContainer<T>(lContainer: LContainer, index: number)
  */
 export function shouldAddViewToDom(
     tNode: TNode, dehydratedView?: DehydratedContainerView|null): boolean {
-  return !dehydratedView || hasInSkipHydrationBlockFlag(tNode);
+  return !dehydratedView || dehydratedView.firstChild === null ||
+      hasInSkipHydrationBlockFlag(tNode);
 }
 
 export function addLViewToLContainer(
     lContainer: LContainer, lView: LView<unknown>, index: number, addToDOM = true): void {
   const tView = lView[TVIEW];
 
-  // insert to the view tree so the new view can be change-detected
+  // Insert into the view tree so the new view can be change-detected
   insertView(tView, lView, lContainer, index);
 
-  // insert to the view to the DOM tree
+  // Insert elements that belong to this view into the DOM tree
   if (addToDOM) {
     const beforeNode = getBeforeNodeForView(index, lContainer);
     const renderer = lView[RENDERER];
@@ -89,6 +90,14 @@ export function addLViewToLContainer(
       addViewToDOM(tView, lContainer[T_HOST], renderer, lView, parentRNode, beforeNode);
     }
   }
+
+  // When in hydration mode, reset the pointer to the first child in
+  // the dehydrated view. This indicates that the view was hydrated and
+  // further attaching/detaching should work with this view as normal.
+  const hydrationInfo = lView[HYDRATION];
+  if (hydrationInfo !== null && hydrationInfo.firstChild !== null) {
+    hydrationInfo.firstChild = null;
+  }
 }
 
 export function removeLViewFromLContainer(lContainer: LContainer, index: number): LView<unknown>|
diff --git a/packages/platform-server/test/hydration_spec.ts b/packages/platform-server/test/hydration_spec.ts
@@ -467,6 +467,65 @@ describe('platform-server hydration integration', () => {
           verifyClientAndSSRContentsMatch(ssrContents, clientRootNode);
         });
 
+        it('should hydrate root components with empty templates', async () => {
+          @Component({
+            standalone: true,
+            selector: 'app',
+            template: '',
+          })
+          class SimpleComponent {
+          }
+
+          const html = await ssr(SimpleComponent);
+          const ssrContents = getAppContents(html);
+
+          expect(ssrContents).toContain(`<app ${NGH_ATTR_NAME}`);
+
+          resetTViewsFor(SimpleComponent);
+
+          const appRef = await hydrate(html, SimpleComponent);
+          const compRef = getComponentRef<SimpleComponent>(appRef);
+          appRef.tick();
+
+          const clientRootNode = compRef.location.nativeElement;
+          verifyAllNodesClaimedForHydration(clientRootNode);
+          verifyClientAndSSRContentsMatch(ssrContents, clientRootNode);
+        });
+
+        it('should hydrate child components with empty templates', async () => {
+          @Component({
+            standalone: true,
+            selector: 'child',
+            template: '',
+          })
+          class ChildComponent {
+          }
+
+          @Component({
+            standalone: true,
+            imports: [ChildComponent],
+            selector: 'app',
+            template: '<child />',
+          })
+          class SimpleComponent {
+          }
+
+          const html = await ssr(SimpleComponent);
+          const ssrContents = getAppContents(html);
+
+          expect(ssrContents).toContain(`<app ${NGH_ATTR_NAME}`);
+
+          resetTViewsFor(SimpleComponent, ChildComponent);
+
+          const appRef = await hydrate(html, SimpleComponent);
+          const compRef = getComponentRef<SimpleComponent>(appRef);
+          appRef.tick();
+
+          const clientRootNode = compRef.location.nativeElement;
+          verifyAllNodesClaimedForHydration(clientRootNode);
+          verifyClientAndSSRContentsMatch(ssrContents, clientRootNode);
+        });
+
         it('should support a single text interpolation', async () => {
           @Component({
             standalone: true,
@@ -6469,6 +6528,54 @@ describe('platform-server hydration integration', () => {
                [4, 5].map(id => compRef.location.nativeElement.querySelector(`[id=${id}]`));
            verifyAllNodesClaimedForHydration(clientRootNode, Array.from(clientRenderedItems));
          });
+
+      it('should handle a reconciliation with swaps', async () => {
+        @Component({
+          selector: 'app',
+          standalone: true,
+          template: `
+                @for(item of items; track item) {
+                  <div>{{ item }}</div>
+                }
+              `,
+        })
+        class SimpleComponent {
+          items = ['a', 'b', 'c'];
+
+          swap() {
+            // Reshuffling of the array will result in
+            // "swap" operations in repeater.
+            this.items = ['b', 'c', 'a'];
+          }
+        }
+
+        const html = await ssr(SimpleComponent);
+        const ssrContents = getAppContents(html);
+
+        expect(ssrContents).toContain(`<app ${NGH_ATTR_NAME}`);
+
+        resetTViewsFor(SimpleComponent);
+
+        expect(ssrContents).toContain('a');
+        expect(ssrContents).toContain('b');
+        expect(ssrContents).toContain('c');
+
+        const appRef = await hydrate(html, SimpleComponent);
+        const compRef = getComponentRef<SimpleComponent>(appRef);
+        appRef.tick();
+
+        await whenStable(appRef);
+
+        const root: HTMLElement = compRef.location.nativeElement;
+        const divs = root.querySelectorAll('div');
+        expect(divs.length).toBe(3);
+
+        compRef.instance.swap();
+        compRef.changeDetectorRef.detectChanges();
+
+        const divsAfterSwap = root.querySelectorAll('div');
+        expect(divsAfterSwap.length).toBe(3);
+      });
     });
 
     describe('Router', () => {
