From 2324e67d6467391ebdb245e09ca170e08da16b3e Mon Sep 17 00:00:00 2001 From: Josh Story Date: Mon, 22 Jan 2024 17:00:07 -0800 Subject: [PATCH] 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',