From e66ae43ef8dd2374ffd213972cb753ba76dce5c3 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Tue, 9 Jan 2024 08:57:25 -0800 Subject: [PATCH 1/4] Implements a concept and set of APIs related to "Dynamic Rendering". Previously this was talked about externally in Docs but implemented as a staticGenerationBailout. There are however different semantics we want to have for dynamic using htings like `unstable_noStore` vs `headers()` for instance and having APIs oriented around these semantics will make future planned changes easier to implement correctly. The two main semantics are "the currently executing scope should be dynamic" and "some dynamic data source is being read" The former will eventually be a noop if we make dynamic the default. In accordance with this the dynamic intent is ignored inside a cache scope from `unstable_cache`. This is a very weak form of dynamic scoping and makes sense because it does not entail a request specific data read. The latter semantic is a stronger form. when doing any kind of static generation we disallow reading dynamic data sources such as headers, cookies, and searchParams. If these are read during a static render the render will bailout or postpone (depending on whether we are using PPR or not) force-static should still forcibly prevent dynamic data reads. This is now implementation outside of the triggering of a dynamic data read. This PR does not actually change yet how we define whether the result of a render was fully, partially, or not static. This will come in a follow up change where we can more faithfully track whether dynamic APIs were used separately from whether the render postpones. --- packages/next/cache.d.ts | 6 +- packages/next/cache.js | 4 +- .../src/client/components/client-page.tsx | 21 +++ .../next/src/client/components/headers.ts | 62 ++++---- .../request-async-storage.external.ts | 10 ++ .../src/client/components/search-params.ts | 73 +++++++++ .../components/searchparams-bailout-proxy.ts | 15 -- ...tatic-generation-async-storage.external.ts | 23 ++- .../components/static-generation-bailout.ts | 30 ++-- ...neration-searchparams-bailout-provider.tsx | 20 --- packages/next/src/lib/metadata/metadata.tsx | 10 +- .../next/src/lib/metadata/resolve-metadata.ts | 25 ++- .../next/src/server/app-render/app-render.tsx | 53 ++++--- .../app-render/create-component-tree.tsx | 142 ++++++++++------- .../server/app-render/dynamic-rendering.ts | 145 ++++++++++++++++++ .../next/src/server/app-render/entry-base.ts | 15 +- .../src/server/app-render/rsc/postpone.ts | 25 +++ ...static-generation-async-storage-wrapper.ts | 25 ++- packages/next/src/server/lib/patch-fetch.ts | 91 +++++------ .../src/server/web/exports/revalidate-path.ts | 2 +- .../src/server/web/exports/revalidate-tag.ts | 2 +- .../web/spec-extension/revalidate-path.ts | 26 ---- .../web/spec-extension/revalidate-tag.ts | 43 ------ .../server/web/spec-extension/revalidate.ts | 78 ++++++++++ .../web/spec-extension/unstable-cache.ts | 2 +- .../web/spec-extension/unstable-no-store.ts | 34 ++-- .../acceptance-app/rsc-runtime-errors.test.ts | 2 +- 27 files changed, 651 insertions(+), 333 deletions(-) create mode 100644 packages/next/src/client/components/client-page.tsx create mode 100644 packages/next/src/client/components/search-params.ts delete mode 100644 packages/next/src/client/components/searchparams-bailout-proxy.ts delete mode 100644 packages/next/src/client/components/static-generation-searchparams-bailout-provider.tsx create mode 100644 packages/next/src/server/app-render/dynamic-rendering.ts create mode 100644 packages/next/src/server/app-render/rsc/postpone.ts delete mode 100644 packages/next/src/server/web/spec-extension/revalidate-path.ts delete mode 100644 packages/next/src/server/web/spec-extension/revalidate-tag.ts create mode 100644 packages/next/src/server/web/spec-extension/revalidate.ts diff --git a/packages/next/cache.d.ts b/packages/next/cache.d.ts index 2f0e6fd1f1810..cd0a2387da47e 100644 --- a/packages/next/cache.d.ts +++ b/packages/next/cache.d.ts @@ -1,4 +1,6 @@ export { unstable_cache } from 'next/dist/server/web/spec-extension/unstable-cache' -export { revalidatePath } from 'next/dist/server/web/spec-extension/revalidate-path' -export { revalidateTag } from 'next/dist/server/web/spec-extension/revalidate-tag' +export { + revalidateTag, + revalidatePath, +} from 'next/dist/server/web/spec-extension/revalidate' export { unstable_noStore } from 'next/dist/server/web/spec-extension/unstable-no-store' diff --git a/packages/next/cache.js b/packages/next/cache.js index fd4a1dc19c8a7..091bdc46b7a35 100644 --- a/packages/next/cache.js +++ b/packages/next/cache.js @@ -1,9 +1,9 @@ const cacheExports = { unstable_cache: require('next/dist/server/web/spec-extension/unstable-cache') .unstable_cache, - revalidateTag: require('next/dist/server/web/spec-extension/revalidate-tag') + revalidateTag: require('next/dist/server/web/spec-extension/revalidate') .revalidateTag, - revalidatePath: require('next/dist/server/web/spec-extension/revalidate-path') + revalidatePath: require('next/dist/server/web/spec-extension/revalidate') .revalidatePath, unstable_noStore: require('next/dist/server/web/spec-extension/unstable-no-store') diff --git a/packages/next/src/client/components/client-page.tsx b/packages/next/src/client/components/client-page.tsx new file mode 100644 index 0000000000000..80482c678acf1 --- /dev/null +++ b/packages/next/src/client/components/client-page.tsx @@ -0,0 +1,21 @@ +'use client' +import { createDynamicallyTrackedSearchParams } from './search-params' + +export function ClientPageRoot({ + Component, + props, +}: { + Component: React.ComponentType + props: { [props: string]: any } +}) { + // We expect to be passed searchParams but even if we aren't we can construct one from + // an empty object. We only do this if we are in a static generation as a performance + // optimization. Ideally we'd unconditionally construct the tracked params but since + // this creates a proxy which is slow and this would happen even for client navigations + // that are done entirely dynamically and we know there the dynamic tracking is a noop + // in this dynamic case we can safely elide it. + props.searchParams = createDynamicallyTrackedSearchParams( + props.searchParams || {} + ) + return +} diff --git a/packages/next/src/client/components/headers.ts b/packages/next/src/client/components/headers.ts index a0a27a184cbfe..404c86df2f6e9 100644 --- a/packages/next/src/client/components/headers.ts +++ b/packages/next/src/client/components/headers.ts @@ -4,45 +4,46 @@ import { } from '../../server/web/spec-extension/adapters/request-cookies' import { HeadersAdapter } from '../../server/web/spec-extension/adapters/headers' import { RequestCookies } from '../../server/web/spec-extension/cookies' -import { requestAsyncStorage } from './request-async-storage.external' import { actionAsyncStorage } from './action-async-storage.external' -import { staticGenerationBailout } from './static-generation-bailout' import { DraftMode } from './draft-mode' +import { trackDynamicDataAccessed } from '../../server/app-render/dynamic-rendering' +import { staticGenerationAsyncStorage } from './static-generation-async-storage.external' +import { getExpectedRequestStore } from './request-async-storage.external' export function headers() { - if ( - staticGenerationBailout('headers', { - link: 'https://nextjs.org/docs/app/building-your-application/rendering/static-and-dynamic#dynamic-rendering', - }) - ) { - return HeadersAdapter.seal(new Headers({})) - } - const requestStore = requestAsyncStorage.getStore() - if (!requestStore) { - throw new Error( - `Invariant: headers() expects to have requestAsyncStorage, none available.` - ) + const callingExpression = 'headers' + const staticGenerationStore = staticGenerationAsyncStorage.getStore() + + if (staticGenerationStore) { + if (staticGenerationStore.forceStatic) { + // When we are forcings static we don't mark this as a Dynamic read and we return an empty headers object + return HeadersAdapter.seal(new Headers({})) + } else { + // We will return a real headers object below so we mark this call as reading from a dynamic data source + trackDynamicDataAccessed(staticGenerationStore, callingExpression) + } } + const requestStore = getExpectedRequestStore(callingExpression) return requestStore.headers } export function cookies() { - if ( - staticGenerationBailout('cookies', { - link: 'https://nextjs.org/docs/app/building-your-application/rendering/static-and-dynamic#dynamic-rendering', - }) - ) { - return RequestCookiesAdapter.seal(new RequestCookies(new Headers({}))) - } + const callingExpression = 'cookies' + const staticGenerationStore = staticGenerationAsyncStorage.getStore() - const requestStore = requestAsyncStorage.getStore() - if (!requestStore) { - throw new Error( - `Invariant: cookies() expects to have requestAsyncStorage, none available.` - ) + if (staticGenerationStore) { + if (staticGenerationStore.forceStatic) { + // When we are forcings static we don't mark this as a Dynamic read and we return an empty cookies object + return RequestCookiesAdapter.seal(new RequestCookies(new Headers({}))) + } else { + // We will return a real headers object below so we mark this call as reading from a dynamic data source + trackDynamicDataAccessed(staticGenerationStore, callingExpression) + } } + const requestStore = getExpectedRequestStore(callingExpression) + const asyncActionStore = actionAsyncStorage.getStore() if ( asyncActionStore && @@ -57,11 +58,8 @@ export function cookies() { } export function draftMode() { - const requestStore = requestAsyncStorage.getStore() - if (!requestStore) { - throw new Error( - `Invariant: draftMode() expects to have requestAsyncStorage, none available.` - ) - } + const callingExpression = 'draftMode' + const requestStore = getExpectedRequestStore(callingExpression) + return new DraftMode(requestStore.draftMode) } diff --git a/packages/next/src/client/components/request-async-storage.external.ts b/packages/next/src/client/components/request-async-storage.external.ts index a8a71c7906621..147764db04b66 100644 --- a/packages/next/src/client/components/request-async-storage.external.ts +++ b/packages/next/src/client/components/request-async-storage.external.ts @@ -17,3 +17,13 @@ export type RequestAsyncStorage = AsyncLocalStorage export const requestAsyncStorage: RequestAsyncStorage = createAsyncLocalStorage() + +export function getExpectedRequestStore(callingExpression: string) { + const store = requestAsyncStorage.getStore() + if (!store) { + throw new Error( + `Invariant: \`${callingExpression}\` expects to have requestAsyncStorage, none available.` + ) + } + return store +} diff --git a/packages/next/src/client/components/search-params.ts b/packages/next/src/client/components/search-params.ts new file mode 100644 index 0000000000000..d686f41c6fda8 --- /dev/null +++ b/packages/next/src/client/components/search-params.ts @@ -0,0 +1,73 @@ +import type { ParsedUrlQuery } from 'querystring' + +import { staticGenerationAsyncStorage } from './static-generation-async-storage.external' +import { trackDynamicDataAccessed } from '../../server/app-render/dynamic-rendering' +import { ReflectAdapter } from '../../server/web/spec-extension/adapters/reflect' + +/** + * Takes a ParsedUrlQuery object and either returns it unmodified or returns an empty object + * + * Even though we do not track read access on the returned searchParams we need to + * return an empty object if we are doing a 'force-static' render. This is to ensure + * we don't encode the searchParams into the flight data. + */ +export function createUntrackedSearchParams( + searchParams: ParsedUrlQuery +): ParsedUrlQuery { + const store = staticGenerationAsyncStorage.getStore() + if (store && store.forceStatic) { + return {} + } else { + return searchParams + } +} + +/** + * Takes a ParsedUrlQuery object and returns a Proxy that tracks read access to the object + * + * If running in the browser will always return the provided searchParams object. + * When running during SSR will return empty during a 'force-static' render and + * otherwise it returns a searchParams object which tracks reads to trigger dynamic rendering + * behavior if appropriate + */ +export function createDynamicallyTrackedSearchParams( + searchParams: ParsedUrlQuery +): ParsedUrlQuery { + const store = staticGenerationAsyncStorage.getStore() + if (!store) { + // we assume we are in a route handler or page render. just return the searchParams + return searchParams + } else if (store.forceStatic) { + // If we forced static we omit searchParams entirely. This is true both during SSR + // and browser render because we need there to be parity between these environments + return {} + } else if (!store.isStaticGeneration && !store.dynamicShouldError) { + // during dynamic renders we don't actually have to track anything so we just return + // the searchParams directly. However if we dynamic data access should error then we + // still want to track access. This covers the case in Dev where all renders are dynamic + // but we still want to error if you use a dynamic data source because it will fail the build + // or revalidate if you do. + return searchParams + } else { + // We need to track dynamic access with a Proxy. We implement get, has, and ownKeys because + // these can all be used to exfiltrate information about searchParams. + return new Proxy({} as ParsedUrlQuery, { + get(target, prop, receiver) { + if (typeof prop === 'string') { + trackDynamicDataAccessed(store, `searchParams.${prop}`) + } + return ReflectAdapter.get(target, prop, receiver) + }, + has(target, prop) { + if (typeof prop === 'string') { + trackDynamicDataAccessed(store, `searchParams.${prop}`) + } + return Reflect.has(target, prop) + }, + ownKeys(target) { + trackDynamicDataAccessed(store, 'searchParams') + return Reflect.ownKeys(target) + }, + }) + } +} diff --git a/packages/next/src/client/components/searchparams-bailout-proxy.ts b/packages/next/src/client/components/searchparams-bailout-proxy.ts deleted file mode 100644 index fbae8de1f5d23..0000000000000 --- a/packages/next/src/client/components/searchparams-bailout-proxy.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { staticGenerationBailout } from './static-generation-bailout' - -export function createSearchParamsBailoutProxy() { - return new Proxy( - {}, - { - get(_target, prop) { - // React adds some properties on the object when serializing for client components - if (typeof prop === 'string') { - staticGenerationBailout(`searchParams.${prop}`) - } - }, - } - ) -} diff --git a/packages/next/src/client/components/static-generation-async-storage.external.ts b/packages/next/src/client/components/static-generation-async-storage.external.ts index 9ad563d3bbff2..f17c507811f4e 100644 --- a/packages/next/src/client/components/static-generation-async-storage.external.ts +++ b/packages/next/src/client/components/static-generation-async-storage.external.ts @@ -6,6 +6,10 @@ import type { Revalidate } from '../../server/lib/revalidate' import { createAsyncLocalStorage } from './async-local-storage' +type PrerenderState = { + hasDynamic: boolean +} + export interface StaticGenerationStore { readonly isStaticGeneration: boolean readonly pagePath?: string @@ -16,12 +20,8 @@ export interface StaticGenerationStore { readonly isRevalidate?: boolean readonly isUnstableCacheCallback?: boolean - /** - * If defined, this function when called will throw an error postponing - * rendering during the React render phase. This should not be invoked outside - * of the React render phase as it'll throw an error. - */ - readonly postpone: ((reason: string) => never) | undefined + // When this exists (is not null) it means we are in a Prerender + prerenderState: null | PrerenderState forceDynamic?: boolean fetchCache?: @@ -36,7 +36,6 @@ export interface StaticGenerationStore { forceStatic?: boolean dynamicShouldError?: boolean pendingRevalidates?: Record> - postponeWasTriggered?: boolean dynamicUsageDescription?: string dynamicUsageStack?: string @@ -59,3 +58,13 @@ export type StaticGenerationAsyncStorage = export const staticGenerationAsyncStorage: StaticGenerationAsyncStorage = createAsyncLocalStorage() + +export function getExpectedStaticGenerationStore(callingExpression: string) { + const store = staticGenerationAsyncStorage.getStore() + if (!store) { + throw new Error( + `Invariant: \`${callingExpression}\` expects to have staticGenerationAsyncStorage, none available.` + ) + } + return store +} diff --git a/packages/next/src/client/components/static-generation-bailout.ts b/packages/next/src/client/components/static-generation-bailout.ts index 740ee5421065d..17e7cb1cc978d 100644 --- a/packages/next/src/client/components/static-generation-bailout.ts +++ b/packages/next/src/client/components/static-generation-bailout.ts @@ -1,11 +1,14 @@ import type { AppConfigDynamic } from '../../build/utils' +import React from 'react' import { DynamicServerError } from './hooks-server-context' import { staticGenerationAsyncStorage } from './static-generation-async-storage.external' +const hasPostpone = typeof React.unstable_postpone === 'function' + const NEXT_STATIC_GEN_BAILOUT = 'NEXT_STATIC_GEN_BAILOUT' -class StaticGenBailoutError extends Error { +export class StaticGenBailoutError extends Error { public readonly code = NEXT_STATIC_GEN_BAILOUT } @@ -51,22 +54,23 @@ export const staticGenerationBailout: StaticGenerationBailout = ( ) } - const message = formatErrorMessage(reason, { - dynamic, - // this error should be caught by Next to bail out of static generation - // in case it's uncaught, this link provides some additional context as to why - link: 'https://nextjs.org/docs/messages/dynamic-server-error', - }) - - // If postpone is available, we should postpone the render. - staticGenerationStore.postpone?.(reason) + if (staticGenerationStore.prerenderState && hasPostpone) { + throw React.unstable_postpone(formatErrorMessage(reason, { link, dynamic })) + } // As this is a bailout, we don't want to revalidate, so set the revalidate // to 0. staticGenerationStore.revalidate = 0 if (staticGenerationStore.isStaticGeneration) { - const err = new DynamicServerError(message) + const err = new DynamicServerError( + formatErrorMessage(reason, { + dynamic, + // this error should be caught by Next to bail out of static generation + // in case it's uncaught, this link provides some additional context as to why + link: 'https://nextjs.org/docs/messages/dynamic-server-error', + }) + ) staticGenerationStore.dynamicUsageDescription = reason staticGenerationStore.dynamicUsageStack = err.stack @@ -75,3 +79,7 @@ export const staticGenerationBailout: StaticGenerationBailout = ( return false } + +// export function interuptStaticGeneration(store: StaticGenerationStore) { +// if (store.) +// } diff --git a/packages/next/src/client/components/static-generation-searchparams-bailout-provider.tsx b/packages/next/src/client/components/static-generation-searchparams-bailout-provider.tsx deleted file mode 100644 index 18519c912e154..0000000000000 --- a/packages/next/src/client/components/static-generation-searchparams-bailout-provider.tsx +++ /dev/null @@ -1,20 +0,0 @@ -'use client' -import React from 'react' -import { createSearchParamsBailoutProxy } from './searchparams-bailout-proxy' - -export default function StaticGenerationSearchParamsBailoutProvider({ - Component, - propsForComponent, - isStaticGeneration, -}: { - Component: React.ComponentType - propsForComponent: any - isStaticGeneration: boolean -}) { - if (isStaticGeneration) { - const searchParams = createSearchParamsBailoutProxy() - return - } - - return -} diff --git a/packages/next/src/lib/metadata/metadata.tsx b/packages/next/src/lib/metadata/metadata.tsx index 45fefdff81d87..198a6aa7a3a98 100644 --- a/packages/next/src/lib/metadata/metadata.tsx +++ b/packages/next/src/lib/metadata/metadata.tsx @@ -1,3 +1,4 @@ +import type { ParsedUrlQuery } from 'querystring' import type { GetDynamicParamFromSegment } from '../../server/app-render/app-render' import type { LoaderTree } from '../../server/lib/app-dir-module' @@ -38,17 +39,21 @@ import { isNotFoundError } from '../../client/components/not-found' export function createMetadataComponents({ tree, pathname, - searchParams, + query, getDynamicParamFromSegment, appUsingSizeAdjustment, errorType, + createDynamicallyTrackedSearchParams, }: { tree: LoaderTree pathname: string - searchParams: { [key: string]: any } + query: ParsedUrlQuery getDynamicParamFromSegment: GetDynamicParamFromSegment appUsingSizeAdjustment: boolean errorType?: 'not-found' | 'redirect' + createDynamicallyTrackedSearchParams: ( + searchParams: ParsedUrlQuery + ) => ParsedUrlQuery }): [React.ComponentType, React.ComponentType] { const metadataContext = { pathname, @@ -68,6 +73,7 @@ export function createMetadataComponents({ let error: any const errorMetadataItem: [null, null, null] = [null, null, null] const errorConvention = errorType === 'redirect' ? undefined : errorType + const searchParams = createDynamicallyTrackedSearchParams(query) const [resolvedError, resolvedMetadata, resolvedViewport] = await resolveMetadata({ diff --git a/packages/next/src/lib/metadata/resolve-metadata.ts b/packages/next/src/lib/metadata/resolve-metadata.ts index 824dde3f337aa..c0bf667d10d33 100644 --- a/packages/next/src/lib/metadata/resolve-metadata.ts +++ b/packages/next/src/lib/metadata/resolve-metadata.ts @@ -14,6 +14,8 @@ import type { ComponentsType } from '../../build/webpack/loaders/next-app-loader import type { MetadataContext } from './types/resolvers' import type { LoaderTree } from '../../server/lib/app-dir-module' import type { AbsoluteTemplateString } from './types/metadata-types' +import type { ParsedUrlQuery } from 'querystring' + import { createDefaultMetadata, createDefaultViewport, @@ -67,6 +69,14 @@ type BuildState = { warnings: Set } +type LayoutProps = { + params: { [key: string]: any } +} +type PageProps = { + params: { [key: string]: any } + searchParams: { [key: string]: any } +} + function hasIconsProperty( icons: Metadata['icons'], prop: 'icon' | 'apple' @@ -462,7 +472,7 @@ export async function resolveMetadataItems({ /** Provided tree can be nested subtree, this argument says what is the path of such subtree */ treePrefix?: string[] getDynamicParamFromSegment: GetDynamicParamFromSegment - searchParams: { [key: string]: any } + searchParams: ParsedUrlQuery errorConvention: 'not-found' | undefined }): Promise { const [segment, parallelRoutes, { page }] = tree @@ -484,9 +494,16 @@ export async function resolveMetadataItems({ : // Pass through parent params to children parentParams - const layerProps = { - params: currentParams, - ...(isPage && { searchParams }), + let layerProps: LayoutProps | PageProps + if (isPage) { + layerProps = { + params: currentParams, + searchParams, + } + } else { + layerProps = { + params: currentParams, + } } await collectMetadata({ diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index a88c869987db7..7ccacfe860027 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -104,9 +104,7 @@ export type AppRenderContext = AppRenderBaseContext & { getDynamicParamFromSegment: GetDynamicParamFromSegment query: NextParsedUrlQuery isPrefetch: boolean - providedSearchParams: NextParsedUrlQuery requestTimestamp: number - searchParamsProps: { searchParams: NextParsedUrlQuery } appUsingSizeAdjustment: boolean providedFlightRouterState?: FlightRouterState requestId: string @@ -250,11 +248,15 @@ async function generateFlight( let flightData: FlightData | null = null const { - componentMod: { tree: loaderTree, renderToReadableStream }, + componentMod: { + tree: loaderTree, + renderToReadableStream, + createDynamicallyTrackedSearchParams, + }, getDynamicParamFromSegment, appUsingSizeAdjustment, staticGenerationStore: { urlPathname }, - providedSearchParams, + query, requestId, providedFlightRouterState, } = ctx @@ -263,9 +265,10 @@ async function generateFlight( const [MetadataTree, MetadataOutlet] = createMetadataComponents({ tree: loaderTree, pathname: urlPathname, - searchParams: providedSearchParams, + query, getDynamicParamFromSegment, appUsingSizeAdjustment, + createDynamicallyTrackedSearchParams, }) flightData = ( await walkTreeWithFlightRouterState({ @@ -374,9 +377,12 @@ async function ReactServerApp({ const { getDynamicParamFromSegment, query, - providedSearchParams, appUsingSizeAdjustment, - componentMod: { AppRouter, GlobalError }, + componentMod: { + AppRouter, + GlobalError, + createDynamicallyTrackedSearchParams, + }, staticGenerationStore: { urlPathname }, } = ctx const initialTree = createFlightRouterStateFromLoaderTree( @@ -389,9 +395,10 @@ async function ReactServerApp({ tree, errorType: asNotFound ? 'not-found' : undefined, pathname: urlPathname, - searchParams: providedSearchParams, + query, getDynamicParamFromSegment: getDynamicParamFromSegment, appUsingSizeAdjustment: appUsingSizeAdjustment, + createDynamicallyTrackedSearchParams, }) const { seedData, styles } = await createComponentTree({ @@ -454,9 +461,12 @@ async function ReactServerError({ const { getDynamicParamFromSegment, query, - providedSearchParams, appUsingSizeAdjustment, - componentMod: { AppRouter, GlobalError }, + componentMod: { + AppRouter, + GlobalError, + createDynamicallyTrackedSearchParams, + }, staticGenerationStore: { urlPathname }, requestId, res, @@ -467,9 +477,10 @@ async function ReactServerError({ tree, pathname: urlPathname, errorType, - searchParams: providedSearchParams, + query, getDynamicParamFromSegment, appUsingSizeAdjustment, + createDynamicallyTrackedSearchParams, }) const head = ( @@ -683,11 +694,7 @@ async function renderToHTMLOrFlightImpl( const generateStaticHTML = supportsDynamicHTML !== true // Pull out the hooks/references from the component. - const { - createSearchParamsBailoutProxy, - tree: loaderTree, - taintObjectReference, - } = ComponentMod + const { tree: loaderTree, taintObjectReference } = ComponentMod if (enableTainting) { taintObjectReference( @@ -733,13 +740,6 @@ async function renderToHTMLOrFlightImpl( requestId = require('next/dist/compiled/nanoid').nanoid() } - // During static generation we need to call the static generation bailout when reading searchParams - const providedSearchParams = isStaticGeneration - ? createSearchParamsBailoutProxy() - : query - - const searchParamsProps = { searchParams: providedSearchParams } - /** * Dynamic parameters. E.g. when you visit `/dashboard/vercel` which is rendered by `/dashboard/[slug]` the value will be {"slug": "vercel"}. */ @@ -755,9 +755,7 @@ async function renderToHTMLOrFlightImpl( getDynamicParamFromSegment, query, isPrefetch: isPrefetchRSCRequest, - providedSearchParams, requestTimestamp, - searchParamsProps, appUsingSizeAdjustment, providedFlightRouterState, requestId, @@ -1224,8 +1222,9 @@ async function renderToHTMLOrFlightImpl( // bailout error was also emitted which indicates that part of the stream was // not rendered. if ( - renderOpts.experimental.ppr && - staticGenerationStore.postponeWasTriggered && + staticGenerationStore.prerenderState && + staticGenerationStore.prerenderState.hasDynamic && + // but there's no postpone state !metadata.postponed && (!response.err || !isBailoutToCSRError(response.err)) ) { diff --git a/packages/next/src/server/app-render/create-component-tree.tsx b/packages/next/src/server/app-render/create-component-tree.tsx index c179e9245bd5a..f4fb98ab5e24c 100644 --- a/packages/next/src/server/app-render/create-component-tree.tsx +++ b/packages/next/src/server/app-render/create-component-tree.tsx @@ -23,18 +23,6 @@ type Params = { [key: string]: string | string[] } -/** - * This component will call `React.postpone` that throws the postponed error. - */ -export const Postpone = ({ - postpone, -}: { - postpone: (reason: string) => never -}): never => { - // Call the postpone API now with the reason set to "force-dynamic". - return postpone('dynamic = "force-dynamic" was used') -} - /** * Use the provided loader tree to create the React Component tree. */ @@ -96,13 +84,16 @@ async function createComponentTreeInternal({ NotFoundBoundary, LayoutRouter, RenderFromTemplateContext, - StaticGenerationSearchParamsBailoutProvider, + ClientPageRoot, + createUntrackedSearchParams, + createDynamicallyTrackedSearchParams, serverHooks: { DynamicServerError }, + Postpone, }, pagePath, getDynamicParamFromSegment, isPrefetch, - searchParamsProps, + query, } = ctx const { page, layoutOrPagePath, segment, components, parallelRoutes } = @@ -213,7 +204,7 @@ async function createComponentTreeInternal({ staticGenerationStore.forceDynamic = true // TODO: (PPR) remove this bailout once PPR is the default - if (!staticGenerationStore.postpone) { + if (!staticGenerationStore.prerenderState) { // If the postpone API isn't available, we can't postpone the render and // therefore we can't use the dynamic API. staticGenerationBailout(`force-dynamic`, { dynamic }) @@ -256,7 +247,7 @@ async function createComponentTreeInternal({ ctx.defaultRevalidate === 0 && // If the postpone API isn't available, we can't postpone the render and // therefore we can't use the dynamic API. - !staticGenerationStore.postpone + !staticGenerationStore.prerenderState ) { const dynamicUsageDescription = `revalidate: 0 configured ${segment}` staticGenerationStore.dynamicUsageDescription = dynamicUsageDescription @@ -519,12 +510,22 @@ async function createComponentTreeInternal({ // replace it with a node that will postpone the render. This ensures that the // postpone is invoked during the react render phase and not during the next // render phase. - if (staticGenerationStore.forceDynamic && staticGenerationStore.postpone) { + // @TODO this does not actually do what it seems like it would or should do. The idea is that + // if we are rendering in a force-dynamic mode and we can postpone we should only make the segments + // that ask for force-dynamic to be dynamic, allowing other segments to still prerender. However + // because this comes after the children traversal and the static generation store is mutated every segment + // along the parent path of a force-dynamic segment will hit this condition effectively making the entire + // render force-dynamic. We should refactor this function so that we can correctly track which segments + // need to be dynamic + if ( + staticGenerationStore.forceDynamic && + staticGenerationStore.prerenderState + ) { return { seedData: [ actualSegment, parallelRouteCacheNodeSeedData, - , + , ], styles: layerAssets, } @@ -532,9 +533,11 @@ async function createComponentTreeInternal({ const isClientComponent = isClientReference(layoutOrPageMod) + // We avoid cloning this object because it gets consumed here exclusively. + const props: { [prop: string]: any } = parallelRouteProps + // If it's a not found route, and we don't have any matched parallel // routes, we try to render the not found component if it exists. - let notFoundComponent = {} if ( NotFound && asNotFound && @@ -542,37 +545,70 @@ async function createComponentTreeInternal({ // Or if there's no parallel routes means it reaches the end. !parallelRouteMap.length ) { - notFoundComponent = { - children: ( - <> - - {process.env.NODE_ENV === 'development' && ( - - )} - {notFoundStyles} - - - ), - } + props.children = ( + <> + + {process.env.NODE_ENV === 'development' && ( + + )} + {notFoundStyles} + + + ) } - const props = { - ...parallelRouteProps, - ...notFoundComponent, - // TODO-APP: params and query have to be blocked parallel route names. Might have to add a reserved name list. - // Params are always the current params that apply to the layout - // If you have a `/dashboard/[team]/layout.js` it will provide `team` as a param but not anything further down. - params: currentParams, - // Query is only provided to page - ...(() => { - if (isClientComponent && staticGenerationStore.isStaticGeneration) { - return {} - } + // Assign params to props + if ( + process.env.NODE_ENV === 'development' && + 'params' in parallelRouteProps + ) { + // @TODO consider making this an error and runnign the check in build as well + console.error( + `The prop name "params" is reserved in Layouts and Pages and cannot be used as the name of a parallel routes in ${segment}.` + ) + } + props.params = currentParams - if (isPage) { - return searchParamsProps - } - })(), + let segmentElement: React.ReactNode + if (isPage) { + // Assign searchParams to props if this is a page + if ( + process.env.NODE_ENV === 'development' && + 'searchParams' in parallelRouteProps + ) { + // @TODO consider making this an error and runnign the check in build as well + console.error( + `The prop name "searchParams" is reserved in Layouts and Pages and cannot be used as the name of a parallel routes in ${segment}.` + ) + } + + if (isClientComponent) { + // When we are passing searchParams to a client component Page we don't want to try the access + // dynamically here in the RSC layer because the serialization will trigger a dynamic API usage. + // Instead we pass the searchParams untracked but we wrap the Page in a root client component + // which can among other things adds the dynamic tracking before rendering the page. + // @TODO make the root wrapper part of next-app-loader so we don't need the extra client component + props.searchParams = createUntrackedSearchParams(query) + segmentElement = ( + <> + {metadataOutlet} + + + ) + } else { + // If we are passing searchParams to a server component Page we need to track their usage in case + // the current render mode tracks dynamic API usage. + props.searchParams = createDynamicallyTrackedSearchParams(query) + segmentElement = ( + <> + {metadataOutlet} + + + ) + } + } else { + // For layouts we just render the component + segmentElement = } return { @@ -580,17 +616,7 @@ async function createComponentTreeInternal({ actualSegment, parallelRouteCacheNodeSeedData, <> - {isPage ? metadataOutlet : null} - {/* needs to be the first element because we use `findDOMNode` in layout router to locate it. */} - {isPage && isClientComponent ? ( - - ) : ( - - )} + {segmentElement} {/* This null is currently critical. The wrapped Component can render null and if there was not fragment surrounding it this would look like a pending tree data state on the client which will cause an error and break the app. Long-term we need to move away from using null as a partial tree identifier since it diff --git a/packages/next/src/server/app-render/dynamic-rendering.ts b/packages/next/src/server/app-render/dynamic-rendering.ts new file mode 100644 index 0000000000000..98ca899d91f22 --- /dev/null +++ b/packages/next/src/server/app-render/dynamic-rendering.ts @@ -0,0 +1,145 @@ +/** + * The functions provided by this module are used to communicate certain properties + * about the currently running code so that Next.js can make decisions on how to handle + * the current execution in different rendering modes such as pre-rendering, resuming, and SSR. + * + * Today Next.js treats all code as potentially static. Certain APIs may only make sense when dynamically rendering. + * Traditionally this meant deopting the entire render to dynamic however with PPR we can now deopt parts + * of a React tree as dynamic while still keeping other parts static. There are really two different kinds of + * Dynamic indications. + * + * The first is simply an intention to be dynamic. unstable_noStore is an example of this where + * the currently executing code simply declares that the current scope is dynamic but if you use it + * inside unstable_cache it can still be cached. This type of indication can be removed if we ever + * make the default dynamic to begin with because the only way you would ever be static is inside + * a cache scope which this indication does not affect. + * + * The second is an indication that a dynamic data source was read. This is a stronger form of dynamic + * because it means that it is inappropriate to cache this at all. using a dynamic data source inside + * unstable_cache should error. If you want to use some dynamic data inside unstable_cache you should + * read that data outside the cache and pass it in as an argument to the cached function. + */ + +// Once postpone is in stable we should switch to importing the postpone export directly +import React from 'react' + +import type { StaticGenerationStore } from '../../client/components/static-generation-async-storage.external' +import { DynamicServerError } from '../../client/components/hooks-server-context' + +import { StaticGenBailoutError } from '../../client/components/static-generation-bailout' + +const hasPostpone = typeof React.unstable_postpone === 'function' + +/** + * This function communicates that the current scope should be treated as dynamic. + * + * In most cases this function is a no-op but if called during + * a PPR prerender it will postpone the current sub-tree. + */ +export function markCurrentScopeAsDynamic( + store: StaticGenerationStore, + expression: string +): void { + if (store.isUnstableCacheCallback) { + // inside cache scopes marking a scope as dynamic has no effect because the outer cache scope + // creates a cache boundary. This is subtly different from reading a dynamic data source which is + // forbidden inside a cache scope. + return + } else if (store.dynamicShouldError) { + throw new StaticGenBailoutError( + `Page with \`dynamic = "error"\` couldn't be rendered statically because it used \`${expression}\`. See more info here: https://nextjs.org/docs/app/building-your-application/rendering/static-and-dynamic#dynamic-rendering` + ) + } else if ( + // We are in a prerender (PPR enabled, during build) + store.prerenderState && + hasPostpone + ) { + // We track that we had a dynamic scope that postponed. + // This will be used by the renderer to decide whether + // the prerender requires a resume + store.prerenderState.hasDynamic = true + React.unstable_postpone(createPostponeReason(expression)) + } else { + store.revalidate = 0 + + if (store.isStaticGeneration) { + // We aren't prerendering but we are generating a static page We need to bail out of static generation + const err = new DynamicServerError( + `Page couldn't be rendered statically because it used ${expression}. See more info here: https://nextjs.org/docs/messages/dynamic-server-error` + ) + store.dynamicUsageDescription = expression + store.dynamicUsageStack = err.stack + + throw err + } + } +} + +/** + * This function communicates that some dynamic data was read. This typically would refer to accessing + * a Request specific data store such as cookies or headers. This function is not how end-users will + * describe reading from dynamic data sources which are valid to cache and up to the author to make + * a determination of when to do so. + * + * If we are inside a cache scope we error + * Also during a PPR Prerender we postpone + */ +export function trackDynamicDataAccessed( + store: StaticGenerationStore, + expression: string +): void { + if (store.isUnstableCacheCallback) { + throw new Error( + `used "${expression}" inside a function cached with "unstable_cache(...)". Accessing Dynamic data sources inside a cache scope is not supported. If you need this data inside a cached function use "${expression}" oustide the cached function and pass the required dynamic data in as an argument. See more info here: https://nextjs.org/docs/app/building-your-application/rendering/static-and-dynamic#dynamic-rendering` + ) + } else if (store.dynamicShouldError) { + throw new StaticGenBailoutError( + `Page with \`dynamic = "error"\` couldn't be rendered statically because it used \`${expression}\`. See more info here: https://nextjs.org/docs/app/building-your-application/rendering/static-and-dynamic#dynamic-rendering` + ) + } else if ( + // We are in a prerender (PPR enabled, during build) + store.prerenderState && + hasPostpone + ) { + // We track that we had a dynamic scope that postponed. + // This will be used by the renderer to decide whether + // the prerender requires a resume + store.prerenderState.hasDynamic = true + React.unstable_postpone(createPostponeReason(expression)) + } else { + store.revalidate = 0 + + if (store.isStaticGeneration) { + // We aren't prerendering but we are generating a static page We need to bail out of static generation + const err = new DynamicServerError( + `Page couldn't be rendered statically because it used ${expression}. See more info here: https://nextjs.org/docs/messages/dynamic-server-error` + ) + store.dynamicUsageDescription = expression + store.dynamicUsageStack = err.stack + + throw err + } + } +} + +// @TODO refactor patch-fetch and this function to better model dynamic semantics. Currently this implementation +// is too explicit about postponing if we are in a prerender and patch-fetch contains a lot of logic for determining +// what makes the fetch "dynamic". It also doesn't handle Non PPR cases so it is isn't as consistent with the other +// dynamic-rendering methods. +export function trackDynamicFetch( + store: StaticGenerationStore, + expression: string +) { + if (store.prerenderState && hasPostpone) { + store.prerenderState.hasDynamic = true + React.unstable_postpone(createPostponeReason(expression)) + } +} + +function createPostponeReason(expression: string) { + return ( + `This page needs to bail out of prerendering at this point because it used ${expression}. ` + + `React throws this special object to indicate where. It should not be caught by ` + + `your own try/catch. Learn more: https://nextjs.org/docs/messages/ppr-caught-error` + ) +} diff --git a/packages/next/src/server/app-render/entry-base.ts b/packages/next/src/server/app-render/entry-base.ts index ebd618faa7eb0..3ac94d9e0cfcc 100644 --- a/packages/next/src/server/app-render/entry-base.ts +++ b/packages/next/src/server/app-render/entry-base.ts @@ -13,8 +13,11 @@ import { staticGenerationAsyncStorage } from '../../client/components/static-gen import { requestAsyncStorage } from '../../client/components/request-async-storage.external' import { actionAsyncStorage } from '../../client/components/action-async-storage.external' import { staticGenerationBailout } from '../../client/components/static-generation-bailout' -import StaticGenerationSearchParamsBailoutProvider from '../../client/components/static-generation-searchparams-bailout-provider' -import { createSearchParamsBailoutProxy } from '../../client/components/searchparams-bailout-proxy' +import { ClientPageRoot } from '../../client/components/client-page' +import { + createUntrackedSearchParams, + createDynamicallyTrackedSearchParams, +} from '../../client/components/search-params' import * as serverHooks from '../../client/components/hooks-server-context' import { NotFoundBoundary } from '../../client/components/not-found-boundary' import { patchFetch as _patchFetch } from '../lib/patch-fetch' @@ -26,7 +29,7 @@ import { preloadFont, preconnect, } from '../../server/app-render/rsc/preloads' - +import { Postpone } from '../../server/app-render/rsc/postpone' import { taintObjectReference } from '../../server/app-render/rsc/taint' // patchFetch makes use of APIs such as `React.unstable_postpone` which are only available @@ -43,13 +46,15 @@ export { requestAsyncStorage, actionAsyncStorage, staticGenerationBailout, - createSearchParamsBailoutProxy, + createUntrackedSearchParams, + createDynamicallyTrackedSearchParams, serverHooks, preloadStyle, preloadFont, preconnect, + Postpone, taintObjectReference, - StaticGenerationSearchParamsBailoutProvider, + ClientPageRoot, NotFoundBoundary, patchFetch, } diff --git a/packages/next/src/server/app-render/rsc/postpone.ts b/packages/next/src/server/app-render/rsc/postpone.ts new file mode 100644 index 0000000000000..2f40199583da4 --- /dev/null +++ b/packages/next/src/server/app-render/rsc/postpone.ts @@ -0,0 +1,25 @@ +/* + +Files in the rsc directory are meant to be packaged as part of the RSC graph using next-app-loader. + +*/ + +// When postpone is available in canary React we can switch to importing it directly +import React from 'react' + +const missingPostpone = typeof React.unstable_postpone !== 'function' + +/** + * This component will call `React.postpone` that throws the postponed error. + */ +type PostponeProps = { + reason: string +} +export function Postpone({ reason }: PostponeProps): never { + if (missingPostpone) { + throw new Error( + 'Invariant: Postpone component expected `postpone` to exist in React.' + ) + } + return React.unstable_postpone(reason) +} diff --git a/packages/next/src/server/async-storage/static-generation-async-storage-wrapper.ts b/packages/next/src/server/async-storage/static-generation-async-storage-wrapper.ts index 70c25d5b8b440..bb7765164849f 100644 --- a/packages/next/src/server/async-storage/static-generation-async-storage-wrapper.ts +++ b/packages/next/src/server/async-storage/static-generation-async-storage-wrapper.ts @@ -38,7 +38,7 @@ export const StaticGenerationAsyncStorageWrapper: AsyncStorageWrapper< > = { wrap( storage: AsyncLocalStorage, - { urlPathname, renderOpts, postpone }: StaticGenerationContext, + { urlPathname, renderOpts }: StaticGenerationContext, callback: (store: StaticGenerationStore) => Result ): Result { /** @@ -63,6 +63,13 @@ export const StaticGenerationAsyncStorageWrapper: AsyncStorageWrapper< !renderOpts.isDraftMode && !renderOpts.isServerAction + const prerenderState: StaticGenerationStore['prerenderState'] = + isStaticGeneration && renderOpts.experimental.ppr + ? { + hasDynamic: false, + } + : null + const store: StaticGenerationStore = { isStaticGeneration, urlPathname, @@ -78,21 +85,7 @@ export const StaticGenerationAsyncStorageWrapper: AsyncStorageWrapper< isDraftMode: renderOpts.isDraftMode, - postpone: - // If we aren't performing a static generation or we aren't using PPR then - // we don't need to postpone. - isStaticGeneration && renderOpts.experimental.ppr && postpone - ? (reason: string) => { - // Keep track of if the postpone API has been called. - store.postponeWasTriggered = true - - return postpone( - `This page needs to bail out of prerendering at this point because it used ${reason}. ` + - `React throws this special object to indicate where. It should not be caught by ` + - `your own try/catch. Learn more: https://nextjs.org/docs/messages/ppr-caught-error` - ) - } - : undefined, + prerenderState, } // TODO: remove this when we resolve accessing the store outside the execution context diff --git a/packages/next/src/server/lib/patch-fetch.ts b/packages/next/src/server/lib/patch-fetch.ts index 843f40353a7a9..cbe2202e5cb08 100644 --- a/packages/next/src/server/lib/patch-fetch.ts +++ b/packages/next/src/server/lib/patch-fetch.ts @@ -12,6 +12,7 @@ import { NEXT_CACHE_TAG_MAX_LENGTH, } from '../../lib/constants' import * as Log from '../../build/output/log' +import { trackDynamicFetch } from '../app-render/dynamic-rendering' const isEdgeRuntime = process.env.NEXT_RUNTIME === 'edge' @@ -280,16 +281,7 @@ export function patchFetch({ } const implicitTags = addImplicitTags(staticGenerationStore) - const isOnlyCache = staticGenerationStore.fetchCache === 'only-cache' - const isForceCache = staticGenerationStore.fetchCache === 'force-cache' - const isDefaultCache = - staticGenerationStore.fetchCache === 'default-cache' - const isDefaultNoStore = - staticGenerationStore.fetchCache === 'default-no-store' - const isOnlyNoStore = - staticGenerationStore.fetchCache === 'only-no-store' - const isForceNoStore = - staticGenerationStore.fetchCache === 'force-no-store' + const fetchCacheMode = staticGenerationStore.fetchCache const isUsingNoStore = !!staticGenerationStore.isUnstableNoStore let _cache = getRequestMeta('cache') @@ -314,8 +306,8 @@ export function patchFetch({ } else if ( _cache === 'no-cache' || _cache === 'no-store' || - isForceNoStore || - isOnlyNoStore + fetchCacheMode === 'force-no-store' || + fetchCacheMode === 'only-no-store' ) { curRevalidate = 0 } @@ -349,45 +341,54 @@ export function patchFetch({ (hasUnCacheableHeader || isUnCacheableMethod) && staticGenerationStore.revalidate === 0 - if (isForceNoStore) { - cacheReason = 'fetchCache = force-no-store' - } - - if (isOnlyNoStore) { - if ( - _cache === 'force-cache' || - (typeof revalidate !== 'undefined' && - (revalidate === false || revalidate > 0)) - ) { - throw new Error( - `cache: 'force-cache' used on fetch for ${fetchUrl} with 'export const fetchCache = 'only-no-store'` - ) + switch (fetchCacheMode) { + case 'force-no-store': { + cacheReason = 'fetchCache = force-no-store' + break } - cacheReason = 'fetchCache = only-no-store' - } - - if (isOnlyCache && _cache === 'no-store') { - throw new Error( - `cache: 'no-store' used on fetch for ${fetchUrl} with 'export const fetchCache = 'only-cache'` - ) - } - - if ( - isForceCache && - (typeof curRevalidate === 'undefined' || curRevalidate === 0) - ) { - cacheReason = 'fetchCache = force-cache' - revalidate = false + case 'only-no-store': { + if ( + _cache === 'force-cache' || + (typeof revalidate !== 'undefined' && + (revalidate === false || revalidate > 0)) + ) { + throw new Error( + `cache: 'force-cache' used on fetch for ${fetchUrl} with 'export const fetchCache = 'only-no-store'` + ) + } + cacheReason = 'fetchCache = only-no-store' + break + } + case 'only-cache': { + if (_cache === 'no-store') { + throw new Error( + `cache: 'no-store' used on fetch for ${fetchUrl} with 'export const fetchCache = 'only-cache'` + ) + } + break + } + case 'force-cache': { + if (typeof curRevalidate === 'undefined' || curRevalidate === 0) { + cacheReason = 'fetchCache = force-cache' + revalidate = false + } + break + } + default: + // sometimes we won't match the above cases. the reason we don't move + // everything to this switch is the use of autoNoCache which is not a fetchCacheMode + // I suspect this could be unified with fetchCacheMode however in which case we could + // simplify the switch case and ensure we have an exhaustive switch handling all modes } if (typeof revalidate === 'undefined') { - if (isDefaultCache) { + if (fetchCacheMode === 'default-cache') { revalidate = false cacheReason = 'fetchCache = default-cache' } else if (autoNoCache) { revalidate = 0 cacheReason = 'auto no cache' - } else if (isDefaultNoStore) { + } else if (fetchCacheMode === 'default-no-store') { revalidate = 0 cacheReason = 'fetchCache = default-no-store' } else if (isUsingNoStore) { @@ -424,7 +425,7 @@ export function patchFetch({ // If we were setting the revalidate value to 0, we should try to // postpone instead first. if (revalidate === 0) { - staticGenerationStore.postpone?.('revalidate: 0') + trackDynamicFetch(staticGenerationStore, 'revalidate: 0') } staticGenerationStore.revalidate = revalidate @@ -640,7 +641,7 @@ export function patchFetch({ }` // If enabled, we should bail out of static generation. - staticGenerationStore.postpone?.(dynamicUsageReason) + trackDynamicFetch(staticGenerationStore, dynamicUsageReason) // PPR is not enabled, or React postpone is not available, we // should set the revalidate to 0. @@ -671,7 +672,7 @@ export function patchFetch({ }` // If enabled, we should bail out of static generation. - staticGenerationStore.postpone?.(dynamicUsageReason) + trackDynamicFetch(staticGenerationStore, dynamicUsageReason) const err = new DynamicServerError(dynamicUsageReason) staticGenerationStore.dynamicUsageErr = err diff --git a/packages/next/src/server/web/exports/revalidate-path.ts b/packages/next/src/server/web/exports/revalidate-path.ts index 66d69a335792d..440dcee3884c7 100644 --- a/packages/next/src/server/web/exports/revalidate-path.ts +++ b/packages/next/src/server/web/exports/revalidate-path.ts @@ -1,2 +1,2 @@ // This file is for modularized imports for next/server to get fully-treeshaking. -export { revalidatePath as default } from '../spec-extension/revalidate-path' +export { revalidatePath as default } from '../spec-extension/revalidate' diff --git a/packages/next/src/server/web/exports/revalidate-tag.ts b/packages/next/src/server/web/exports/revalidate-tag.ts index f6cdfffe8b527..d99655a5ce425 100644 --- a/packages/next/src/server/web/exports/revalidate-tag.ts +++ b/packages/next/src/server/web/exports/revalidate-tag.ts @@ -1,2 +1,2 @@ // This file is for modularized imports for next/server to get fully-treeshaking. -export { revalidateTag as default } from '../spec-extension/revalidate-tag' +export { revalidateTag as default } from '../spec-extension/revalidate' diff --git a/packages/next/src/server/web/spec-extension/revalidate-path.ts b/packages/next/src/server/web/spec-extension/revalidate-path.ts deleted file mode 100644 index c8c7c6e2fbce1..0000000000000 --- a/packages/next/src/server/web/spec-extension/revalidate-path.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { revalidateTag } from './revalidate-tag' -import { isDynamicRoute } from '../../../shared/lib/router/utils' -import { - NEXT_CACHE_IMPLICIT_TAG_ID, - NEXT_CACHE_SOFT_TAG_MAX_LENGTH, -} from '../../../lib/constants' - -export function revalidatePath(originalPath: string, type?: 'layout' | 'page') { - if (originalPath.length > NEXT_CACHE_SOFT_TAG_MAX_LENGTH) { - console.warn( - `Warning: revalidatePath received "${originalPath}" which exceeded max length of ${NEXT_CACHE_SOFT_TAG_MAX_LENGTH}. See more info here https://nextjs.org/docs/app/api-reference/functions/revalidatePath` - ) - return - } - - let normalizedPath = `${NEXT_CACHE_IMPLICIT_TAG_ID}${originalPath}` - - if (type) { - normalizedPath += `${normalizedPath.endsWith('/') ? '' : '/'}${type}` - } else if (isDynamicRoute(originalPath)) { - console.warn( - `Warning: a dynamic page path "${originalPath}" was passed to "revalidatePath" without the "page" argument. This has no affect by default, see more info here https://nextjs.org/docs/app/api-reference/functions/revalidatePath` - ) - } - return revalidateTag(normalizedPath) -} diff --git a/packages/next/src/server/web/spec-extension/revalidate-tag.ts b/packages/next/src/server/web/spec-extension/revalidate-tag.ts deleted file mode 100644 index 2b3a7734561b2..0000000000000 --- a/packages/next/src/server/web/spec-extension/revalidate-tag.ts +++ /dev/null @@ -1,43 +0,0 @@ -import type { - StaticGenerationAsyncStorage, - StaticGenerationStore, -} from '../../../client/components/static-generation-async-storage.external' -import { staticGenerationBailout } from '../../../client/components/static-generation-bailout' - -export function revalidateTag(tag: string) { - const staticGenerationAsyncStorage = ( - fetch as any - ).__nextGetStaticStore?.() as undefined | StaticGenerationAsyncStorage - - const store: undefined | StaticGenerationStore = - staticGenerationAsyncStorage?.getStore() - - if (!store || !store.incrementalCache) { - throw new Error( - `Invariant: static generation store missing in revalidateTag ${tag}` - ) - } - - // a route that makes use of revalidation APIs should be considered dynamic - // as otherwise it would be impossible to revalidate - staticGenerationBailout(`revalidateTag ${tag}`) - - if (!store.revalidatedTags) { - store.revalidatedTags = [] - } - if (!store.revalidatedTags.includes(tag)) { - store.revalidatedTags.push(tag) - } - - if (!store.pendingRevalidates) { - store.pendingRevalidates = {} - } - store.pendingRevalidates[tag] = store.incrementalCache - .revalidateTag?.(tag) - .catch((err) => { - console.error(`revalidateTag failed for ${tag}`, err) - }) - - // TODO: only revalidate if the path matches - store.pathWasRevalidated = true -} diff --git a/packages/next/src/server/web/spec-extension/revalidate.ts b/packages/next/src/server/web/spec-extension/revalidate.ts new file mode 100644 index 0000000000000..edc9621638e06 --- /dev/null +++ b/packages/next/src/server/web/spec-extension/revalidate.ts @@ -0,0 +1,78 @@ +import type { + StaticGenerationAsyncStorage, + StaticGenerationStore, +} from '../../../client/components/static-generation-async-storage.external' +import { trackDynamicDataAccessed } from '../../app-render/dynamic-rendering' +import { isDynamicRoute } from '../../../shared/lib/router/utils' +import { + NEXT_CACHE_IMPLICIT_TAG_ID, + NEXT_CACHE_SOFT_TAG_MAX_LENGTH, +} from '../../../lib/constants' + +export function revalidateTag(tag: string) { + return revalidate(tag, `revalidateTag ${tag}`) +} + +export function revalidatePath(originalPath: string, type?: 'layout' | 'page') { + if (originalPath.length > NEXT_CACHE_SOFT_TAG_MAX_LENGTH) { + console.warn( + `Warning: revalidatePath received "${originalPath}" which exceeded max length of ${NEXT_CACHE_SOFT_TAG_MAX_LENGTH}. See more info here https://nextjs.org/docs/app/api-reference/functions/revalidatePath` + ) + return + } + + let normalizedPath = `${NEXT_CACHE_IMPLICIT_TAG_ID}${originalPath}` + + if (type) { + normalizedPath += `${normalizedPath.endsWith('/') ? '' : '/'}${type}` + } else if (isDynamicRoute(originalPath)) { + console.warn( + `Warning: a dynamic page path "${originalPath}" was passed to "revalidatePath" without the "page" argument. This has no affect by default, see more info here https://nextjs.org/docs/app/api-reference/functions/revalidatePath` + ) + } + return revalidate(normalizedPath, `revalidatePath ${originalPath}`) +} + +function revalidate(tag: string, expression: string) { + const staticGenerationAsyncStorage = ( + fetch as any + ).__nextGetStaticStore?.() as undefined | StaticGenerationAsyncStorage + + const store: undefined | StaticGenerationStore = + staticGenerationAsyncStorage?.getStore() + + if (!store || !store.incrementalCache) { + throw new Error( + `Invariant: static generation store missing in ${expression}` + ) + } + + if (store.isUnstableCacheCallback) { + throw new Error( + `used "${expression}" inside a function cached with "unstable_cache(...)" which is unsupported. To ensure revalidation is performed consistently it must always happen outside of renders and cached functions. See more info here: https://nextjs.org/docs/app/building-your-application/rendering/static-and-dynamic#dynamic-rendering` + ) + } + + // a route that makes use of revalidation APIs should be considered dynamic + // as otherwise it would be impossible to revalidate + trackDynamicDataAccessed(store, expression) + + if (!store.revalidatedTags) { + store.revalidatedTags = [] + } + if (!store.revalidatedTags.includes(tag)) { + store.revalidatedTags.push(tag) + } + + if (!store.pendingRevalidates) { + store.pendingRevalidates = {} + } + store.pendingRevalidates[tag] = store.incrementalCache + .revalidateTag?.(tag) + .catch((err) => { + console.error(`revalidate failed for ${tag}`, err) + }) + + // TODO: only revalidate if the path matches + store.pathWasRevalidated = true +} diff --git a/packages/next/src/server/web/spec-extension/unstable-cache.ts b/packages/next/src/server/web/spec-extension/unstable-cache.ts index 8533fa3fa7909..c5ed9587f3e09 100644 --- a/packages/next/src/server/web/spec-extension/unstable-cache.ts +++ b/packages/next/src/server/web/spec-extension/unstable-cache.ts @@ -301,7 +301,7 @@ export function unstable_cache( isUnstableCacheCallback: true, urlPathname: '/', isStaticGeneration: false, - postpone: undefined, + prerenderState: null, }, cb, ...args diff --git a/packages/next/src/server/web/spec-extension/unstable-no-store.ts b/packages/next/src/server/web/spec-extension/unstable-no-store.ts index 50484387d7a3a..e09e38a89668b 100644 --- a/packages/next/src/server/web/spec-extension/unstable-no-store.ts +++ b/packages/next/src/server/web/spec-extension/unstable-no-store.ts @@ -1,20 +1,26 @@ import { staticGenerationAsyncStorage } from '../../../client/components/static-generation-async-storage.external' -import { staticGenerationBailout } from '../../../client/components/static-generation-bailout' +import { markCurrentScopeAsDynamic } from '../../app-render/dynamic-rendering' +/** + * Expects to be called in an App Router render and will error if not. + * + * marks the current scope as dynamic. In non PPR cases this will make a static render + * halt and mark the page as dynamic. In PPR cases this will postpone the render at this location. + * + * If we are inside a cache scope then this function is a noop + */ export function unstable_noStore() { - const staticGenerationStore = staticGenerationAsyncStorage.getStore() - - if (staticGenerationStore?.isUnstableCacheCallback) { - // if called within a next/cache call, we want to cache the result - // and defer to the next/cache call to handle how to cache the result. + const callingExpression = 'unstable_noStore()' + const store = staticGenerationAsyncStorage.getStore() + if (!store) { + // This generally implies we are being called in Pages router. We should probably not support + // unstable_noStore in contexts outside of `react-server` condition but since we historically + // have not errored here previously, we maintain that behavior for now. return + } else if (store.forceStatic) { + return + } else { + store.isUnstableNoStore = true + markCurrentScopeAsDynamic(store, callingExpression) } - - // Mark the static generation context has unstable_noStore - if (staticGenerationStore) { - staticGenerationStore.isUnstableNoStore = true - } - staticGenerationBailout('unstable_noStore', { - link: 'https://nextjs.org/docs/app/building-your-application/rendering/static-and-dynamic#dynamic-rendering', - }) } diff --git a/test/development/acceptance-app/rsc-runtime-errors.test.ts b/test/development/acceptance-app/rsc-runtime-errors.test.ts index 9a506afd556dc..9a55e98ef8d30 100644 --- a/test/development/acceptance-app/rsc-runtime-errors.test.ts +++ b/test/development/acceptance-app/rsc-runtime-errors.test.ts @@ -74,7 +74,7 @@ createNextDescribe( const errorDescription = await getRedboxDescription(browser) expect(errorDescription).toContain( - `Error: Invariant: cookies() expects to have requestAsyncStorage, none available.` + `Error: Invariant: \`cookies\` expects to have requestAsyncStorage, none available.` ) }) From b8f40bc685a78a217773615867ebad7db6631e51 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Wed, 17 Jan 2024 15:31:13 -0800 Subject: [PATCH 2/4] Adds tests to cover some of the new erroring semantics around dynamically tracked data reads --- .../app-dir/dynamic-data/dynamic-data.test.ts | 294 ++++++++++++++++++ .../fixtures/cache-scoped/app/cookies/page.js | 35 +++ .../fixtures/cache-scoped/app/headers/page.js | 29 ++ .../fixtures/cache-scoped/app/layout.js | 23 ++ .../fixtures/main/app/client-page/page.js | 34 ++ .../fixtures/main/app/force-dynamic/page.js | 59 ++++ .../fixtures/main/app/force-static/page.js | 59 ++++ .../dynamic-data/fixtures/main/app/layout.js | 25 ++ .../fixtures/main/app/setenv/route.js | 4 + .../fixtures/main/app/top-level/page.js | 57 ++++ .../require-static/app/cookies/page.js | 33 ++ .../require-static/app/headers/page.js | 26 ++ .../fixtures/require-static/app/layout.js | 23 ++ .../require-static/app/search/page.js | 23 ++ 14 files changed, 724 insertions(+) create mode 100644 test/e2e/app-dir/dynamic-data/dynamic-data.test.ts create mode 100644 test/e2e/app-dir/dynamic-data/fixtures/cache-scoped/app/cookies/page.js create mode 100644 test/e2e/app-dir/dynamic-data/fixtures/cache-scoped/app/headers/page.js create mode 100644 test/e2e/app-dir/dynamic-data/fixtures/cache-scoped/app/layout.js create mode 100644 test/e2e/app-dir/dynamic-data/fixtures/main/app/client-page/page.js create mode 100644 test/e2e/app-dir/dynamic-data/fixtures/main/app/force-dynamic/page.js create mode 100644 test/e2e/app-dir/dynamic-data/fixtures/main/app/force-static/page.js create mode 100644 test/e2e/app-dir/dynamic-data/fixtures/main/app/layout.js create mode 100644 test/e2e/app-dir/dynamic-data/fixtures/main/app/setenv/route.js create mode 100644 test/e2e/app-dir/dynamic-data/fixtures/main/app/top-level/page.js create mode 100644 test/e2e/app-dir/dynamic-data/fixtures/require-static/app/cookies/page.js create mode 100644 test/e2e/app-dir/dynamic-data/fixtures/require-static/app/headers/page.js create mode 100644 test/e2e/app-dir/dynamic-data/fixtures/require-static/app/layout.js create mode 100644 test/e2e/app-dir/dynamic-data/fixtures/require-static/app/search/page.js diff --git a/test/e2e/app-dir/dynamic-data/dynamic-data.test.ts b/test/e2e/app-dir/dynamic-data/dynamic-data.test.ts new file mode 100644 index 0000000000000..bcb03c9235456 --- /dev/null +++ b/test/e2e/app-dir/dynamic-data/dynamic-data.test.ts @@ -0,0 +1,294 @@ +import { createNextDescribe } from 'e2e-utils' +import { getRedboxHeader, hasRedbox } from 'next-test-utils' + +process.env.__TEST_SENTINEL = 'build' + +createNextDescribe( + 'dynamic-data', + { + files: __dirname + '/fixtures/main', + skipStart: true, + }, + ({ next, isNextDev, isNextDeploy }) => { + if (isNextDeploy) { + it.skip('should not run in next deploy', () => {}) + return + } + + beforeAll(async () => { + await next.start() + // This will update the __TEST_SENTINEL value to "run" + await next.render('/setenv?value=run') + }) + + it('should render the dynamic apis dynamically when used in a top-level scope', async () => { + const $ = await next.render$( + '/top-level?foo=foosearch', + {}, + { + headers: { + fooheader: 'foo header value', + cookie: 'foocookie=foo cookie value', + }, + } + ) + if (isNextDev) { + // in dev we expect the entire page to be rendered at runtime + expect($('#layout').text()).toBe('run') + expect($('#page').text()).toBe('run') + // we expect there to be no susupense boundary in fallback state + expect($('#boundary').html()).toBeNull() + } else if (process.env.__NEXT_EXPERIMENTAL_PPR) { + // in PPR we expect the shell to be rendered at build and the page to be rendered at runtime + expect($('#layout').text()).toBe('build') + expect($('#page').text()).toBe('run') + // we expect there to be a suspense boundary in fallback state + expect($('#boundary').html()).not.toBeNull() + } else { + // in static generation we expect the entire page to be rendered at runtime + expect($('#layout').text()).toBe('run') + expect($('#page').text()).toBe('run') + // we expect there to be no susupense boundary in fallback state + expect($('#boundary').html()).toBeNull() + } + + expect($('#headers .fooheader').text()).toBe('foo header value') + expect($('#cookies .foocookie').text()).toBe('foo cookie value') + expect($('#searchparams .foo').text()).toBe('foosearch') + }) + + it('should render the dynamic apis dynamically when used in a top-level scope with force dynamic', async () => { + const $ = await next.render$( + '/force-dynamic?foo=foosearch', + {}, + { + headers: { + fooheader: 'foo header value', + cookie: 'foocookie=foo cookie value', + }, + } + ) + if (isNextDev) { + // in dev we expect the entire page to be rendered at runtime + expect($('#layout').text()).toBe('run') + expect($('#page').text()).toBe('run') + // we expect there to be no susupense boundary in fallback state + expect($('#boundary').html()).toBeNull() + } else if (process.env.__NEXT_EXPERIMENTAL_PPR) { + // in PPR with force + // @TODO this should actually be build but there is a bug in how we do segment level dynamic in PPR at the moment + // see not in create-component-tree + expect($('#layout').text()).toBe('run') + expect($('#page').text()).toBe('run') + // we expect there to be a suspense boundary in fallback state + expect($('#boundary').html()).toBeNull() + } else { + // in static generation we expect the entire page to be rendered at runtime + expect($('#layout').text()).toBe('run') + expect($('#page').text()).toBe('run') + // we expect there to be no susupense boundary in fallback state + expect($('#boundary').html()).toBeNull() + } + + expect($('#headers .fooheader').text()).toBe('foo header value') + expect($('#cookies .foocookie').text()).toBe('foo cookie value') + expect($('#searchparams .foo').text()).toBe('foosearch') + }) + + it('should render empty objects for dynamic APIs when rendering with force-static', async () => { + const $ = await next.render$( + '/force-static?foo=foosearch', + {}, + { + headers: { + fooheader: 'foo header value', + cookie: 'foocookie=foo cookie value', + }, + } + ) + if (isNextDev) { + // in dev we expect the entire page to be rendered at runtime + expect($('#layout').text()).toBe('run') + expect($('#page').text()).toBe('run') + // we expect there to be no susupense boundary in fallback state + expect($('#boundary').html()).toBeNull() + } else if (process.env.__NEXT_EXPERIMENTAL_PPR) { + // in PPR we expect the shell to be rendered at build and the page to be rendered at runtime + expect($('#layout').text()).toBe('build') + expect($('#page').text()).toBe('build') + // we expect there to be a suspense boundary in fallback state + expect($('#boundary').html()).toBeNull() + } else { + // in static generation we expect the entire page to be rendered at runtime + expect($('#layout').text()).toBe('build') + expect($('#page').text()).toBe('build') + // we expect there to be no susupense boundary in fallback state + expect($('#boundary').html()).toBeNull() + } + + expect($('#headers .fooheader').html()).toBeNull() + expect($('#cookies .foocookie').html()).toBeNull() + expect($('#searchparams .foo').html()).toBeNull() + }) + + it('should track searchParams access as dynamic when the Page is a client component', async () => { + const $ = await next.render$( + '/client-page?foo=foosearch', + {}, + { + headers: { + fooheader: 'foo header value', + cookie: 'foocookie=foo cookie value', + }, + } + ) + if (isNextDev) { + // in dev we expect the entire page to be rendered at runtime + expect($('#layout').text()).toBe('run') + expect($('#page').text()).toBe('run') + // we don't assert the state of the fallback because it can depend on the timing + // of when streaming starts and how fast the client references resolve + } else if (process.env.__NEXT_EXPERIMENTAL_PPR) { + // in PPR we expect the shell to be rendered at build and the page to be rendered at runtime + expect($('#layout').text()).toBe('build') + expect($('#page').text()).toBe('run') + // we expect there to be a suspense boundary in fallback state + expect($('#boundary').html()).not.toBeNull() + } else { + // in static generation we expect the entire page to be rendered at runtime + expect($('#layout').text()).toBe('run') + expect($('#page').text()).toBe('run') + // we don't assert the state of the fallback because it can depend on the timing + // of when streaming starts and how fast the client references resolve + } + + expect($('#searchparams .foo').text()).toBe('foosearch') + }) + } +) + +createNextDescribe( + 'dynamic-data with dynamic = "error"', + { + files: __dirname + '/fixtures/require-static', + skipStart: true, + }, + ({ next, isNextDev, isNextDeploy }) => { + if (isNextDeploy) { + it.skip('should not run in next deploy.', () => {}) + return + } + + if (isNextDev) { + beforeAll(async () => { + await next.start() + }) + + it('displays redbox when `dynamic = "error"` and dynamic data is read in dev', async () => { + let browser = await next.browser('/cookies?foo=foosearch') + try { + expect(await hasRedbox(browser)).toBe(true) + expect(await getRedboxHeader(browser)).toMatch( + 'Error: Page with `dynamic = "error"` couldn\'t be rendered statically because it used `cookies`' + ) + } finally { + await browser.close() + } + + browser = await next.browser('/headers?foo=foosearch') + try { + expect(await hasRedbox(browser)).toBe(true) + expect(await getRedboxHeader(browser)).toMatch( + 'Error: Page with `dynamic = "error"` couldn\'t be rendered statically because it used `headers`' + ) + } finally { + await browser.close() + } + + browser = await next.browser('/search?foo=foosearch') + try { + expect(await hasRedbox(browser)).toBe(true) + expect(await getRedboxHeader(browser)).toMatch( + 'Error: Page with `dynamic = "error"` couldn\'t be rendered statically because it used `searchParams`' + ) + } finally { + await browser.close() + } + }) + } else { + it('error when the build when `dynamic = "error"` and dynamic data is read', async () => { + try { + await next.start() + } catch (err) { + // We expect this to fail + } + // Error: Page with `dynamic = "error"` couldn't be rendered statically because it used `headers` + expect(next.cliOutput).toMatch( + 'Error: Page with `dynamic = "error"` couldn\'t be rendered statically because it used `cookies`' + ) + expect(next.cliOutput).toMatch( + 'Error: Page with `dynamic = "error"` couldn\'t be rendered statically because it used `headers`' + ) + expect(next.cliOutput).toMatch( + 'Error: Page with `dynamic = "error"` couldn\'t be rendered statically because it used `searchParams`.' + ) + }) + } + } +) + +createNextDescribe( + 'dynamic-data inside cache scope', + { + files: __dirname + '/fixtures/cache-scoped', + skipStart: true, + }, + ({ next, isNextDev, isNextDeploy }) => { + if (isNextDeploy) { + it.skip('should not run in next deploy..', () => {}) + return + } + + if (isNextDev) { + beforeAll(async () => { + await next.start() + }) + + it('displays redbox when accessing dynamic data inside a cache scope', async () => { + let browser = await next.browser('/cookies') + try { + expect(await hasRedbox(browser)).toBe(true) + expect(await getRedboxHeader(browser)).toMatch( + 'Error: used "cookies" inside a function cached with "unstable_cache(...)".' + ) + } finally { + await browser.close() + } + + browser = await next.browser('/headers') + try { + expect(await hasRedbox(browser)).toBe(true) + expect(await getRedboxHeader(browser)).toMatch( + 'Error: used "headers" inside a function cached with "unstable_cache(...)".' + ) + } finally { + await browser.close() + } + }) + } else { + it('error when the build when accessing dynamic data inside a cache scope', async () => { + try { + await next.start() + } catch (err) { + // We expect this to fail + } + expect(next.cliOutput).toMatch( + 'Error: used "cookies" inside a function cached with "unstable_cache(...)".' + ) + expect(next.cliOutput).toMatch( + 'Error: used "headers" inside a function cached with "unstable_cache(...)".' + ) + }) + } + } +) diff --git a/test/e2e/app-dir/dynamic-data/fixtures/cache-scoped/app/cookies/page.js b/test/e2e/app-dir/dynamic-data/fixtures/cache-scoped/app/cookies/page.js new file mode 100644 index 0000000000000..4a1ecddc606eb --- /dev/null +++ b/test/e2e/app-dir/dynamic-data/fixtures/cache-scoped/app/cookies/page.js @@ -0,0 +1,35 @@ +import { cookies as nextCookies } from 'next/headers' +import { unstable_cache as cache } from 'next/cache' + +const cookies = cache(() => nextCookies()) + +export default async function Page({ searchParams }) { + console.log('cookies()', await cookies()) + return ( +
+
+ This example uses `cookies()` but is configured with `dynamic = 'error'` + which should cause the page to fail to build +
+
+

cookies

+ {cookies() + .getAll() + .map((cookie) => { + const key = cookie.name + let value = cookie.value + + if (key === 'userCache') { + value = value.slice(0, 10) + '...' + } + return ( +
+

{key}

+ 
{value}
+
+ ) + })} +
+
+ ) +} diff --git a/test/e2e/app-dir/dynamic-data/fixtures/cache-scoped/app/headers/page.js b/test/e2e/app-dir/dynamic-data/fixtures/cache-scoped/app/headers/page.js new file mode 100644 index 0000000000000..148312a50a2b9 --- /dev/null +++ b/test/e2e/app-dir/dynamic-data/fixtures/cache-scoped/app/headers/page.js @@ -0,0 +1,29 @@ +import { headers as nextHeaders } from 'next/headers' +import { unstable_cache as cache } from 'next/cache' + +const headers = cache(() => nextHeaders()) + +export default async function Page() { + return ( +
+
+ This example uses `headers()` but is configured with `dynamic = 'error'` + which should cause the page to fail to build +
+
+

headers

+ {Array.from(await headers()) + .entries() + .map(([key, value]) => { + if (key === 'cookie') return null + return ( +
+

{key}

+ 
{value}
+
+ ) + })} +
+
+ ) +} diff --git a/test/e2e/app-dir/dynamic-data/fixtures/cache-scoped/app/layout.js b/test/e2e/app-dir/dynamic-data/fixtures/cache-scoped/app/layout.js new file mode 100644 index 0000000000000..6dfbbdf73384e --- /dev/null +++ b/test/e2e/app-dir/dynamic-data/fixtures/cache-scoped/app/layout.js @@ -0,0 +1,23 @@ +import { Suspense } from 'react' + +export default async function Layout({ children }) { + return ( + + + app-dynamic-data + + +

+ This test fixture helps us assert that accessing dynamic data in + various scopes and with various `dynamic` configurations works as + intended +

+
+ loading...}> + {children} + +
+ + + ) +} diff --git a/test/e2e/app-dir/dynamic-data/fixtures/main/app/client-page/page.js b/test/e2e/app-dir/dynamic-data/fixtures/main/app/client-page/page.js new file mode 100644 index 0000000000000..6f996293c640b --- /dev/null +++ b/test/e2e/app-dir/dynamic-data/fixtures/main/app/client-page/page.js @@ -0,0 +1,34 @@ +'use client' + +export default async function Page({ searchParams }) { + const { __TEST_SENTINEL } = process.env + return ( +
+
{__TEST_SENTINEL}
+
+ This example uses headers/cookies/searchParams directly in a Page + configured with `dynamic = 'force-dynamic'`. This should cause the page + to always render dynamically regardless of dynamic APIs used +
+
+

headers

+

This is a client Page so `headers()` is not available

+
+
+

cookies

{' '} +

This is a client Page so `cookies()` is not available

+
+
+

searchParams

+ {Object.entries(searchParams).map(([key, value]) => { + return ( +
+

{key}

+ 
{value}
+
+ ) + })} +
+
+ ) +} diff --git a/test/e2e/app-dir/dynamic-data/fixtures/main/app/force-dynamic/page.js b/test/e2e/app-dir/dynamic-data/fixtures/main/app/force-dynamic/page.js new file mode 100644 index 0000000000000..2600585338d87 --- /dev/null +++ b/test/e2e/app-dir/dynamic-data/fixtures/main/app/force-dynamic/page.js @@ -0,0 +1,59 @@ +import { headers, cookies } from 'next/headers' + +export const dynamic = 'force-dynamic' + +export default async function Page({ searchParams }) { + const { __TEST_SENTINEL } = process.env + return ( +
+
{__TEST_SENTINEL}
+
+ This example uses headers/cookies/searchParams directly in a Page + configured with `dynamic = 'force-dynamic'`. This should cause the page + to always render dynamically regardless of dynamic APIs used +
+
+

headers

+ {Array.from(headers().entries()).map(([key, value]) => { + if (key === 'cookie') return null + return ( +
+

{key}

+ 
{value}
+
+ ) + })} +
+
+

cookies

+ {cookies() + .getAll() + .map((cookie) => { + const key = cookie.name + let value = cookie.value + + if (key === 'userCache') { + value = value.slice(0, 10) + '...' + } + return ( +
+

{key}

+ 
{value}
+
+ ) + })} +
+
+

searchParams

+ {Object.entries(searchParams).map(([key, value]) => { + return ( +
+

{key}

+ 
{value}
+
+ ) + })} +
+
+ ) +} diff --git a/test/e2e/app-dir/dynamic-data/fixtures/main/app/force-static/page.js b/test/e2e/app-dir/dynamic-data/fixtures/main/app/force-static/page.js new file mode 100644 index 0000000000000..386920e307fbc --- /dev/null +++ b/test/e2e/app-dir/dynamic-data/fixtures/main/app/force-static/page.js @@ -0,0 +1,59 @@ +import { headers, cookies } from 'next/headers' + +export const dynamic = 'force-static' + +export default async function Page({ searchParams }) { + const { __TEST_SENTINEL } = process.env + return ( +
+
{__TEST_SENTINEL}
+
+ This example uses headers/cookies/searchParams directly in a Page + configured with `dynamic = 'force-static'`. This should cause the page + to always statically render but without exposing dynamic data +
+
+

headers

+ {Array.from(headers().entries()).map(([key, value]) => { + if (key === 'cookie') return null + return ( +
+

{key}

+ 
{value}
+
+ ) + })} +
+
+

cookies

+ {cookies() + .getAll() + .map((cookie) => { + const key = cookie.name + let value = cookie.value + + if (key === 'userCache') { + value = value.slice(0, 10) + '...' + } + return ( +
+

{key}

+ 
{value}
+
+ ) + })} +
+
+

searchParams

+ {Object.entries(searchParams).map(([key, value]) => { + return ( +
+

{key}

+ 
{value}
+
+ ) + })} +
+
+ ) +} diff --git a/test/e2e/app-dir/dynamic-data/fixtures/main/app/layout.js b/test/e2e/app-dir/dynamic-data/fixtures/main/app/layout.js new file mode 100644 index 0000000000000..0146d8e26a029 --- /dev/null +++ b/test/e2e/app-dir/dynamic-data/fixtures/main/app/layout.js @@ -0,0 +1,25 @@ +import { Suspense } from 'react' + +export default async function Layout({ children }) { + const { __TEST_SENTINEL } = process.env + return ( + + + app-dynamic-data + + +

+ This test fixture helps us assert that accessing dynamic data in + various scopes and with various `dynamic` configurations works as + intended +

+
+
{__TEST_SENTINEL}
+ loading...}> + {children} + +
+ + + ) +} diff --git a/test/e2e/app-dir/dynamic-data/fixtures/main/app/setenv/route.js b/test/e2e/app-dir/dynamic-data/fixtures/main/app/setenv/route.js new file mode 100644 index 0000000000000..b5f2c27c1a19d --- /dev/null +++ b/test/e2e/app-dir/dynamic-data/fixtures/main/app/setenv/route.js @@ -0,0 +1,4 @@ +export async function GET(request) { + process.env.__TEST_SENTINEL = request.nextUrl.searchParams.get('value') + return new Response('ok') +} diff --git a/test/e2e/app-dir/dynamic-data/fixtures/main/app/top-level/page.js b/test/e2e/app-dir/dynamic-data/fixtures/main/app/top-level/page.js new file mode 100644 index 0000000000000..5c84fa636af01 --- /dev/null +++ b/test/e2e/app-dir/dynamic-data/fixtures/main/app/top-level/page.js @@ -0,0 +1,57 @@ +import { headers, cookies } from 'next/headers' + +export default async function Page({ searchParams }) { + const { __TEST_SENTINEL } = process.env + return ( +
+
{__TEST_SENTINEL}
+
+ This example uses headers/cookies/searchParams directly. In static + generation we'd expect this to bail out to dynamic. In PPR we expect + this to partially render the root layout only +
+
+

headers

+ {Array.from(headers().entries()).map(([key, value]) => { + if (key === 'cookie') return null + return ( +
+

{key}

+ 
{value}
+
+ ) + })} +
+
+

cookies

+ {cookies() + .getAll() + .map((cookie) => { + const key = cookie.name + let value = cookie.value + + if (key === 'userCache') { + value = value.slice(0, 10) + '...' + } + return ( +
+

{key}

+ 
{value}
+
+ ) + })} +
+
+

searchParams

+ {Object.entries(searchParams).map(([key, value]) => { + return ( +
+

{key}

+ 
{value}
+
+ ) + })} +
+
+ ) +} diff --git a/test/e2e/app-dir/dynamic-data/fixtures/require-static/app/cookies/page.js b/test/e2e/app-dir/dynamic-data/fixtures/require-static/app/cookies/page.js new file mode 100644 index 0000000000000..e5c3cf5147e3b --- /dev/null +++ b/test/e2e/app-dir/dynamic-data/fixtures/require-static/app/cookies/page.js @@ -0,0 +1,33 @@ +import { cookies } from 'next/headers' + +export const dynamic = 'error' + +export default async function Page({ searchParams }) { + return ( +
+
+ This example uses `cookies()` but is configured with `dynamic = 'error'` + which should cause the page to fail to build +
+
+

cookies

+ {cookies() + .getAll() + .map((cookie) => { + const key = cookie.name + let value = cookie.value + + if (key === 'userCache') { + value = value.slice(0, 10) + '...' + } + return ( +
+

{key}

+ 
{value}
+
+ ) + })} +
+
+ ) +} diff --git a/test/e2e/app-dir/dynamic-data/fixtures/require-static/app/headers/page.js b/test/e2e/app-dir/dynamic-data/fixtures/require-static/app/headers/page.js new file mode 100644 index 0000000000000..1b3486550f816 --- /dev/null +++ b/test/e2e/app-dir/dynamic-data/fixtures/require-static/app/headers/page.js @@ -0,0 +1,26 @@ +import { headers } from 'next/headers' + +export const dynamic = 'error' + +export default async function Page() { + return ( +
+
+ This example uses `headers()` but is configured with `dynamic = 'error'` + which should cause the page to fail to build +
+
+

headers

+ {Array.from(headers().entries()).map(([key, value]) => { + if (key === 'cookie') return null + return ( +
+

{key}

+ 
{value}
+
+ ) + })} +
+
+ ) +} diff --git a/test/e2e/app-dir/dynamic-data/fixtures/require-static/app/layout.js b/test/e2e/app-dir/dynamic-data/fixtures/require-static/app/layout.js new file mode 100644 index 0000000000000..6dfbbdf73384e --- /dev/null +++ b/test/e2e/app-dir/dynamic-data/fixtures/require-static/app/layout.js @@ -0,0 +1,23 @@ +import { Suspense } from 'react' + +export default async function Layout({ children }) { + return ( + + + app-dynamic-data + + +

+ This test fixture helps us assert that accessing dynamic data in + various scopes and with various `dynamic` configurations works as + intended +

+
+ loading...}> + {children} + +
+ + + ) +} diff --git a/test/e2e/app-dir/dynamic-data/fixtures/require-static/app/search/page.js b/test/e2e/app-dir/dynamic-data/fixtures/require-static/app/search/page.js new file mode 100644 index 0000000000000..c87718537996b --- /dev/null +++ b/test/e2e/app-dir/dynamic-data/fixtures/require-static/app/search/page.js @@ -0,0 +1,23 @@ +export const dynamic = 'error' + +export default async function Page({ searchParams }) { + return ( +
+
+ This example uses `searchParams` but is configured with `dynamic = + 'error'` which should cause the page to fail to build +
+
+

searchParams

+ {Object.entries(searchParams).map(([key, value]) => { + return ( +
+

{key}

+ 
{value}
+
+ ) + })} +
+
+ ) +} From d59e4b885b7e2d046d40baac3b93ebe685c7dfe6 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Thu, 18 Jan 2024 08:14:29 -0800 Subject: [PATCH 3/4] upstream mdx change needed formatting --- .../next/src/client/components/headers.ts | 4 +-- .../src/client/components/search-params.ts | 2 +- .../next/src/server/app-render/app-render.tsx | 1 - .../app-render/create-component-tree.tsx | 18 +++------- .../server/app-render/dynamic-rendering.ts | 36 ++++++++++++++----- .../src/server/app-render/rsc/postpone.ts | 19 +--------- .../app-dir/dynamic-data/dynamic-data.test.ts | 23 +++++------- 7 files changed, 45 insertions(+), 58 deletions(-) diff --git a/packages/next/src/client/components/headers.ts b/packages/next/src/client/components/headers.ts index 404c86df2f6e9..ed461974a1c2d 100644 --- a/packages/next/src/client/components/headers.ts +++ b/packages/next/src/client/components/headers.ts @@ -16,7 +16,7 @@ export function headers() { if (staticGenerationStore) { if (staticGenerationStore.forceStatic) { - // When we are forcings static we don't mark this as a Dynamic read and we return an empty headers object + // When we are forcing static we don't mark this as a Dynamic read and we return an empty headers object return HeadersAdapter.seal(new Headers({})) } else { // We will return a real headers object below so we mark this call as reading from a dynamic data source @@ -34,7 +34,7 @@ export function cookies() { if (staticGenerationStore) { if (staticGenerationStore.forceStatic) { - // When we are forcings static we don't mark this as a Dynamic read and we return an empty cookies object + // When we are forcing static we don't mark this as a Dynamic read and we return an empty cookies object return RequestCookiesAdapter.seal(new RequestCookies(new Headers({}))) } else { // We will return a real headers object below so we mark this call as reading from a dynamic data source diff --git a/packages/next/src/client/components/search-params.ts b/packages/next/src/client/components/search-params.ts index d686f41c6fda8..8cf4b608774d3 100644 --- a/packages/next/src/client/components/search-params.ts +++ b/packages/next/src/client/components/search-params.ts @@ -43,7 +43,7 @@ export function createDynamicallyTrackedSearchParams( return {} } else if (!store.isStaticGeneration && !store.dynamicShouldError) { // during dynamic renders we don't actually have to track anything so we just return - // the searchParams directly. However if we dynamic data access should error then we + // the searchParams directly. However if dynamic data access should error then we // still want to track access. This covers the case in Dev where all renders are dynamic // but we still want to error if you use a dynamic data source because it will fail the build // or revalidate if you do. diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index 7ccacfe860027..107aabb82e320 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -1224,7 +1224,6 @@ async function renderToHTMLOrFlightImpl( if ( staticGenerationStore.prerenderState && staticGenerationStore.prerenderState.hasDynamic && - // but there's no postpone state !metadata.postponed && (!response.err || !isBailoutToCSRError(response.err)) ) { diff --git a/packages/next/src/server/app-render/create-component-tree.tsx b/packages/next/src/server/app-render/create-component-tree.tsx index f4fb98ab5e24c..a795c462a8882 100644 --- a/packages/next/src/server/app-render/create-component-tree.tsx +++ b/packages/next/src/server/app-render/create-component-tree.tsx @@ -562,9 +562,9 @@ async function createComponentTreeInternal({ process.env.NODE_ENV === 'development' && 'params' in parallelRouteProps ) { - // @TODO consider making this an error and runnign the check in build as well + // @TODO consider making this an error and running the check in build as well console.error( - `The prop name "params" is reserved in Layouts and Pages and cannot be used as the name of a parallel routes in ${segment}.` + `"params" is a reserved prop in Layouts and Pages and cannot be used as the name of a parallel route in ${segment}` ) } props.params = currentParams @@ -572,19 +572,9 @@ async function createComponentTreeInternal({ let segmentElement: React.ReactNode if (isPage) { // Assign searchParams to props if this is a page - if ( - process.env.NODE_ENV === 'development' && - 'searchParams' in parallelRouteProps - ) { - // @TODO consider making this an error and runnign the check in build as well - console.error( - `The prop name "searchParams" is reserved in Layouts and Pages and cannot be used as the name of a parallel routes in ${segment}.` - ) - } - if (isClientComponent) { - // When we are passing searchParams to a client component Page we don't want to try the access - // dynamically here in the RSC layer because the serialization will trigger a dynamic API usage. + // When we are passing searchParams to a client component Page we don't want to track the dynamic access + // here in the RSC layer because the serialization will trigger a dynamic API usage. // Instead we pass the searchParams untracked but we wrap the Page in a root client component // which can among other things adds the dynamic tracking before rendering the page. // @TODO make the root wrapper part of next-app-loader so we don't need the extra client component diff --git a/packages/next/src/server/app-render/dynamic-rendering.ts b/packages/next/src/server/app-render/dynamic-rendering.ts index 98ca899d91f22..512efae5b8eda 100644 --- a/packages/next/src/server/app-render/dynamic-rendering.ts +++ b/packages/next/src/server/app-render/dynamic-rendering.ts @@ -51,9 +51,9 @@ export function markCurrentScopeAsDynamic( ) } else if ( // We are in a prerender (PPR enabled, during build) - store.prerenderState && - hasPostpone + store.prerenderState ) { + assertPostpone() // We track that we had a dynamic scope that postponed. // This will be used by the renderer to decide whether // the prerender requires a resume @@ -63,7 +63,7 @@ export function markCurrentScopeAsDynamic( store.revalidate = 0 if (store.isStaticGeneration) { - // We aren't prerendering but we are generating a static page We need to bail out of static generation + // We aren't prerendering but we are generating a static page. We need to bail out of static generation const err = new DynamicServerError( `Page couldn't be rendered statically because it used ${expression}. See more info here: https://nextjs.org/docs/messages/dynamic-server-error` ) @@ -90,7 +90,7 @@ export function trackDynamicDataAccessed( ): void { if (store.isUnstableCacheCallback) { throw new Error( - `used "${expression}" inside a function cached with "unstable_cache(...)". Accessing Dynamic data sources inside a cache scope is not supported. If you need this data inside a cached function use "${expression}" oustide the cached function and pass the required dynamic data in as an argument. See more info here: https://nextjs.org/docs/app/building-your-application/rendering/static-and-dynamic#dynamic-rendering` + `used "${expression}" inside a function cached with "unstable_cache(...)". Accessing Dynamic data sources inside a cache scope is not supported. If you need this data inside a cached function use "${expression}" oustide of the cached function and pass the required dynamic data in as an argument. See more info here: https://nextjs.org/docs/app/api-reference/functions/unstable_cache` ) } else if (store.dynamicShouldError) { throw new StaticGenBailoutError( @@ -98,9 +98,9 @@ export function trackDynamicDataAccessed( ) } else if ( // We are in a prerender (PPR enabled, during build) - store.prerenderState && - hasPostpone + store.prerenderState ) { + assertPostpone() // We track that we had a dynamic scope that postponed. // This will be used by the renderer to decide whether // the prerender requires a resume @@ -110,7 +110,7 @@ export function trackDynamicDataAccessed( store.revalidate = 0 if (store.isStaticGeneration) { - // We aren't prerendering but we are generating a static page We need to bail out of static generation + // We aren't prerendering but we are generating a static page. We need to bail out of static generation const err = new DynamicServerError( `Page couldn't be rendered statically because it used ${expression}. See more info here: https://nextjs.org/docs/messages/dynamic-server-error` ) @@ -122,6 +122,17 @@ export function trackDynamicDataAccessed( } } +/** + * This component will call `React.postpone` that throws the postponed error. + */ +type PostponeProps = { + reason: string +} +export function Postpone({ reason }: PostponeProps): never { + assertPostpone() + return React.unstable_postpone(reason) +} + // @TODO refactor patch-fetch and this function to better model dynamic semantics. Currently this implementation // is too explicit about postponing if we are in a prerender and patch-fetch contains a lot of logic for determining // what makes the fetch "dynamic". It also doesn't handle Non PPR cases so it is isn't as consistent with the other @@ -130,7 +141,8 @@ export function trackDynamicFetch( store: StaticGenerationStore, expression: string ) { - if (store.prerenderState && hasPostpone) { + if (store.prerenderState) { + assertPostpone() store.prerenderState.hasDynamic = true React.unstable_postpone(createPostponeReason(expression)) } @@ -143,3 +155,11 @@ function createPostponeReason(expression: string) { `your own try/catch. Learn more: https://nextjs.org/docs/messages/ppr-caught-error` ) } + +function assertPostpone() { + if (!hasPostpone) { + throw new Error( + `Invariant: React.unstable_postpone is not defined. This suggests the wrong version of React was loaded. This is a bug in Next.js` + ) + } +} diff --git a/packages/next/src/server/app-render/rsc/postpone.ts b/packages/next/src/server/app-render/rsc/postpone.ts index 2f40199583da4..e486523811b51 100644 --- a/packages/next/src/server/app-render/rsc/postpone.ts +++ b/packages/next/src/server/app-render/rsc/postpone.ts @@ -5,21 +5,4 @@ Files in the rsc directory are meant to be packaged as part of the RSC graph usi */ // When postpone is available in canary React we can switch to importing it directly -import React from 'react' - -const missingPostpone = typeof React.unstable_postpone !== 'function' - -/** - * This component will call `React.postpone` that throws the postponed error. - */ -type PostponeProps = { - reason: string -} -export function Postpone({ reason }: PostponeProps): never { - if (missingPostpone) { - throw new Error( - 'Invariant: Postpone component expected `postpone` to exist in React.' - ) - } - return React.unstable_postpone(reason) -} +export { Postpone } from '../dynamic-rendering' diff --git a/test/e2e/app-dir/dynamic-data/dynamic-data.test.ts b/test/e2e/app-dir/dynamic-data/dynamic-data.test.ts index bcb03c9235456..e6344391c01f4 100644 --- a/test/e2e/app-dir/dynamic-data/dynamic-data.test.ts +++ b/test/e2e/app-dir/dynamic-data/dynamic-data.test.ts @@ -8,13 +8,9 @@ createNextDescribe( { files: __dirname + '/fixtures/main', skipStart: true, + skipDeployment: true, }, - ({ next, isNextDev, isNextDeploy }) => { - if (isNextDeploy) { - it.skip('should not run in next deploy', () => {}) - return - } - + ({ next, isNextDev }) => { beforeAll(async () => { await next.start() // This will update the __TEST_SENTINEL value to "run" @@ -36,7 +32,7 @@ createNextDescribe( // in dev we expect the entire page to be rendered at runtime expect($('#layout').text()).toBe('run') expect($('#page').text()).toBe('run') - // we expect there to be no susupense boundary in fallback state + // we expect there to be no suspense boundary in fallback state expect($('#boundary').html()).toBeNull() } else if (process.env.__NEXT_EXPERIMENTAL_PPR) { // in PPR we expect the shell to be rendered at build and the page to be rendered at runtime @@ -48,7 +44,7 @@ createNextDescribe( // in static generation we expect the entire page to be rendered at runtime expect($('#layout').text()).toBe('run') expect($('#page').text()).toBe('run') - // we expect there to be no susupense boundary in fallback state + // we expect there to be no suspense boundary in fallback state expect($('#boundary').html()).toBeNull() } @@ -72,12 +68,11 @@ createNextDescribe( // in dev we expect the entire page to be rendered at runtime expect($('#layout').text()).toBe('run') expect($('#page').text()).toBe('run') - // we expect there to be no susupense boundary in fallback state + // we expect there to be no suspense boundary in fallback state expect($('#boundary').html()).toBeNull() } else if (process.env.__NEXT_EXPERIMENTAL_PPR) { - // in PPR with force // @TODO this should actually be build but there is a bug in how we do segment level dynamic in PPR at the moment - // see not in create-component-tree + // see note in create-component-tree expect($('#layout').text()).toBe('run') expect($('#page').text()).toBe('run') // we expect there to be a suspense boundary in fallback state @@ -86,7 +81,7 @@ createNextDescribe( // in static generation we expect the entire page to be rendered at runtime expect($('#layout').text()).toBe('run') expect($('#page').text()).toBe('run') - // we expect there to be no susupense boundary in fallback state + // we expect there to be no suspense boundary in fallback state expect($('#boundary').html()).toBeNull() } @@ -110,7 +105,7 @@ createNextDescribe( // in dev we expect the entire page to be rendered at runtime expect($('#layout').text()).toBe('run') expect($('#page').text()).toBe('run') - // we expect there to be no susupense boundary in fallback state + // we expect there to be no suspense boundary in fallback state expect($('#boundary').html()).toBeNull() } else if (process.env.__NEXT_EXPERIMENTAL_PPR) { // in PPR we expect the shell to be rendered at build and the page to be rendered at runtime @@ -122,7 +117,7 @@ createNextDescribe( // in static generation we expect the entire page to be rendered at runtime expect($('#layout').text()).toBe('build') expect($('#page').text()).toBe('build') - // we expect there to be no susupense boundary in fallback state + // we expect there to be no suspense boundary in fallback state expect($('#boundary').html()).toBeNull() } From 2324e67d6467391ebdb245e09ca170e08da16b3e Mon Sep 17 00:00:00 2001 From: Josh Story Date: Mon, 22 Jan 2024 17:00:07 -0800 Subject: [PATCH 4/4] removes static-generation-bailout entirely by refactoring last usages to other dynamic apis and bailout techniques --- .../04-functions/next-request.mdx | 1 - .../next/src/build/templates/app-route.ts | 11 +- packages/next/src/build/utils.ts | 10 +- .../next/src/client/components/draft-mode.ts | 17 +- .../components/static-generation-bailout.ts | 70 --- .../app-render/create-component-tree.tsx | 24 +- .../next/src/server/app-render/entry-base.ts | 2 - .../app-route/helpers/clean-url.ts | 13 +- .../helpers/get-non-static-methods.ts | 28 - .../app-route/helpers/proxy-request.ts | 149 ------ .../future/route-modules/app-route/module.ts | 488 +++++++++++++++--- .../e2e/app-dir/not-found/basic/index.test.ts | 23 +- .../test/dynamicpage-dev.test.ts | 2 +- .../test/dynamicpage-prod.test.ts | 2 +- 14 files changed, 488 insertions(+), 352 deletions(-) delete mode 100644 packages/next/src/server/future/route-modules/app-route/helpers/get-non-static-methods.ts delete mode 100644 packages/next/src/server/future/route-modules/app-route/helpers/proxy-request.ts diff --git a/docs/02-app/02-api-reference/04-functions/next-request.mdx b/docs/02-app/02-api-reference/04-functions/next-request.mdx index c9deeec562888..64a177b0ac39a 100644 --- a/docs/02-app/02-api-reference/04-functions/next-request.mdx +++ b/docs/02-app/02-api-reference/04-functions/next-request.mdx @@ -109,7 +109,6 @@ The following options are available: | -------------- | ----------------------- | ----------------------------------------------------------------------------------------------------------------------------- | | `basePath` | `string` | The [base path](/docs/app/api-reference/next-config-js/basePath) of the URL. | | `buildId` | `string` \| `undefined` | The build identifier of the Next.js application. Can be [customized](/docs/app/api-reference/next-config-js/generateBuildId). | -| `url` | `URL` | The URL object. | | `pathname` | `string` | The pathname of the URL. | | `searchParams` | `Object` | The search parameters of the URL. | diff --git a/packages/next/src/build/templates/app-route.ts b/packages/next/src/build/templates/app-route.ts index 3a10bb61d0e89..4aaea88b9a4d9 100644 --- a/packages/next/src/build/templates/app-route.ts +++ b/packages/next/src/build/templates/app-route.ts @@ -32,13 +32,8 @@ const routeModule = new AppRouteRouteModule({ // Pull out the exports that we need to expose from the module. This should // be eliminated when we've moved the other routes to the new format. These // are used to hook into the route. -const { - requestAsyncStorage, - staticGenerationAsyncStorage, - serverHooks, - headerHooks, - staticGenerationBailout, -} = routeModule +const { requestAsyncStorage, staticGenerationAsyncStorage, serverHooks } = + routeModule const originalPathname = 'VAR_ORIGINAL_PATHNAME' @@ -51,8 +46,6 @@ export { requestAsyncStorage, staticGenerationAsyncStorage, serverHooks, - headerHooks, - staticGenerationBailout, originalPathname, patchFetch, } diff --git a/packages/next/src/build/utils.ts b/packages/next/src/build/utils.ts index 63815f59d3550..f8ddd8b4c74a7 100644 --- a/packages/next/src/build/utils.ts +++ b/packages/next/src/build/utils.ts @@ -1580,7 +1580,13 @@ export async function isPageStatic({ const routeModule: RouteModule = componentsResult.ComponentMod?.routeModule + let supportsPPR = false + if (pageType === 'app') { + if (ppr && routeModule.definition.kind === RouteKind.APP_PAGE) { + supportsPPR = true + } + const ComponentMod: AppPageModule = componentsResult.ComponentMod isClientComponent = isClientReference(componentsResult.ComponentMod) @@ -1650,7 +1656,7 @@ export async function isPageStatic({ // If force dynamic was set and we don't have PPR enabled, then set the // revalidate to 0. // TODO: (PPR) remove this once PPR is enabled by default - if (appConfig.dynamic === 'force-dynamic' && !ppr) { + if (appConfig.dynamic === 'force-dynamic' && !supportsPPR) { appConfig.revalidate = 0 } @@ -1748,7 +1754,7 @@ export async function isPageStatic({ // When PPR is enabled, any route may be completely static, so // mark this route as static. let isPPR = false - if (ppr && routeModule.definition.kind === RouteKind.APP_PAGE) { + if (supportsPPR) { isPPR = true isStatic = true } diff --git a/packages/next/src/client/components/draft-mode.ts b/packages/next/src/client/components/draft-mode.ts index 8688cca8698d9..e1c5cf94fd58e 100644 --- a/packages/next/src/client/components/draft-mode.ts +++ b/packages/next/src/client/components/draft-mode.ts @@ -1,6 +1,7 @@ import type { DraftModeProvider } from '../../server/async-storage/draft-mode-provider' -import { staticGenerationBailout } from './static-generation-bailout' +import { staticGenerationAsyncStorage } from './static-generation-async-storage.external' +import { trackDynamicDataAccessed } from '../../server/app-render/dynamic-rendering' export class DraftMode { /** @@ -15,14 +16,20 @@ export class DraftMode { return this._provider.isEnabled } public enable() { - if (staticGenerationBailout('draftMode().enable()')) { - return + const store = staticGenerationAsyncStorage.getStore() + if (store) { + // We we have a store we want to track dynamic data access to ensure we + // don't statically generate routes that manipulate draft mode. + trackDynamicDataAccessed(store, 'draftMode().enable()') } return this._provider.enable() } public disable() { - if (staticGenerationBailout('draftMode().disable()')) { - return + const store = staticGenerationAsyncStorage.getStore() + if (store) { + // We we have a store we want to track dynamic data access to ensure we + // don't statically generate routes that manipulate draft mode. + trackDynamicDataAccessed(store, 'draftMode().disable()') } return this._provider.disable() } diff --git a/packages/next/src/client/components/static-generation-bailout.ts b/packages/next/src/client/components/static-generation-bailout.ts index 17e7cb1cc978d..2bad070ebdc7e 100644 --- a/packages/next/src/client/components/static-generation-bailout.ts +++ b/packages/next/src/client/components/static-generation-bailout.ts @@ -1,11 +1,3 @@ -import type { AppConfigDynamic } from '../../build/utils' - -import React from 'react' -import { DynamicServerError } from './hooks-server-context' -import { staticGenerationAsyncStorage } from './static-generation-async-storage.external' - -const hasPostpone = typeof React.unstable_postpone === 'function' - const NEXT_STATIC_GEN_BAILOUT = 'NEXT_STATIC_GEN_BAILOUT' export class StaticGenBailoutError extends Error { @@ -21,65 +13,3 @@ export function isStaticGenBailoutError( return error.code === NEXT_STATIC_GEN_BAILOUT } - -type BailoutOpts = { dynamic?: AppConfigDynamic; link?: string } - -export type StaticGenerationBailout = ( - reason: string, - opts?: BailoutOpts -) => boolean | never - -function formatErrorMessage(reason: string, opts?: BailoutOpts) { - const { dynamic, link } = opts || {} - const suffix = link ? ` See more info here: ${link}` : '' - return `Page${ - dynamic ? ` with \`dynamic = "${dynamic}"\`` : '' - } couldn't be rendered statically because it used \`${reason}\`.${suffix}` -} - -export const staticGenerationBailout: StaticGenerationBailout = ( - reason, - { dynamic, link } = {} -) => { - const staticGenerationStore = staticGenerationAsyncStorage.getStore() - if (!staticGenerationStore) return false - - if (staticGenerationStore.forceStatic) { - return true - } - - if (staticGenerationStore.dynamicShouldError) { - throw new StaticGenBailoutError( - formatErrorMessage(reason, { link, dynamic: dynamic ?? 'error' }) - ) - } - - if (staticGenerationStore.prerenderState && hasPostpone) { - throw React.unstable_postpone(formatErrorMessage(reason, { link, dynamic })) - } - - // As this is a bailout, we don't want to revalidate, so set the revalidate - // to 0. - staticGenerationStore.revalidate = 0 - - if (staticGenerationStore.isStaticGeneration) { - const err = new DynamicServerError( - formatErrorMessage(reason, { - dynamic, - // this error should be caught by Next to bail out of static generation - // in case it's uncaught, this link provides some additional context as to why - link: 'https://nextjs.org/docs/messages/dynamic-server-error', - }) - ) - staticGenerationStore.dynamicUsageDescription = reason - staticGenerationStore.dynamicUsageStack = err.stack - - throw err - } - - return false -} - -// export function interuptStaticGeneration(store: StaticGenerationStore) { -// if (store.) -// } diff --git a/packages/next/src/server/app-render/create-component-tree.tsx b/packages/next/src/server/app-render/create-component-tree.tsx index a795c462a8882..310234bd1508c 100644 --- a/packages/next/src/server/app-render/create-component-tree.tsx +++ b/packages/next/src/server/app-render/create-component-tree.tsx @@ -13,6 +13,7 @@ import { validateRevalidate } from '../lib/patch-fetch' import { PARALLEL_ROUTE_DEFAULT_PATH } from '../../client/components/parallel-route-default' import { getTracer } from '../lib/trace/tracer' import { NextNodeServerSpan } from '../lib/trace/constants' +import { StaticGenBailoutError } from '../../client/components/static-generation-bailout' type ComponentTree = { seedData: CacheNodeSeedData @@ -80,7 +81,6 @@ async function createComponentTreeInternal({ renderOpts: { nextConfigOutput, experimental }, staticGenerationStore, componentMod: { - staticGenerationBailout, NotFoundBoundary, LayoutRouter, RenderFromTemplateContext, @@ -185,12 +185,10 @@ async function createComponentTreeInternal({ if (!dynamic || dynamic === 'auto') { dynamic = 'error' } else if (dynamic === 'force-dynamic') { - staticGenerationStore.forceDynamic = true - staticGenerationStore.dynamicShouldError = true - staticGenerationBailout(`output: export`, { - dynamic, - link: 'https://nextjs.org/docs/advanced-features/static-html-export', - }) + // force-dynamic is always incompatible with 'export'. We must interrupt the build + throw new StaticGenBailoutError( + `Page with \`dynamic = "force-dynamic"\` couldn't be exported. \`output: "export"\` requires all pages be renderable statically because there is not runtime server to dynamic render routes in this output format. Learn more: https://nextjs.org/docs/app/building-your-application/deploying/static-exports` + ) } } @@ -204,10 +202,18 @@ async function createComponentTreeInternal({ staticGenerationStore.forceDynamic = true // TODO: (PPR) remove this bailout once PPR is the default - if (!staticGenerationStore.prerenderState) { + if ( + staticGenerationStore.isStaticGeneration && + !staticGenerationStore.prerenderState + ) { // If the postpone API isn't available, we can't postpone the render and // therefore we can't use the dynamic API. - staticGenerationBailout(`force-dynamic`, { dynamic }) + const err = new DynamicServerError( + `Page with \`dynamic = "force-dynamic"\` won't be rendered statically.` + ) + staticGenerationStore.dynamicUsageDescription = err.message + staticGenerationStore.dynamicUsageStack = err.stack + throw err } } else { staticGenerationStore.dynamicShouldError = false diff --git a/packages/next/src/server/app-render/entry-base.ts b/packages/next/src/server/app-render/entry-base.ts index 3ac94d9e0cfcc..822ba08ed7df7 100644 --- a/packages/next/src/server/app-render/entry-base.ts +++ b/packages/next/src/server/app-render/entry-base.ts @@ -12,7 +12,6 @@ import RenderFromTemplateContext from '../../client/components/render-from-templ import { staticGenerationAsyncStorage } from '../../client/components/static-generation-async-storage.external' import { requestAsyncStorage } from '../../client/components/request-async-storage.external' import { actionAsyncStorage } from '../../client/components/action-async-storage.external' -import { staticGenerationBailout } from '../../client/components/static-generation-bailout' import { ClientPageRoot } from '../../client/components/client-page' import { createUntrackedSearchParams, @@ -45,7 +44,6 @@ export { staticGenerationAsyncStorage, requestAsyncStorage, actionAsyncStorage, - staticGenerationBailout, createUntrackedSearchParams, createDynamicallyTrackedSearchParams, serverHooks, diff --git a/packages/next/src/server/future/route-modules/app-route/helpers/clean-url.ts b/packages/next/src/server/future/route-modules/app-route/helpers/clean-url.ts index 517478d8b8ee0..f75e4c9d57162 100644 --- a/packages/next/src/server/future/route-modules/app-route/helpers/clean-url.ts +++ b/packages/next/src/server/future/route-modules/app-route/helpers/clean-url.ts @@ -4,10 +4,11 @@ * @param urlString the url to clean * @returns the cleaned url */ -export function cleanURL(urlString: string): string { - const url = new URL(urlString) - url.host = 'localhost:3000' - url.search = '' - url.protocol = 'http' - return url.toString() + +export function cleanURL(url: string | URL): URL { + const u = new URL(url) + u.host = 'localhost:3000' + u.search = '' + u.protocol = 'http' + return u } diff --git a/packages/next/src/server/future/route-modules/app-route/helpers/get-non-static-methods.ts b/packages/next/src/server/future/route-modules/app-route/helpers/get-non-static-methods.ts deleted file mode 100644 index 721ce1a6230e4..0000000000000 --- a/packages/next/src/server/future/route-modules/app-route/helpers/get-non-static-methods.ts +++ /dev/null @@ -1,28 +0,0 @@ -import type { HTTP_METHOD } from '../../../../web/http' -import type { AppRouteHandlers } from '../module' - -const NON_STATIC_METHODS = [ - 'OPTIONS', - 'POST', - 'PUT', - 'DELETE', - 'PATCH', -] as const - -/** - * Gets all the method names for handlers that are not considered static. - * - * @param handlers the handlers from the userland module - * @returns the method names that are not considered static or false if all - * methods are static - */ -export function getNonStaticMethods( - handlers: AppRouteHandlers -): ReadonlyArray | false { - // We can currently only statically optimize if only GET/HEAD are used as - // prerender can't be used conditionally based on the method currently. - const methods = NON_STATIC_METHODS.filter((method) => handlers[method]) - if (methods.length === 0) return false - - return methods -} diff --git a/packages/next/src/server/future/route-modules/app-route/helpers/proxy-request.ts b/packages/next/src/server/future/route-modules/app-route/helpers/proxy-request.ts deleted file mode 100644 index cd58e253a6a98..0000000000000 --- a/packages/next/src/server/future/route-modules/app-route/helpers/proxy-request.ts +++ /dev/null @@ -1,149 +0,0 @@ -import type * as ServerHooks from '../../../../../client/components/hooks-server-context' -import type * as HeaderHooks from '../../../../../client/components/headers' -import type { staticGenerationBailout as StaticGenerationBailout } from '../../../../../client/components/static-generation-bailout' -import type { AppRouteUserlandModule } from '../module' -import type { NextRequest } from '../../../../web/spec-extension/request' - -import { RequestCookies } from 'next/dist/compiled/@edge-runtime/cookies' -import { NextURL } from '../../../../web/next-url' -import { cleanURL } from './clean-url' - -export function proxyRequest( - request: NextRequest, - { dynamic }: Pick, - hooks: { - readonly serverHooks: typeof ServerHooks - readonly headerHooks: typeof HeaderHooks - readonly staticGenerationBailout: typeof StaticGenerationBailout - } -): NextRequest { - function handleNextUrlBailout(prop: string | symbol) { - switch (prop) { - case 'search': - case 'searchParams': - case 'toString': - case 'href': - case 'origin': - hooks.staticGenerationBailout(`nextUrl.${prop as string}`) - return - default: - return - } - } - - const cache: { - url?: string - toString?: () => string - headers?: Headers - cookies?: RequestCookies - searchParams?: URLSearchParams - } = {} - - const handleForceStatic = (url: string, prop: string) => { - switch (prop) { - case 'search': - return '' - case 'searchParams': - if (!cache.searchParams) cache.searchParams = new URLSearchParams() - - return cache.searchParams - case 'url': - case 'href': - if (!cache.url) cache.url = cleanURL(url) - - return cache.url - case 'toJSON': - case 'toString': - if (!cache.url) cache.url = cleanURL(url) - if (!cache.toString) cache.toString = () => cache.url! - - return cache.toString - case 'headers': - if (!cache.headers) cache.headers = new Headers() - - return cache.headers - case 'cookies': - if (!cache.headers) cache.headers = new Headers() - if (!cache.cookies) cache.cookies = new RequestCookies(cache.headers) - - return cache.cookies - case 'clone': - if (!cache.url) cache.url = cleanURL(url) - - return () => new NextURL(cache.url!) - default: - break - } - } - - const wrappedNextUrl = new Proxy(request.nextUrl, { - get(target, prop) { - handleNextUrlBailout(prop) - - if (dynamic === 'force-static' && typeof prop === 'string') { - const result = handleForceStatic(target.href, prop) - if (result !== undefined) return result - } - const value = (target as any)[prop] - - if (typeof value === 'function') { - return value.bind(target) - } - return value - }, - set(target, prop, value) { - handleNextUrlBailout(prop) - ;(target as any)[prop] = value - return true - }, - }) - - const handleReqBailout = (prop: string | symbol) => { - switch (prop) { - case 'headers': - hooks.headerHooks.headers() - return - // if request.url is accessed directly instead of - // request.nextUrl we bail since it includes query - // values that can be relied on dynamically - case 'url': - case 'cookies': - case 'body': - case 'blob': - case 'json': - case 'text': - case 'arrayBuffer': - case 'formData': - hooks.staticGenerationBailout(`request.${prop}`) - return - default: - return - } - } - - return new Proxy(request, { - get(target, prop) { - handleReqBailout(prop) - - if (prop === 'nextUrl') { - return wrappedNextUrl - } - - if (dynamic === 'force-static' && typeof prop === 'string') { - const result = handleForceStatic(target.url, prop) - if (result !== undefined) return result - } - const value: any = (target as any)[prop] - - if (typeof value === 'function') { - return value.bind(target) - } - return value - }, - set(target, prop, value) { - handleReqBailout(prop) - ;(target as any)[prop] = value - return true - }, - }) -} 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 eb8cc3e41d5f9..963353856f1c1 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 @@ -3,6 +3,7 @@ import type { AppRouteRouteDefinition } from '../../route-definitions/app-route- import type { AppConfig } from '../../../../build/utils' import type { NextRequest } from '../../../web/spec-extension/request' import type { PrerenderManifest } from '../../../../build' +import type { NextURL } from '../../../web/next-url' import { RouteModule, @@ -26,23 +27,28 @@ import { addImplicitTags, patchFetch } from '../../../lib/patch-fetch' import { getTracer } from '../../../lib/trace/tracer' import { AppRouteRouteHandlersSpan } from '../../../lib/trace/constants' import { getPathnameFromAbsolutePath } from './helpers/get-pathname-from-absolute-path' -import { proxyRequest } from './helpers/proxy-request' import { resolveHandlerError } from './helpers/resolve-handler-error' import * as Log from '../../../../build/output/log' import { autoImplementMethods } from './helpers/auto-implement-methods' -import { getNonStaticMethods } from './helpers/get-non-static-methods' -import { appendMutableCookies } from '../../../web/spec-extension/adapters/request-cookies' +import { + appendMutableCookies, + type ReadonlyRequestCookies, +} from '../../../web/spec-extension/adapters/request-cookies' +import { HeadersAdapter } from '../../../web/spec-extension/adapters/headers' +import { RequestCookiesAdapter } from '../../../web/spec-extension/adapters/request-cookies' import { parsedUrlQueryToParams } from './helpers/parsed-url-query-to-params' import * as serverHooks from '../../../../client/components/hooks-server-context' -import * as headerHooks from '../../../../client/components/headers' -import { staticGenerationBailout } from '../../../../client/components/static-generation-bailout' +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 { actionAsyncStorage } from '../../../../client/components/action-async-storage.external' import * as sharedModules from './shared-modules' import { getIsServerAction } from '../../../lib/server-action-request-meta' +import { RequestCookies } from 'next/dist/compiled/@edge-runtime/cookies' +import { cleanURL } from './helpers/clean-url' +import { StaticGenBailoutError } from '../../../../client/components/static-generation-bailout' /** * The AppRouteModule is the type of the module exported by the bundled App @@ -135,18 +141,6 @@ export class AppRouteRouteModule extends RouteModule< */ public readonly serverHooks = serverHooks - /** - * An interface to call header hooks which interact with the underlying - * request storage. - */ - public readonly headerHooks = headerHooks - - /** - * An interface to call static generation bailout hooks which interact with - * the underlying static generation storage. - */ - public readonly staticGenerationBailout = staticGenerationBailout - public static readonly sharedModules = sharedModules /** @@ -159,7 +153,7 @@ export class AppRouteRouteModule extends RouteModule< public readonly nextConfigOutput: NextConfig['output'] | undefined private readonly methods: Record - private readonly nonStaticMethods: ReadonlyArray | false + private readonly hasNonStaticMethods: boolean private readonly dynamic: AppRouteUserlandModule['dynamic'] constructor({ @@ -178,7 +172,7 @@ export class AppRouteRouteModule extends RouteModule< this.methods = autoImplementMethods(userland) // Get the non-static methods for this route. - this.nonStaticMethods = getNonStaticMethods(userland) + this.hasNonStaticMethods = hasNonStaticMethods(userland) // Get the dynamic property from the userland module. this.dynamic = this.userland.dynamic @@ -244,15 +238,15 @@ export class AppRouteRouteModule extends RouteModule< * Executes the route handler. */ private async execute( - request: NextRequest, + rawRequest: NextRequest, context: AppRouteRouteHandlerContext ): Promise { // Get the handler function for the given method. - const handler = this.resolve(request.method) + const handler = this.resolve(rawRequest.method) // Get the context for the request. const requestContext: RequestContext = { - req: request, + req: rawRequest, } // TODO: types for renderOpts should include previewProps @@ -262,7 +256,7 @@ export class AppRouteRouteModule extends RouteModule< // Get the context for the static generation. const staticGenerationContext: StaticGenerationContext = { - urlPathname: request.nextUrl.pathname, + urlPathname: rawRequest.nextUrl.pathname, renderOpts: context.renderOpts, } @@ -275,7 +269,7 @@ export class AppRouteRouteModule extends RouteModule< const response: unknown = await this.actionAsyncStorage.run( { isAppRoute: true, - isAction: getIsServerAction(request), + isAction: getIsServerAction(rawRequest), }, () => RequestAsyncStorageWrapper.wrap( @@ -288,36 +282,81 @@ export class AppRouteRouteModule extends RouteModule< (staticGenerationStore) => { // Check to see if we should bail out of static generation based on // having non-static methods. - if (this.nonStaticMethods) { - this.staticGenerationBailout( - `non-static methods used ${this.nonStaticMethods.join( - ', ' - )}` - ) + const isStaticGeneration = + staticGenerationStore.isStaticGeneration + + if (this.hasNonStaticMethods) { + if (isStaticGeneration) { + const err = new DynamicServerError( + 'Route is configured with methods that cannot be statically generated.' + ) + staticGenerationStore.dynamicUsageDescription = err.message + staticGenerationStore.dynamicUsageStack = err.stack + throw err + } else { + // We aren't statically generating but since this route has non-static methods + // we still need to set the default caching to no cache by setting revalidate = 0 + // @TODO this type of logic is too indirect. we need to refactor how we set fetch cache + // behavior. Prior to the most recent refactor this logic was buried deep in staticGenerationBailout + // so it is possible it was unintentional and then tests were written to assert the current behavior + staticGenerationStore.revalidate = 0 + } } + // We assume we can pass the original request through however we may end up + // proxying it in certain circumstances based on execution type and configuraiton + let request = rawRequest + // Update the static generation store based on the dynamic property. - switch (this.dynamic) { - case 'force-dynamic': - // The dynamic property is set to force-dynamic, so we should - // force the page to be dynamic. - staticGenerationStore.forceDynamic = true - this.staticGenerationBailout(`force-dynamic`, { - dynamic: this.dynamic, - }) - break - case 'force-static': + if (isStaticGeneration) { + switch (this.dynamic) { + case 'force-dynamic': + // We should never be in this case but since it can happen based on the way our build/execution is structured + // We defend against it for the time being + throw new Error( + 'Invariant: `dynamic-error` during static generation not expected for app routes. This is a bug in Next.js' + ) + 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 + ) + } + } 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') { // The dynamic property is set to force-static, so we should // force the page to be static. staticGenerationStore.forceStatic = true - 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 - break - default: - break + request = new Proxy(rawRequest, forceStaticRequestHandlers) + } } // If the static generation store does not have a revalidate value @@ -326,18 +365,6 @@ export class AppRouteRouteModule extends RouteModule< staticGenerationStore.revalidate ??= this.userland.revalidate ?? false - // Wrap the request so we can add additional functionality to cases - // that might change it's output or affect the rendering. - const wrappedRequest = proxyRequest( - request, - { dynamic: this.dynamic }, - { - headerHooks: this.headerHooks, - serverHooks: this.serverHooks, - staticGenerationBailout: this.staticGenerationBailout, - } - ) - // TODO: propagate this pathname from route matcher const route = getPathnameFromAbsolutePath(this.resolvedPagePath) getTracer().getRootSpanAttributes()?.set('next.route', route) @@ -356,7 +383,7 @@ export class AppRouteRouteModule extends RouteModule< staticGenerationAsyncStorage: this.staticGenerationAsyncStorage, }) - const res = await handler(wrappedRequest, { + const res = await handler(request, { params: context.params ? parsedUrlQueryToParams(context.params) : undefined, @@ -472,3 +499,342 @@ export class AppRouteRouteModule extends RouteModule< } export default AppRouteRouteModule + +/** + * Gets all the method names for handlers that are not considered static. + * + * @param handlers the handlers from the userland module + * @returns the method names that are not considered static or false if all + * methods are static + */ +export function hasNonStaticMethods(handlers: AppRouteHandlers): boolean { + if ( + // Order these by how common they are to be used + handlers.POST || + handlers.POST || + handlers.DELETE || + handlers.PATCH || + handlers.OPTIONS + ) { + return true + } + return false +} + +// These symbols will be used to stash cached values on Proxied requests without requiring +// additional closures or storage such as WeakMaps. +const nextURLSymbol = Symbol('nextUrl') +const requestCloneSymbol = Symbol('clone') +const urlCloneSymbol = Symbol('clone') +const searchParamsSymbol = Symbol('searchParams') +const hrefSymbol = Symbol('href') +const toStringSymbol = Symbol('toString') +const headersSymbol = Symbol('headers') +const cookiesSymbol = Symbol('cookies') + +type RequestSymbolTarget = { + [headersSymbol]?: Headers + [cookiesSymbol]?: RequestCookies | ReadonlyRequestCookies + [nextURLSymbol]?: NextURL + [requestCloneSymbol]?: () => NextRequest +} + +type UrlSymbolTarget = { + [searchParamsSymbol]?: URLSearchParams + [hrefSymbol]?: string + [toStringSymbol]?: () => string + [urlCloneSymbol]?: () => NextURL +} + +/** + * The general technique with these proxy handlers is to prioritize keeping them static + * by stashing computed values on the Proxy itself. This is safe because the Proxy is + * inaccessible to the consumer since all operations are forwarded + */ +const forceStaticRequestHandlers = { + get( + target: NextRequest & RequestSymbolTarget, + prop: string | symbol, + receiver: any + ): unknown { + switch (prop) { + case 'headers': + return ( + target[headersSymbol] || + (target[headersSymbol] = HeadersAdapter.seal(new Headers({}))) + ) + case 'cookies': + return ( + target[cookiesSymbol] || + (target[cookiesSymbol] = RequestCookiesAdapter.seal( + new RequestCookies(new Headers({})) + )) + ) + case 'nextUrl': + return ( + target[nextURLSymbol] || + (target[nextURLSymbol] = new Proxy( + target.nextUrl, + forceStaticNextUrlHandlers + )) + ) + case 'url': + // we don't need to separately cache this we can just read the nextUrl + // and return the href since we know it will have been stripped of any + // dynamic parts. We access via the receiver to trigger the get trap + return receiver.nextUrl.href + case 'geo': + case 'ip': + return undefined + 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, + forceStaticRequestHandlers + )) + ) + default: + const result = Reflect.get(target, prop, receiver) + if (typeof result === 'function') { + return result.bind(target) + } + return result + } + }, + // We don't need to proxy set because all the properties we proxy are ready only + // and will be ignored +} + +const forceStaticNextUrlHandlers = { + get( + target: NextURL & UrlSymbolTarget, + prop: string | symbol, + receiver: any + ): unknown { + switch (prop) { + // URL properties + case 'search': + return '' + case 'searchParams': + return ( + target[searchParamsSymbol] || + (target[searchParamsSymbol] = new URLSearchParams()) + ) + case 'href': + return ( + target[hrefSymbol] || + (target[hrefSymbol] = cleanURL(target.href).href) + ) + case 'toJSON': + case 'toString': + return ( + target[toStringSymbol] || + (target[toStringSymbol] = () => receiver.href) + ) + + // NextUrl properties + case 'url': + // Currently nextURL does not expose url but our Docs indicate that it is an available property + // I am forcing this to undefined here to avoid accidentally exposing a dynamic value later if + // the underlying nextURL ends up adding this property + return undefined + case 'clone': + return ( + target[urlCloneSymbol] || + (target[urlCloneSymbol] = () => + new Proxy(target.clone(), forceStaticNextUrlHandlers)) + ) + default: + const result = Reflect.get(target, prop, receiver) + if (typeof result === 'function') { + return result.bind(target) + } + return result + } + }, +} + +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 couldn't be rendered statically because it accessed 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) + } + return result + } + }, + // We don't need to proxy set because all the properties we proxy are ready only + // and will be ignored +} + +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 couldn't be rendered statically because it accessed 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) + } + return result + } + }, +} + +const requireStaticRequestHandlers = { + get( + target: NextRequest & RequestSymbolTarget, + prop: string | symbol, + receiver: any + ): unknown { + switch (prop) { + case 'nextUrl': + return ( + target[nextURLSymbol] || + (target[nextURLSymbol] = new Proxy( + target.nextUrl, + requireStaticNextUrlHandlers + )) + ) + case 'headers': + case 'cookies': + case 'url': + case 'body': + case 'blob': + case 'json': + case 'text': + case 'arrayBuffer': + case 'formData': + throw new StaticGenBailoutError( + `Route with \`dynamic = "error"\` couldn't be rendered statically because it accessed accessed \`request.${prop}\`.` + ) + 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, + requireStaticRequestHandlers + )) + ) + default: + const result = Reflect.get(target, prop, receiver) + if (typeof result === 'function') { + return result.bind(target) + } + return result + } + }, + // We don't need to proxy set because all the properties we proxy are ready only + // and will be ignored +} + +const requireStaticNextUrlHandlers = { + 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 StaticGenBailoutError( + `Route with \`dynamic = "error"\` couldn't be rendered statically because it accessed accessed \`nextUrl.${prop}\`.` + ) + case 'clone': + return ( + target[urlCloneSymbol] || + (target[urlCloneSymbol] = () => + new Proxy(target.clone(), requireStaticNextUrlHandlers)) + ) + default: + const result = Reflect.get(target, prop, receiver) + if (typeof result === 'function') { + return result.bind(target) + } + return result + } + }, +} diff --git a/test/e2e/app-dir/not-found/basic/index.test.ts b/test/e2e/app-dir/not-found/basic/index.test.ts index bbef534c0aa57..18c16f9379583 100644 --- a/test/e2e/app-dir/not-found/basic/index.test.ts +++ b/test/e2e/app-dir/not-found/basic/index.test.ts @@ -116,14 +116,21 @@ createNextDescribe( }, 'success') }) - it('should render the 404 page when the file is removed, and restore the page when re-added', async () => { - const browser = await next.browser('/') - await check(() => browser.elementByCss('h1').text(), 'My page') - await next.renameFile('./app/page.js', './app/foo.js') - await check(() => browser.elementByCss('h1').text(), 'Root Not Found') - await next.renameFile('./app/foo.js', './app/page.js') - await check(() => browser.elementByCss('h1').text(), 'My page') - }) + // Disabling for Edge because it is too flakey. + // @TODO investigate a proper for fix for this flake + if (!isEdge) { + it('should render the 404 page when the file is removed, and restore the page when re-added', async () => { + const browser = await next.browser('/') + await check(() => browser.elementByCss('h1').text(), 'My page') + await next.renameFile('./app/page.js', './app/foo.js') + await check( + () => browser.elementByCss('h1').text(), + 'Root Not Found' + ) + await next.renameFile('./app/foo.js', './app/page.js') + await check(() => browser.elementByCss('h1').text(), 'My page') + }) + } } if (!isNextDev && !isEdge) { diff --git a/test/integration/app-dir-export/test/dynamicpage-dev.test.ts b/test/integration/app-dir-export/test/dynamicpage-dev.test.ts index 07ec6f9bfaea0..74ea4a7c308c3 100644 --- a/test/integration/app-dir-export/test/dynamicpage-dev.test.ts +++ b/test/integration/app-dir-export/test/dynamicpage-dev.test.ts @@ -9,7 +9,7 @@ describe('app dir - with output export - dynamic page dev', () => { { dynamicPage: "'force-dynamic'", expectedErrMsg: - 'Page with `dynamic = "force-dynamic"` couldn\'t be rendered statically because it used `output: export`.', + 'Page with `dynamic = "force-dynamic"` couldn\'t be exported. `output: "export"` requires all pages be renderable statically', }, ])( 'should work in dev with dynamicPage $dynamicPage', diff --git a/test/integration/app-dir-export/test/dynamicpage-prod.test.ts b/test/integration/app-dir-export/test/dynamicpage-prod.test.ts index 4f2029d099bc8..a5365ded2f12c 100644 --- a/test/integration/app-dir-export/test/dynamicpage-prod.test.ts +++ b/test/integration/app-dir-export/test/dynamicpage-prod.test.ts @@ -9,7 +9,7 @@ describe('app dir - with output export - dynamic api route prod', () => { { dynamicPage: "'force-dynamic'", expectedErrMsg: - 'Page with `dynamic = "force-dynamic"` couldn\'t be rendered statically because it used `output: export`.', + 'Page with `dynamic = "force-dynamic"` couldn\'t be exported. `output: "export"` requires all pages be renderable statically', }, ])( 'should work in prod with dynamicPage $dynamicPage',