diff --git a/packages/next/src/server/future/route-modules/app-route/module.ts b/packages/next/src/server/future/route-modules/app-route/module.ts index fd0db1a8e41065..9cd98219157dd6 100644 --- a/packages/next/src/server/future/route-modules/app-route/module.ts +++ b/packages/next/src/server/future/route-modules/app-route/module.ts @@ -43,7 +43,10 @@ import * as serverHooks from '../../../../client/components/hooks-server-context import { DynamicServerError } from '../../../../client/components/hooks-server-context' import { requestAsyncStorage } from '../../../../client/components/request-async-storage.external' -import { staticGenerationAsyncStorage } from '../../../../client/components/static-generation-async-storage.external' +import { + staticGenerationAsyncStorage, + type StaticGenerationStore, +} from '../../../../client/components/static-generation-async-storage.external' import { actionAsyncStorage } from '../../../../client/components/action-async-storage.external' import * as sharedModules from './shared-modules' import { getIsServerAction } from '../../../lib/server-action-request-meta' @@ -51,6 +54,7 @@ import { RequestCookies } from 'next/dist/compiled/@edge-runtime/cookies' import { cleanURL } from './helpers/clean-url' import { StaticGenBailoutError } from '../../../../client/components/static-generation-bailout' import { isStaticGenEnabled } from './helpers/is-static-gen-enabled' +import { trackDynamicDataAccessed } from '../../../app-render/dynamic-rendering' /** * The AppRouteModule is the type of the module exported by the bundled App @@ -317,53 +321,46 @@ export class AppRouteRouteModule extends RouteModule< let request = rawRequest // Update the static generation store based on the dynamic property. - if (isStaticGeneration) { - switch (this.dynamic) { - case 'force-dynamic': { - // Routes of generated paths should be dynamic - staticGenerationStore.forceDynamic = true - break - } - case 'force-static': - // The dynamic property is set to force-static, so we should - // force the page to be static. - staticGenerationStore.forceStatic = true - // We also Proxy the request to replace dynamic data on the request - // with empty stubs to allow for safely executing as static - request = new Proxy( - rawRequest, - forceStaticRequestHandlers - ) - break - case 'error': - // The dynamic property is set to error, so we should throw an - // error if the page is being statically generated. - staticGenerationStore.dynamicShouldError = true - if (isStaticGeneration) - request = new Proxy( - rawRequest, - requireStaticRequestHandlers - ) - break - default: - // When we are statically generating a route we want to bail out if anything dynamic - // is accessed. We only create this proxy in the staticGenerationCase because it is overhead - // for dynamic runtime executions - request = new Proxy( - rawRequest, - staticGenerationRequestHandlers - ) + switch (this.dynamic) { + case 'force-dynamic': { + // Routes of generated paths should be dynamic + staticGenerationStore.forceDynamic = true + break } - } else { - // Generally if we are in a dynamic render we don't have to modify much however for - // force-static specifically we ensure the dynamic and static behavior is consistent - // by proxying the request in the same way in both cases - if (this.dynamic === 'force-static') { + case 'force-static': // The dynamic property is set to force-static, so we should // force the page to be static. staticGenerationStore.forceStatic = true + // We also Proxy the request to replace dynamic data on the request + // with empty stubs to allow for safely executing as static request = new Proxy(rawRequest, forceStaticRequestHandlers) - } + break + case 'error': + // The dynamic property is set to error, so we should throw an + // error if the page is being statically generated. + staticGenerationStore.dynamicShouldError = true + if (isStaticGeneration) + request = new Proxy( + rawRequest, + requireStaticRequestHandlers + ) + break + default: + // We proxy `NextRequest` to track dynamic access, and potentially bail out of static generation + request = proxyNextRequest( + rawRequest, + staticGenerationStore + ) + } + + // Generally if we are in a dynamic render we don't have to modify much however for + // force-static specifically we ensure the dynamic and static behavior is consistent + // by proxying the request in the same way in both cases + if (this.dynamic === 'force-static') { + // The dynamic property is set to force-static, so we should + // force the page to be static. + staticGenerationStore.forceStatic = true + request = new Proxy(rawRequest, forceStaticRequestHandlers) } // If the static generation store does not have a revalidate value @@ -672,92 +669,113 @@ const forceStaticNextUrlHandlers = { }, } -const staticGenerationRequestHandlers = { - get( - target: NextRequest & RequestSymbolTarget, - prop: string | symbol, - receiver: any - ): unknown { - switch (prop) { - case 'nextUrl': - return ( - target[nextURLSymbol] || - (target[nextURLSymbol] = new Proxy( - target.nextUrl, - staticGenerationNextUrlHandlers - )) - ) - case 'headers': - case 'cookies': - case 'url': - case 'body': - case 'blob': - case 'json': - case 'text': - case 'arrayBuffer': - case 'formData': - throw new DynamicServerError( - `Route ${target.nextUrl.pathname} couldn't be rendered statically because it accessed \`request.${prop}\`. See more info here: https://nextjs.org/docs/messages/dynamic-server-error` - ) - case 'clone': - return ( - target[requestCloneSymbol] || - (target[requestCloneSymbol] = () => - new Proxy( - // This is vaguely unsafe but it's required since NextRequest does not implement - // clone. The reason we might expect this to work in this context is the Proxy will - // respond with static-amenable values anyway somewhat restoring the interface. - // @TODO we need to rethink NextRequest and NextURL because they are not sufficientlly - // sophisticated to adequately represent themselves in all contexts. A better approach is - // to probably embed the static generation logic into the class itself removing the need - // for any kind of proxying - target.clone() as NextRequest, - staticGenerationRequestHandlers - )) - ) - default: - const result = Reflect.get(target, prop, receiver) - if (typeof result === 'function') { - return result.bind(target) +function proxyNextRequest( + request: NextRequest, + staticGenerationStore: StaticGenerationStore +) { + const nextUrlHandlers = { + get( + target: NextURL & UrlSymbolTarget, + prop: string | symbol, + receiver: any + ): unknown { + switch (prop) { + case 'search': + case 'searchParams': + case 'url': + case 'href': + case 'toJSON': + case 'toString': + case 'origin': { + trackDynamicDataAccessed(staticGenerationStore, `nextUrl.${prop}`) + const result = Reflect.get(target, prop, receiver) + if (typeof result === 'function') { + return result.bind(target) + } + return result } - return result - } - }, - // We don't need to proxy set because all the properties we proxy are ready only - // and will be ignored -} + case 'clone': + return ( + target[urlCloneSymbol] || + (target[urlCloneSymbol] = () => + new Proxy(target.clone(), nextUrlHandlers)) + ) + default: + const result = Reflect.get(target, prop, receiver) + if (typeof result === 'function') { + return result.bind(target) + } + return result + } + }, + } -const staticGenerationNextUrlHandlers = { - get( - target: NextURL & UrlSymbolTarget, - prop: string | symbol, - receiver: any - ): unknown { - switch (prop) { - case 'search': - case 'searchParams': - case 'url': - case 'href': - case 'toJSON': - case 'toString': - case 'origin': - throw new DynamicServerError( - `Route ${target.pathname} couldn't be rendered statically because it accessed \`nextUrl.${prop}\`. See more info here: https://nextjs.org/docs/messages/dynamic-server-error` - ) - case 'clone': - return ( - target[urlCloneSymbol] || - (target[urlCloneSymbol] = () => - new Proxy(target.clone(), staticGenerationNextUrlHandlers)) - ) - default: - const result = Reflect.get(target, prop, receiver) - if (typeof result === 'function') { - return result.bind(target) + const nextRequestHandlers = { + get( + target: NextRequest & RequestSymbolTarget, + prop: string | symbol, + receiver: any + ): unknown { + try { + switch (prop) { + case 'nextUrl': + return ( + target[nextURLSymbol] || + (target[nextURLSymbol] = new Proxy( + target.nextUrl, + nextUrlHandlers + )) + ) + case 'headers': + case 'cookies': + case 'url': + case 'body': + case 'blob': + case 'json': + case 'text': + case 'arrayBuffer': + case 'formData': { + trackDynamicDataAccessed(staticGenerationStore, prop) + + const result = Reflect.get(target, prop, receiver) + if (typeof result === 'function') { + return result.bind(target) + } + return result + } + case 'clone': + return ( + target[requestCloneSymbol] || + (target[requestCloneSymbol] = () => + new Proxy( + // This is vaguely unsafe but it's required since NextRequest does not implement + // clone. The reason we might expect this to work in this context is the Proxy will + // respond with static-amenable values anyway somewhat restoring the interface. + // @TODO we need to rethink NextRequest and NextURL because they are not sufficientlly + // sophisticated to adequately represent themselves in all contexts. A better approach is + // to probably embed the static generation logic into the class itself removing the need + // for any kind of proxying + target.clone() as NextRequest, + nextRequestHandlers + )) + ) + default: + const result = Reflect.get(target, prop, receiver) + if (typeof result === 'function') { + return result.bind(target) + } + return result } - return result - } - }, + } catch (err) { + console.log('Error in proxy handler', err) + throw err + } + }, + // We don't need to proxy set because all the properties we proxy are ready only + // and will be ignored + } + + return new Proxy(request, nextRequestHandlers) } const requireStaticRequestHandlers = { diff --git a/packages/next/src/server/lib/patch-fetch.ts b/packages/next/src/server/lib/patch-fetch.ts index 82a81cc9ef7868..e7d51f34f208bf 100644 --- a/packages/next/src/server/lib/patch-fetch.ts +++ b/packages/next/src/server/lib/patch-fetch.ts @@ -374,11 +374,15 @@ function createPatchedFetcher( getRequestMeta('method')?.toLowerCase() || 'get' ) - // if there are authorized headers or a POST method and - // dynamic data usage was present above the tree we bail - // e.g. if cookies() is used before an authed/POST fetch - // or no user provided fetch cache config or revalidate - // is provided we don't cache + /** + * We automatically disable fetch caching under the following conditions: + * - Fetch cache configs are not set. Specifically: + * - A page fetch cache mode is not set (export const fetchCache=...) + * - A fetch cache mode is not set in the fetch call (fetch(url, { cache: ... })) + * - A fetch revalidate value is not set in the fetch call (fetch(url, { revalidate: ... })) + * - OR the fetch comes after a configuration that triggered dynamic rendering (e.g., reading cookies()) + * and the fetch was considered uncacheable (e.g., POST method or has authorization headers) + */ const autoNoCache = // this condition is hit for null/undefined // eslint-disable-next-line eqeqeq 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 16388306fadf3c..ef7a78f27f3d55 100644 --- a/test/e2e/app-dir/app-static/app-static.test.ts +++ b/test/e2e/app-dir/app-static/app-static.test.ts @@ -154,6 +154,44 @@ describe('app-dir static/dynamic handling', () => { expect($2('#data').text()).toBeTruthy() expect($2('#data').text()).not.toBe(initData) }) + + // Check route handlers as well + const initFetchData = await ( + await next.fetch('/force-dynamic-fetch-cache/no-fetch-cache/route') + ).json() + + await retry(async () => { + const newFetchData = await ( + await next.fetch('/force-dynamic-fetch-cache/no-fetch-cache/route') + ).json() + expect(newFetchData).toBeTruthy() + expect(newFetchData).not.toEqual(initFetchData) + }) + }) + + it('force-dynamic should supercede a "default" cache value', async () => { + const $ = await next.render$('/force-dynamic-fetch-cache/default-cache') + const initData = $('#data').text() + await retry(async () => { + const $2 = await next.render$( + '/force-dynamic-fetch-cache/default-cache' + ) + expect($2('#data').text()).toBeTruthy() + expect($2('#data').text()).not.toBe(initData) + }) + + // Check route handlers as well + const initFetchData = await ( + await next.fetch('/force-dynamic-fetch-cache/default-cache/route') + ).json() + + await retry(async () => { + const newFetchData = await ( + await next.fetch('/force-dynamic-fetch-cache/default-cache/route') + ).json() + expect(newFetchData).toBeTruthy() + expect(newFetchData).not.toEqual(initFetchData) + }) }) it('fetchCache config should supercede dynamic config when force-dynamic is used', async () => { @@ -168,6 +206,42 @@ describe('app-dir static/dynamic handling', () => { expect($2('#data').text()).toBeTruthy() expect($2('#data').text()).toBe(initData) }) + + // Check route handlers as well + const initFetchData = await ( + await next.fetch('/force-dynamic-fetch-cache/with-fetch-cache/route') + ).json() + + await retry(async () => { + const newFetchData = await ( + await next.fetch('/force-dynamic-fetch-cache/with-fetch-cache/route') + ).json() + expect(newFetchData).toBeTruthy() + expect(newFetchData).toEqual(initFetchData) + }) + }) + + it('fetch `cache` should supercede dynamic config when force-dynamic is used', async () => { + const $ = await next.render$('/force-dynamic-fetch-cache/force-cache') + const initData = $('#data').text() + await retry(async () => { + const $2 = await next.render$('/force-dynamic-fetch-cache/force-cache') + expect($2('#data').text()).toBeTruthy() + expect($2('#data').text()).toBe(initData) + }) + + // Check route handlers as well + const initFetchData = await ( + await next.fetch('/force-dynamic-fetch-cache/force-cache/route') + ).json() + + await retry(async () => { + const newFetchData = await ( + await next.fetch('/force-dynamic-fetch-cache/force-cache/route') + ).json() + expect(newFetchData).toBeTruthy() + expect(newFetchData).toEqual(initFetchData) + }) }) if (!process.env.CUSTOM_CACHE_HANDLER) { @@ -725,10 +799,18 @@ describe('app-dir static/dynamic handling', () => { "force-cache/page_client-reference-manifest.js", "force-dynamic-catch-all/[slug]/[[...id]]/page.js", "force-dynamic-catch-all/[slug]/[[...id]]/page_client-reference-manifest.js", + "force-dynamic-fetch-cache/default-cache/page.js", + "force-dynamic-fetch-cache/default-cache/page_client-reference-manifest.js", + "force-dynamic-fetch-cache/default-cache/route/route.js", + "force-dynamic-fetch-cache/force-cache/page.js", + "force-dynamic-fetch-cache/force-cache/page_client-reference-manifest.js", + "force-dynamic-fetch-cache/force-cache/route/route.js", "force-dynamic-fetch-cache/no-fetch-cache/page.js", "force-dynamic-fetch-cache/no-fetch-cache/page_client-reference-manifest.js", + "force-dynamic-fetch-cache/no-fetch-cache/route/route.js", "force-dynamic-fetch-cache/with-fetch-cache/page.js", "force-dynamic-fetch-cache/with-fetch-cache/page_client-reference-manifest.js", + "force-dynamic-fetch-cache/with-fetch-cache/route/route.js", "force-dynamic-no-prerender/[id]/page.js", "force-dynamic-no-prerender/[id]/page_client-reference-manifest.js", "force-dynamic-prerender/[slug]/page.js", @@ -896,6 +978,8 @@ describe('app-dir static/dynamic handling', () => { "variable-revalidate/authorization.rsc", "variable-revalidate/authorization/page.js", "variable-revalidate/authorization/page_client-reference-manifest.js", + "variable-revalidate/authorization/route-cookies/route.js", + "variable-revalidate/authorization/route-request/route.js", "variable-revalidate/cookie.html", "variable-revalidate/cookie.rsc", "variable-revalidate/cookie/page.js", @@ -2104,6 +2188,10 @@ describe('app-dir static/dynamic handling', () => { expect(cur$('#data-revalidate-and-fetch-cache').text()).toBe( prev$('#data-revalidate-and-fetch-cache').text() ) + + expect(cur$('#data-auto-cache').text()).not.toBe( + prev$('data-auto-cache').text() + ) } finally { prevHtml = curHtml prev$ = cur$ @@ -2562,6 +2650,38 @@ describe('app-dir static/dynamic handling', () => { }, 'success') }) + it('should skip fetch cache when an authorization header is present after dynamic usage', async () => { + const initialReq = await next.fetch( + '/variable-revalidate/authorization/route-cookies' + ) + const initialJson = await initialReq.json() + + await retry(async () => { + const req = await next.fetch( + '/variable-revalidate/authorization/route-cookies' + ) + const json = await req.json() + + expect(json).not.toEqual(initialJson) + }) + }) + + it('should skip fetch cache when accessing request properties', async () => { + const initialReq = await next.fetch( + '/variable-revalidate/authorization/route-request' + ) + const initialJson = await initialReq.json() + + await retry(async () => { + const req = await next.fetch( + '/variable-revalidate/authorization/route-request' + ) + const json = await req.json() + + expect(json).not.toEqual(initialJson) + }) + }) + it('should not cache correctly with POST method request init', async () => { const res = await fetchViaHTTP( next.url, diff --git a/test/e2e/app-dir/app-static/app/force-dynamic-fetch-cache/default-cache/page.js b/test/e2e/app-dir/app-static/app/force-dynamic-fetch-cache/default-cache/page.js new file mode 100644 index 00000000000000..dfd72ddaecfab6 --- /dev/null +++ b/test/e2e/app-dir/app-static/app/force-dynamic-fetch-cache/default-cache/page.js @@ -0,0 +1,15 @@ +export const dynamic = 'force-dynamic' + +export default async function Page() { + const data = await fetch( + 'https://next-data-api-endpoint.vercel.app/api/random', + { cache: 'default' } + ).then((res) => res.text()) + + return ( + <> +

/force-dynamic-fetch-cache/default-cache

+

{data}

+ + ) +} diff --git a/test/e2e/app-dir/app-static/app/force-dynamic-fetch-cache/default-cache/route/route.js b/test/e2e/app-dir/app-static/app/force-dynamic-fetch-cache/default-cache/route/route.js new file mode 100644 index 00000000000000..4a6863ce455ca7 --- /dev/null +++ b/test/e2e/app-dir/app-static/app/force-dynamic-fetch-cache/default-cache/route/route.js @@ -0,0 +1,12 @@ +import { NextResponse } from 'next/server' + +export const dynamic = 'force-dynamic' + +export async function GET() { + const data = await fetch( + 'https://next-data-api-endpoint.vercel.app/api/random', + { cache: 'default' } + ).then((res) => res.text()) + + return NextResponse.json({ random: data }) +} diff --git a/test/e2e/app-dir/app-static/app/force-dynamic-fetch-cache/force-cache/page.js b/test/e2e/app-dir/app-static/app/force-dynamic-fetch-cache/force-cache/page.js new file mode 100644 index 00000000000000..cdd820f503c823 --- /dev/null +++ b/test/e2e/app-dir/app-static/app/force-dynamic-fetch-cache/force-cache/page.js @@ -0,0 +1,15 @@ +export const dynamic = 'force-dynamic' + +export default async function Page() { + const data = await fetch( + 'https://next-data-api-endpoint.vercel.app/api/random', + { cache: 'force-cache' } + ).then((res) => res.text()) + + return ( + <> +

/force-dynamic-fetch-cache/force-cache

+

{data}

+ + ) +} diff --git a/test/e2e/app-dir/app-static/app/force-dynamic-fetch-cache/force-cache/route/route.js b/test/e2e/app-dir/app-static/app/force-dynamic-fetch-cache/force-cache/route/route.js new file mode 100644 index 00000000000000..eb8e51b59a9bc1 --- /dev/null +++ b/test/e2e/app-dir/app-static/app/force-dynamic-fetch-cache/force-cache/route/route.js @@ -0,0 +1,12 @@ +import { NextResponse } from 'next/server' + +export const dynamic = 'force-dynamic' + +export async function GET() { + const data = await fetch( + 'https://next-data-api-endpoint.vercel.app/api/random', + { cache: 'force-cache' } + ).then((res) => res.text()) + + return NextResponse.json({ random: data }) +} diff --git a/test/e2e/app-dir/app-static/app/force-dynamic-fetch-cache/no-fetch-cache/route/route.js b/test/e2e/app-dir/app-static/app/force-dynamic-fetch-cache/no-fetch-cache/route/route.js new file mode 100644 index 00000000000000..6870e27ecd3c3b --- /dev/null +++ b/test/e2e/app-dir/app-static/app/force-dynamic-fetch-cache/no-fetch-cache/route/route.js @@ -0,0 +1,11 @@ +import { NextResponse } from 'next/server' + +export const dynamic = 'force-dynamic' + +export async function GET() { + const data = await fetch( + 'https://next-data-api-endpoint.vercel.app/api/random' + ).then((res) => res.text()) + + return NextResponse.json({ random: data }) +} diff --git a/test/e2e/app-dir/app-static/app/force-dynamic-fetch-cache/with-fetch-cache/route/route.js b/test/e2e/app-dir/app-static/app/force-dynamic-fetch-cache/with-fetch-cache/route/route.js new file mode 100644 index 00000000000000..7455b315bcc8e1 --- /dev/null +++ b/test/e2e/app-dir/app-static/app/force-dynamic-fetch-cache/with-fetch-cache/route/route.js @@ -0,0 +1,12 @@ +import { NextResponse } from 'next/server' + +export const dynamic = 'force-dynamic' +export const fetchCache = 'force-cache' + +export async function GET() { + const data = await fetch( + 'https://next-data-api-endpoint.vercel.app/api/random' + ).then((res) => res.text()) + + return NextResponse.json({ random: data }) +} diff --git a/test/e2e/app-dir/app-static/app/variable-revalidate/authorization/route-cookies/route.js b/test/e2e/app-dir/app-static/app/variable-revalidate/authorization/route-cookies/route.js new file mode 100644 index 00000000000000..6933f6af261cad --- /dev/null +++ b/test/e2e/app-dir/app-static/app/variable-revalidate/authorization/route-cookies/route.js @@ -0,0 +1,19 @@ +import { cookies } from 'next/headers' +import { NextResponse } from 'next/server' + +export async function GET() { + const cookieData = cookies() + + const data = await fetch( + 'https://next-data-api-endpoint.vercel.app/api/random', + { + headers: { + Authorization: 'Bearer token', + }, + } + ).then((res) => res.text()) + + console.log('has cookie data', !!cookieData) + + return NextResponse.json({ random: data }) +} diff --git a/test/e2e/app-dir/app-static/app/variable-revalidate/authorization/route-request/route.js b/test/e2e/app-dir/app-static/app/variable-revalidate/authorization/route-request/route.js new file mode 100644 index 00000000000000..d40cd668c2845c --- /dev/null +++ b/test/e2e/app-dir/app-static/app/variable-revalidate/authorization/route-request/route.js @@ -0,0 +1,18 @@ +import { NextResponse } from 'next/server' + +export async function GET(request) { + const requestCookies = request.cookies + + const data = await fetch( + 'https://next-data-api-endpoint.vercel.app/api/random', + { + headers: { + Authorization: 'Bearer token', + }, + } + ).then((res) => res.text()) + + console.log('has cookie data', !!requestCookies) + + return NextResponse.json({ random: data }) +}