From 1b7024fc76280ad3538cd3f10a4303da87a64899 Mon Sep 17 00:00:00 2001
From: Zack Tanner <1939140+ztanner@users.noreply.github.com>
Date: Mon, 30 Sep 2024 15:28:02 -0700
Subject: [PATCH] bugfix: only replace with full prefetch if existing data was
partial
---
.../router-reducer/prefetch-cache-utils.ts | 34 +++++++---
.../app-prefetch/app/dynamic-page/page.js | 21 ++++++
.../app-prefetch/app/static-page/page.js | 5 ++
.../app-dir/app-prefetch/prefetching.test.ts | 64 +++++++++++++++++++
4 files changed, 114 insertions(+), 10 deletions(-)
create mode 100644 test/e2e/app-dir/app-prefetch/app/dynamic-page/page.js
diff --git a/packages/next/src/client/components/router-reducer/prefetch-cache-utils.ts b/packages/next/src/client/components/router-reducer/prefetch-cache-utils.ts
index 75fb66995d37d..621b74dc6db4f 100644
--- a/packages/next/src/client/components/router-reducer/prefetch-cache-utils.ts
+++ b/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,
+ })
+ }
})
}
diff --git a/test/e2e/app-dir/app-prefetch/app/dynamic-page/page.js b/test/e2e/app-dir/app-prefetch/app/dynamic-page/page.js
new file mode 100644
index 0000000000000..40b0d6f958a74
--- /dev/null
+++ b/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 (
+ <>
+
Dynamic Page
+
+
+ To home
+
+
+
+
+ To Same Page
+
+
+
+ )
+}
diff --git a/test/e2e/app-dir/app-prefetch/app/static-page/page.js b/test/e2e/app-dir/app-prefetch/app/static-page/page.js
index 6331a9319b56f..73fda2a3aa729 100644
--- a/test/e2e/app-dir/app-prefetch/app/static-page/page.js
+++ b/test/e2e/app-dir/app-prefetch/app/static-page/page.js
@@ -9,6 +9,11 @@ export default async function Page() {
To home
+
+
+ To Same Page
+
+
)
}
diff --git a/test/e2e/app-dir/app-prefetch/prefetching.test.ts b/test/e2e/app-dir/app-prefetch/prefetching.test.ts
index f3ec1101cbf26..4858f76ff0ade 100644
--- a/test/e2e/app-dir/app-prefetch/prefetching.test.ts
+++ b/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', () => {