From dd6ab93ddbaa4a4a15ff708f9cbcf3f968a0ace3 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)

Fixes NEXT-3498

Fixed loading shows up and disappear during client navigation, when you
defined `prefetch` is enabled and slow `generateMetadata` is defined. In
suspense, adding it back so that the app can still remain suspensy
during navigation.

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

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 1c3698c604967..cfa1f7c298b34 100644
--- a/packages/next/src/client/components/layout-router.tsx
+++ b/packages/next/src/client/components/layout-router.tsx
@@ -439,10 +439,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
   }
 
   // If we get to this point, then we know we have something we can render.
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 6a9a90efbdf41..f756e3a345d53 100644
--- a/test/e2e/app-dir/navigation/navigation.test.ts
+++ b/test/e2e/app-dir/navigation/navigation.test.ts
@@ -841,16 +841,22 @@ createNextDescribe(
       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 + PPR doesn't trigger the loading boundary as it's not prefetched
-          if (!(isNextDev && process.env.__NEXT_EXPERIMENTAL_PPR)) {
-            expect(await browser.eval(`window.shownLoading`)).toBe(true)
-          }
+          expect(loadingText).toBe('Loading')
+        }
 
+        await retry(async () => {
           expect(await browser.elementById('page-content').text()).toBe(
             'Content'
           )