From 60ab8f6363b06d58879aa9c87e798a02bbad54e2 Mon Sep 17 00:00:00 2001
From: Jiachi Liu <inbox@huozhi.im>
Date: Thu, 6 Jun 2024 00:48:11 +0200
Subject: [PATCH] Fix loading navigation with metadata and prefetch (#66447)

### What & Why

Fixes NEXT-3498

Fixed loading shows up and disappear during client navigation, when you
defined `prefetch` is enabled and slow `generateMetadata` is defined. In
#64532, where in layout-router, we removed the place of infinite
suspense, adding it back so that the app can still remain suspensy
during navigation.

#### Behavior before fix

Prefetch -> Link Navigation -> Show `loading.js` -> RSC payload fetched
(no page content) -> the page content will display later when the
promise is resolved

#### Behavior after the fix

Prefetch -> Link Navigation -> Show `loading.js` -> RSC payload fetched
-> suspensy page content still triggering `loading.js` -> display the
resolved page content when the promise is resolved

---------

Co-authored-by: Zack Tanner <1939140+ztanner@users.noreply.github.com>
---
 .../src/client/components/layout-router.tsx   |  6 ++---
 .../metadata-await-promise/nested/loading.js  |  6 -----
 .../e2e/app-dir/navigation/navigation.test.ts | 22 ++++++++++++-------
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/packages/next/src/client/components/layout-router.tsx b/packages/next/src/client/components/layout-router.tsx
index a6ea8659406fb..f345381af034e 100644
--- a/packages/next/src/client/components/layout-router.tsx
+++ b/packages/next/src/client/components/layout-router.tsx
@@ -450,10 +450,10 @@ function InnerLayoutRouter({
       // It's important that we mark this as resolved, in case this branch is replayed, we don't want to continously re-apply
       // the patch to the tree.
       childNode.lazyDataResolved = true
-
-      // Suspend infinitely as `changeByServerResponse` will cause a different part of the tree to be rendered.
-      use(unresolvedThenable) as never
     }
+    // Suspend infinitely as `changeByServerResponse` will cause a different part of the tree to be rendered.
+    // A falsey `resolvedRsc` indicates missing data -- we should not commit that branch, and we need to wait for the data to arrive.
+    use(unresolvedThenable) as never
   }
 
   // We use `useDeferredValue` to handle switching between the prefetched and
diff --git a/test/e2e/app-dir/navigation/app/metadata-await-promise/nested/loading.js b/test/e2e/app-dir/navigation/app/metadata-await-promise/nested/loading.js
index 3921b6a73d10f..1fc5ff4960f62 100644
--- a/test/e2e/app-dir/navigation/app/metadata-await-promise/nested/loading.js
+++ b/test/e2e/app-dir/navigation/app/metadata-await-promise/nested/loading.js
@@ -1,9 +1,3 @@
-'use client'
-import { useEffect } from 'react'
-
 export default function Loading() {
-  useEffect(() => {
-    window.shownLoading = true
-  }, [])
   return <div id="loading">Loading</div>
 }
diff --git a/test/e2e/app-dir/navigation/navigation.test.ts b/test/e2e/app-dir/navigation/navigation.test.ts
index 73743aefd24c5..fde14d85ed6f1 100644
--- a/test/e2e/app-dir/navigation/navigation.test.ts
+++ b/test/e2e/app-dir/navigation/navigation.test.ts
@@ -864,16 +864,22 @@ describe('app dir - navigation', () => {
     it('should render the final state of the page with correct metadata', async () => {
       const browser = await next.browser('/metadata-await-promise')
 
-      await browser
-        .elementByCss("[href='/metadata-await-promise/nested']")
-        .click()
+      // dev doesn't trigger the loading boundary as it's not prefetched
+      if (isNextDev) {
+        await browser
+          .elementByCss("[href='/metadata-await-promise/nested']")
+          .click()
+      } else {
+        const loadingText = await browser
+          .elementByCss("[href='/metadata-await-promise/nested']")
+          .click()
+          .waitForElementByCss('#loading')
+          .text()
 
-      await retry(async () => {
-        // dev doesn't trigger the loading boundary as it's not prefetched
-        if (!isNextDev) {
-          expect(await browser.eval(`window.shownLoading`)).toBe(true)
-        }
+        expect(loadingText).toBe('Loading')
+      }
 
+      await retry(async () => {
         expect(await browser.elementById('page-content').text()).toBe('Content')
 
         expect(await browser.elementByCss('title').text()).toBe('Async Title')