bugfix: only replace with full prefetch if existing data was partial (v…

…ercel#70650)

By default we are storing the seeded prefetch entry (from the SSR
render) as an "auto" prefetch. The prefetch cache is used because every
navigation checks against the prefetch cache to determine if it has the
necessary data to render the requested segment(s).

We have a router heuristic that will re-fetch a page that's already in
the prefetch cache if the one that is stored is an "auto" prefetch, and
the newly requested prefetch is a "full" prefetch. This is because the
assumption is the "auto" prefetch would contain partial data
(potentially only `loading.js` data) while the "full" prefetch has
everything, so we'd want to replace the cache entry with the most up to
date information.

Currently when we seed the prefetch cache with the initially SSRed page,
we set a cache status of "auto". This is because a "full" prefetch has
staleTime implications: a full prefetch will be client cached for 5
minutes by default (the `static` staleTime), and since we have no
prefetch intent during SSR, we should fallback to the automatic caching
heuristics to avoid caching a page longer than what was intended.
However, this will trigger the above mentioned logic to switch to a
"full" prefetch if the page you're currently on has a link to itself,
with `prefetch={true}`, resulting in a wasted request.

This refactors the logic responsible for switching a prefetch entry to a
"full" prefetch only once we confirm the response payload from the
existing cache entry wasn't a response that started rendering from the
root. If we started rendering from the root, we know that we have the
full data already, so it'd be wasteful to fetch again.

Fixes vercel#70535

packages/next/src/client/components/router-reducer/prefetch-cache-utils.ts

@@ -191,16 +191,30 @@ export function getOrCreatePrefetchCacheEntry({
       kind === PrefetchKind.FULL
 
     if (switchedToFullPrefetch) {
-      return createLazyPrefetchEntry({
-        tree,
-        url,
-        buildId,
-        nextUrl,
-        prefetchCache,
-        // If we didn't get an explicit prefetch kind, we want to set a temporary kind
-        // rather than assuming the same intent as the previous entry, to be consistent with how we
-        // lazily create prefetch entries when intent is left unspecified.
-        kind: kind ?? PrefetchKind.TEMPORARY,
+      // If we switched to a full prefetch, validate that the existing cache entry contained partial data.
+      // It's possible that the cache entry was seeded with full data but has a cache type of "auto" (ie when cache entries
+      // are seeded but without a prefetch intent)
+      existingCacheEntry.data.then((prefetchResponse) => {
+        const isFullPrefetch =
+          Array.isArray(prefetchResponse.flightData) &&
+          prefetchResponse.flightData.some((flightData) => {
+            // If we started rendering from the root and we returned RSC data (seedData), we already had a full prefetch.
+            return flightData.isRootRender && flightData.seedData !== null
+          })
+
+        if (!isFullPrefetch) {
+          return createLazyPrefetchEntry({
+            tree,
+            url,
+            buildId,
+            nextUrl,
+            prefetchCache,
+            // If we didn't get an explicit prefetch kind, we want to set a temporary kind
+            // rather than assuming the same intent as the previous entry, to be consistent with how we
+            // lazily create prefetch entries when intent is left unspecified.
+            kind: kind ?? PrefetchKind.TEMPORARY,
+          })
+        }
       })
     }
 

test/e2e/app-dir/app-prefetch/app/dynamic-page/page.js

@@ -0,0 +1,21 @@
+import Link from 'next/link'
+
+export const dynamic = 'force-dynamic'
+
+export default async function Page() {
+  return (
+    <>
+      <p id="dynamic-page">Dynamic Page</p>
+      <p>
+        <Link href="/" id="to-home">
+          To home
+        </Link>
+      </p>
+      <p>
+        <Link href="/dynamic-page" prefetch>
+          To Same Page
+        </Link>
+      </p>
+    </>
+  )
+}

test/e2e/app-dir/app-prefetch/app/static-page/page.js

@@ -9,6 +9,11 @@ export default async function Page() {
           To home
         </Link>
       </p>
+      <p>
+        <Link href="/static-page" prefetch>
+          To Same Page
+        </Link>
+      </p>
     </>
   )
 }

test/e2e/app-dir/app-prefetch/prefetching.test.ts

@@ -301,6 +301,70 @@ describe('app dir - prefetching', () => {
     await browser.waitForElementByCss('#prefetch-auto-page-data')
   })
 
+  describe('prefetch cache seeding', () => {
+    it('should not re-fetch the initial static page if the same page is prefetched with prefetch={true}', async () => {
+      const rscRequests = []
+      const browser = await next.browser('/static-page', {
+        beforePageLoad(page: Page) {
+          page.on('request', async (req: Request) => {
+            const url = new URL(req.url())
+            const headers = await req.allHeaders()
+            if (headers['rsc']) {
+              rscRequests.push(url.pathname)
+            }
+          })
+        },
+      })
+
+      expect(
+        await browser.hasElementByCssSelector('[href="/static-page"]')
+      ).toBe(true)
+
+      // sanity check: we should see a prefetch request to the root page
+      await retry(async () => {
+        expect(rscRequests.filter((req) => req === '/').length).toBe(1)
+      })
+
+      // We shouldn't see any requests to the static page since the prefetch cache was seeded as part of the SSR render
+      await retry(async () => {
+        expect(rscRequests.filter((req) => req === '/static-page').length).toBe(
+          0
+        )
+      })
+    })
+
+    it('should not re-fetch the initial dynamic page if the same page is prefetched with prefetch={true}', async () => {
+      const rscRequests = []
+      const browser = await next.browser('/dynamic-page', {
+        beforePageLoad(page: Page) {
+          page.on('request', async (req: Request) => {
+            const url = new URL(req.url())
+            const headers = await req.allHeaders()
+            if (headers['rsc']) {
+              rscRequests.push(url.pathname)
+            }
+          })
+        },
+      })
+
+      expect(
+        await browser.hasElementByCssSelector('[href="/dynamic-page"]')
+      ).toBe(true)
+
+      // sanity check: we should see a prefetch request to the root page
+      await retry(async () => {
+        expect(rscRequests.filter((req) => req === '/').length).toBe(1)
+      })
+
+      // We shouldn't see any requests to the dynamic page since the prefetch cache was seeded as part of the SSR render
+      await retry(async () => {
+        expect(
+          rscRequests.filter((req) => req === '/dynamic-page').length
+        ).toBe(0)
+      })
+    })
+  })
+
   // These tests are skipped when deployed as they rely on runtime logs
   if (!isNextDeploy) {
     describe('dynamic rendering', () => {