Skip to content

Commit

Permalink
bugfix: only replace with full prefetch if existing data was partial
Browse files Browse the repository at this point in the history
  • Loading branch information
ztanner committed Sep 30, 2024
1 parent 3c7bfff commit f10566a
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
}
})
}

Expand Down
21 changes: 21 additions & 0 deletions test/e2e/app-dir/app-prefetch/app/dynamic-page/page.js
Original file line number Diff line number Diff line change
@@ -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>
</>
)
}
5 changes: 5 additions & 0 deletions test/e2e/app-dir/app-prefetch/app/static-page/page.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ export default async function Page() {
To home
</Link>
</p>
<p>
<Link href="/static-page" prefetch>
To Same Page
</Link>
</p>
</>
)
}
64 changes: 64 additions & 0 deletions test/e2e/app-dir/app-prefetch/prefetching.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 should see a prefetch request to the dynamic page since the dynamic staletime is 0 by default
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', () => {
Expand Down

0 comments on commit f10566a

Please sign in to comment.