diff --git a/packages/next/src/server/app-render/dynamic-rendering.ts b/packages/next/src/server/app-render/dynamic-rendering.ts index e27f807c16ff3..c818783184fce 100644 --- a/packages/next/src/server/app-render/dynamic-rendering.ts +++ b/packages/next/src/server/app-render/dynamic-rendering.ts @@ -180,9 +180,11 @@ export function trackDynamicFetch( store: StaticGenerationStore, expression: string ) { - if (store.prerenderState) { - postponeWithTracking(store.prerenderState, expression, store.urlPathname) - } + // If we aren't in a prerender, or we're in an unstable cache callback, we + // don't need to postpone. + if (!store.prerenderState || store.isUnstableCacheCallback) return + + postponeWithTracking(store.prerenderState, expression, store.urlPathname) } function postponeWithTracking( diff --git a/packages/next/src/server/lib/patch-fetch.ts b/packages/next/src/server/lib/patch-fetch.ts index 2e7687d728714..ac95be913d426 100644 --- a/packages/next/src/server/lib/patch-fetch.ts +++ b/packages/next/src/server/lib/patch-fetch.ts @@ -259,14 +259,22 @@ function createPatchedFetcher( }, async () => { // If this is an internal fetch, we should not do any special treatment. - if (isInternal) return originFetch(input, init) + if (isInternal) { + return originFetch(input, init) + } const staticGenerationStore = staticGenerationAsyncStorage.getStore() // If the staticGenerationStore is not available, we can't do any // special treatment of fetch, therefore fallback to the original // fetch implementation. - if (!staticGenerationStore || staticGenerationStore.isDraftMode) { + if (!staticGenerationStore) { + return originFetch(input, init) + } + + // We should also fallback to the original fetch implementation if we + // are in draft mode, it does not constitute a static generation. + if (staticGenerationStore.isDraftMode) { return originFetch(input, init) } @@ -686,14 +694,18 @@ function createPatchedFetcher( // If enabled, we should bail out of static generation. trackDynamicFetch(staticGenerationStore, dynamicUsageReason) - // PPR is not enabled, or React postpone is not available, we - // should set the revalidate to 0. - staticGenerationStore.revalidate = 0 + // If partial prerendering is not enabled, then we should throw an + // error to indicate that this fetch is dynamic. + if (!staticGenerationStore.prerenderState) { + // PPR is not enabled, or React postpone is not available, we + // should set the revalidate to 0. + staticGenerationStore.revalidate = 0 - const err = new DynamicServerError(dynamicUsageReason) - staticGenerationStore.dynamicUsageErr = err - staticGenerationStore.dynamicUsageDescription = dynamicUsageReason - throw err + const err = new DynamicServerError(dynamicUsageReason) + staticGenerationStore.dynamicUsageErr = err + staticGenerationStore.dynamicUsageDescription = dynamicUsageReason + throw err + } } const hasNextConfig = 'next' in init @@ -718,10 +730,15 @@ function createPatchedFetcher( // If enabled, we should bail out of static generation. trackDynamicFetch(staticGenerationStore, dynamicUsageReason) - const err = new DynamicServerError(dynamicUsageReason) - staticGenerationStore.dynamicUsageErr = err - staticGenerationStore.dynamicUsageDescription = dynamicUsageReason - throw err + // If partial prerendering is not enabled, then we should throw an + // error to indicate that this fetch is dynamic. + if (!staticGenerationStore.prerenderState) { + const err = new DynamicServerError(dynamicUsageReason) + staticGenerationStore.dynamicUsageErr = err + staticGenerationStore.dynamicUsageDescription = + dynamicUsageReason + throw err + } } if (!staticGenerationStore.forceStatic || next.revalidate !== 0) { diff --git a/test/e2e/app-dir/app-static/app-static.test.ts b/test/e2e/app-dir/app-static/app-static.test.ts index caa46d046f81c..5dc84c72dd8cb 100644 --- a/test/e2e/app-dir/app-static/app-static.test.ts +++ b/test/e2e/app-dir/app-static/app-static.test.ts @@ -1,7 +1,7 @@ import globOrig from 'glob' import cheerio from 'cheerio' -import { promisify } from 'util' -import { join } from 'path' +import { promisify } from 'node:util' +import { join } from 'node:path' import { nextTestSetup } from 'e2e-utils' import { check, @@ -211,24 +211,22 @@ describe('app-dir static/dynamic handling', () => { if (isApi) { prevData = await res.json() } else { - const initialHtml = await res.text() - const initial$ = isApi ? undefined : cheerio.load(initialHtml) - prevData = JSON.parse(initial$('#props').text()) + const $ = isApi ? undefined : cheerio.load(await res.text()) + prevData = JSON.parse($('#props').text()) } expect(prevData.data.random).toBeTruthy() - await check(async () => { + await retry(async () => { res = await next.fetch(pathname) expect(res.status).toBe(200) - let curData + let curData if (isApi) { curData = await res.json() } else { - const curHtml = await res.text() - const cur$ = cheerio.load(curHtml) - curData = JSON.parse(cur$('#props').text()) + const $ = cheerio.load(await res.text()) + curData = JSON.parse($('#props').text()) } try { @@ -237,8 +235,7 @@ describe('app-dir static/dynamic handling', () => { } finally { prevData = curData } - return 'success' - }, 'success') + }) }) it('should not have cache tags header for non-minimal mode', async () => { @@ -2323,7 +2320,7 @@ describe('app-dir static/dynamic handling', () => { /partial-gen-params fetch ([\d]{1,})/ ) - if (matches[1]) { + if (matches?.[1]) { langFetchSlug = matches[1] slugFetchSlug = langFetchSlug } diff --git a/test/e2e/app-dir/ppr-unstable-cache/app/layout.jsx b/test/e2e/app-dir/ppr-unstable-cache/app/layout.jsx new file mode 100644 index 0000000000000..750eb927b1980 --- /dev/null +++ b/test/e2e/app-dir/ppr-unstable-cache/app/layout.jsx @@ -0,0 +1,7 @@ +export default function Layout({ children }) { + return ( + + {children} + + ) +} diff --git a/test/e2e/app-dir/ppr-unstable-cache/app/page.jsx b/test/e2e/app-dir/ppr-unstable-cache/app/page.jsx new file mode 100644 index 0000000000000..eacbb219718f2 --- /dev/null +++ b/test/e2e/app-dir/ppr-unstable-cache/app/page.jsx @@ -0,0 +1,37 @@ +import { unstable_cache } from 'next/cache' + +const getData = unstable_cache( + async () => { + const noStore = await fetch( + process.env.TEST_DATA_SERVER + '?cache=no-store', + { method: 'GET', cache: 'no-store' } + ).then((res) => res.text()) + + const forceCache = await fetch( + process.env.TEST_DATA_SERVER + '?cache=force-cache', + { method: 'GET', cache: 'force-cache' } + ).then((res) => res.text()) + + return JSON.stringify( + { + random: Math.floor(Math.random() * 1000).toString(), + data: { + forceCache, + noStore, + }, + }, + null, + 2 + ) + }, + undefined, + { + tags: ['unstable-cache-fetch'], + } +) + +export default async function Page() { + const data = await getData() + + return
{data}
+} diff --git a/test/e2e/app-dir/ppr-unstable-cache/app/revalidate-tag/route.js b/test/e2e/app-dir/ppr-unstable-cache/app/revalidate-tag/route.js new file mode 100644 index 0000000000000..f50aaa8643f39 --- /dev/null +++ b/test/e2e/app-dir/ppr-unstable-cache/app/revalidate-tag/route.js @@ -0,0 +1,6 @@ +import { revalidateTag } from 'next/cache' + +export const POST = async () => { + revalidateTag('unstable-cache-fetch') + return new Response('OK', { status: 200 }) +} diff --git a/test/e2e/app-dir/ppr-unstable-cache/next.config.js b/test/e2e/app-dir/ppr-unstable-cache/next.config.js new file mode 100644 index 0000000000000..6013aed786290 --- /dev/null +++ b/test/e2e/app-dir/ppr-unstable-cache/next.config.js @@ -0,0 +1,5 @@ +module.exports = { + experimental: { + ppr: true, + }, +} diff --git a/test/e2e/app-dir/ppr-unstable-cache/ppr-unstable-cache.test.ts b/test/e2e/app-dir/ppr-unstable-cache/ppr-unstable-cache.test.ts new file mode 100644 index 0000000000000..f4b5bb01636df --- /dev/null +++ b/test/e2e/app-dir/ppr-unstable-cache/ppr-unstable-cache.test.ts @@ -0,0 +1,103 @@ +import { NextInstance, createNext, isNextDeploy, isNextDev } from 'e2e-utils' +import { findPort } from 'next-test-utils' +import http from 'node:http' + +describe('ppr-unstable-cache', () => { + if (isNextDeploy) { + it.skip('should not run in deploy mode', () => {}) + return + } + + if (isNextDev) { + it.skip('should not run in dev mode', () => {}) + return + } + + let next: NextInstance | null = null + let server: http.Server | null = null + afterEach(async () => { + if (next) { + await next.destroy() + next = null + } + + if (server) { + await server.close() + server = null + } + }) + + it('should not cache inner fetch calls', async () => { + let generations: string[] = [] + server = http.createServer(async (req, res) => { + try { + if (!req.url) throw new Error('No URL') + + const cache = new URL(req.url, 'http://n').searchParams.get('cache') + if (!cache) throw new Error('No cache key') + + const random = Math.floor(Math.random() * 1000).toString() + const data = cache + ':' + random + generations.push(data) + res.end(data) + } catch (err) { + res.statusCode = 500 + res.end(err.message) + } + }) + const port = await findPort() + server.listen(port) + + next = await createNext({ + files: __dirname, + env: { TEST_DATA_SERVER: `http://localhost:${port}/` }, + }) + + expect(generations).toHaveLength(3) + + const first = await next + .render$('/') + .then(($) => JSON.parse($('#data').text())) + + expect(generations).toHaveLength(3) + + expect(first.data.forceCache).toBeOneOf(generations) + expect(first.data.noStore).toBeOneOf(generations) + + // Try a few more times, we should always get the same result. + for (let i = 0; i < 3; i++) { + const again = await next + .render$('/') + .then(($) => JSON.parse($('#data').text())) + + expect(generations).toHaveLength(3) + expect(first).toEqual(again) + } + + // Revalidate the tag associated with the `unstable_cache` call. + const revalidate = await next.fetch('/revalidate-tag', { method: 'POST' }) + expect(revalidate.status).toBe(200) + await revalidate.text() + + const revalidated = await next + .render$('/') + .then(($) => JSON.parse($('#data').text())) + + // Expect that the `cache: no-store` value has been updated, but not + // the `cache: force-cache` value. + expect(generations).toHaveLength(5) + + // We know now that the generations have been updated, so let's try to + // validate the value. We don't need to do this within the retry. + expect(revalidated.random).not.toEqual(first.random) + expect(revalidated.data.forceCache).toBeOneOf(generations.slice(0, 3)) + expect(revalidated.data.noStore).toBeOneOf(generations.slice(3)) + expect(revalidated).not.toEqual(first) + + // Ensure that the `force-cache` value has not been updated, and only called + // once. + expect(generations.filter((g) => g.startsWith('force-cache'))).toHaveLength( + 1 + ) + }) +})