fix: server actions initiated from static pages (#51534)

### What?
Pages marked with `generateStaticParams` don't currently support server actions, and instead return a 405 Method Not Allowed, with no action being taken on the client. Additionally, pages that are marked static & use server actions are opted into dynamic rendering.

### Why?
The page that has `generateStaticParams` is marked as `isSSG` [here](ee2ec3dd1d/packages/next/src/server/base-server.ts (L1337)).

As a result, the request is short-circuited because a POST request isn't supported on static pages. Upon detecting a server action on a page marked SSG, we bypass the static cache and go straight to the lambda. 

This PR introduces an experimental option to the prerender manifest that will allow for selectively bypassing the static cache

This also removes the need to bail out of static generation

Closes NEXT-1167
Closes NEXT-1453
Fixes #49408
Fixes #52840
Fixes #50932
This commit is contained in:
Zack Tanner 2023-09-14 14:22:19 -07:00 committed by GitHub
parent be38d02349
commit a73abad609
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 981 additions and 381 deletions

View file

@ -36,6 +36,7 @@ import loadCustomRoutes, {
normalizeRouteRegex,
Redirect,
Rewrite,
RouteHas,
RouteType,
} from '../lib/load-custom-routes'
import { getRedirectStatus, modifyRouteRegex } from '../lib/redirect-status'
@ -130,6 +131,7 @@ import { flatReaddir } from '../lib/flat-readdir'
import { eventSwcPlugins } from '../telemetry/events/swc-plugins'
import { normalizeAppPath } from '../shared/lib/router/utils/app-paths'
import {
ACTION,
NEXT_ROUTER_PREFETCH,
RSC,
RSC_CONTENT_TYPE_HEADER,
@ -160,6 +162,7 @@ export type SsgRoute = {
dataRoute: string | null
initialStatus?: number
initialHeaders?: Record<string, string>
experimentalBypassFor?: RouteHas[]
}
export type DynamicSsgRoute = {
@ -167,6 +170,7 @@ export type DynamicSsgRoute = {
fallback: string | null | false
dataRoute: string | null
dataRouteRegex: string | null
experimentalBypassFor?: RouteHas[]
}
export type PrerenderManifest = {
@ -1612,14 +1616,6 @@ export default async function build(
`Using edge runtime on a page currently disables static generation for that page`
)
} else {
// If a page has action and it is static, we need to
// change it to SSG to keep the worker created.
// TODO: This is a workaround for now, we should have a
// dedicated worker defined in a heuristic way.
const hasAction = entriesWithAction?.has(
'app' + originalAppPath
)
if (
workerResult.encodedPrerenderRoutes &&
workerResult.prerenderRoutes
@ -1638,47 +1634,39 @@ export default async function build(
const appConfig = workerResult.appConfig || {}
if (appConfig.revalidate !== 0) {
if (hasAction) {
Log.warnOnce(
`Using server actions on a page currently disables static generation for that page`
const isDynamic = isDynamicRoute(page)
const hasGenerateStaticParams =
!!workerResult.prerenderRoutes?.length
if (
config.output === 'export' &&
isDynamic &&
!hasGenerateStaticParams
) {
throw new Error(
`Page "${page}" is missing "generateStaticParams()" so it cannot be used with "output: export" config.`
)
} else {
const isDynamic = isDynamicRoute(page)
const hasGenerateStaticParams =
!!workerResult.prerenderRoutes?.length
}
if (
config.output === 'export' &&
isDynamic &&
!hasGenerateStaticParams
) {
throw new Error(
`Page "${page}" is missing "generateStaticParams()" so it cannot be used with "output: export" config.`
)
}
if (
// Mark the app as static if:
// - It has no dynamic param
// - It doesn't have generateStaticParams but `dynamic` is set to
// `error` or `force-static`
!isDynamic
) {
appStaticPaths.set(originalAppPath, [page])
appStaticPathsEncoded.set(originalAppPath, [
page,
])
isStatic = true
} else if (
isDynamic &&
!hasGenerateStaticParams &&
(appConfig.dynamic === 'error' ||
appConfig.dynamic === 'force-static')
) {
appStaticPaths.set(originalAppPath, [])
appStaticPathsEncoded.set(originalAppPath, [])
isStatic = true
}
if (
// Mark the app as static if:
// - It has no dynamic param
// - It doesn't have generateStaticParams but `dynamic` is set to
// `error` or `force-static`
!isDynamic
) {
appStaticPaths.set(originalAppPath, [page])
appStaticPathsEncoded.set(originalAppPath, [page])
isStatic = true
} else if (
isDynamic &&
!hasGenerateStaticParams &&
(appConfig.dynamic === 'error' ||
appConfig.dynamic === 'force-static')
) {
appStaticPaths.set(originalAppPath, [])
appStaticPathsEncoded.set(originalAppPath, [])
isStatic = true
}
}
@ -2681,6 +2669,17 @@ export default async function build(
const isRouteHandler = isAppRouteRoute(originalAppPath)
// this flag is used to selectively bypass the static cache and invoke the lambda directly
// to enable server actions on static routes
const bypassFor: RouteHas[] = [
{ type: 'header', key: ACTION },
{
type: 'header',
key: 'content-type',
value: 'multipart/form-data',
},
]
routes.forEach((route) => {
if (isDynamicRoute(page) && route === page) return
if (route === '/_not-found') return
@ -2708,10 +2707,7 @@ export default async function build(
? null
: path.posix.join(`${normalizedRoute}.rsc`)
const routeMeta: {
initialStatus?: SsgRoute['initialStatus']
initialHeaders?: SsgRoute['initialHeaders']
} = {}
const routeMeta: Partial<SsgRoute> = {}
const exportRouteMeta: {
status?: number
@ -2748,6 +2744,7 @@ export default async function build(
finalPrerenderRoutes[route] = {
...routeMeta,
experimentalBypassFor: bypassFor,
initialRevalidateSeconds: revalidate,
srcRoute: page,
dataRoute,
@ -2771,6 +2768,7 @@ export default async function build(
// TODO: create a separate manifest to allow enforcing
// dynamicParams for non-static paths?
finalDynamicRoutes[page] = {
experimentalBypassFor: bypassFor,
routeRegex: normalizeRouteRegex(
getNamedRouteRegex(page, false).re.source
),

View file

@ -187,6 +187,16 @@ export async function renderToHTMLOrFlight(
appDirDevErrorLogger,
} = renderOpts
// We need to expose the bundled `require` API globally for
// react-server-dom-webpack. This is a hack until we find a better way.
if (ComponentMod.__next_app__) {
// @ts-ignore
globalThis.__next_require__ = ComponentMod.__next_app__.require
// @ts-ignore
globalThis.__next_chunk_load__ = ComponentMod.__next_app__.loadChunk
}
const extraRenderResultMeta: RenderResultMetadata = {}
const appUsingSizeAdjust = !!nextFontManifest?.appUsingSizeAdjust

View file

@ -34,16 +34,6 @@ export function createServerComponentRenderer<Props>(
serverComponentsErrorHandler: ReturnType<typeof createErrorHandler>,
nonce?: string
): (props: Props) => JSX.Element {
// We need to expose the bundled `require` API globally for
// react-server-dom-webpack. This is a hack until we find a better way.
if (ComponentMod.__next_app__) {
// @ts-ignore
globalThis.__next_require__ = ComponentMod.__next_app__.require
// @ts-ignore
globalThis.__next_chunk_load__ = ComponentMod.__next_app__.loadChunk
}
let RSCStream: ReadableStream<Uint8Array>
const createRSCStream = (props: Props) => {
if (!RSCStream) {

View file

@ -15,6 +15,7 @@ export type StaticGenerationContext = {
nextExport?: boolean
fetchCache?: StaticGenerationStore['fetchCache']
isDraftMode?: boolean
isServerAction?: boolean
/**
* A hack around accessing the store value outside the context of the
@ -49,11 +50,15 @@ export const StaticGenerationAsyncStorageWrapper: AsyncStorageWrapper<
*
* 3.) If the request is in draft mode, we must generate dynamic HTML.
*
* 4.) If the request is a server action, we must generate dynamic HTML.
*
* These rules help ensure that other existing features like request caching,
* coalescing, and ISR continue working as intended.
*/
const isStaticGeneration =
!renderOpts.supportsDynamicHTML && !renderOpts.isDraftMode
!renderOpts.supportsDynamicHTML &&
!renderOpts.isDraftMode &&
!renderOpts.isServerAction
const store: StaticGenerationStore = {
isStaticGeneration,

View file

@ -85,6 +85,7 @@ import {
RSC_VARY_HEADER,
FLIGHT_PARAMETERS,
NEXT_RSC_UNION_QUERY,
ACTION,
NEXT_ROUTER_PREFETCH,
RSC_CONTENT_TYPE_HEADER,
} from '../client/components/app-router-headers'
@ -1588,7 +1589,15 @@ export default abstract class Server<ServerOptions extends Options = Options> {
const isAppPath = components.isAppPath === true
const hasServerProps = !!components.getServerSideProps
let hasStaticPaths = !!components.getStaticPaths
const actionId = req.headers[ACTION.toLowerCase()] as string
const contentType = req.headers['content-type']
const isMultipartAction =
req.method === 'POST' && contentType?.startsWith('multipart/form-data')
const isFetchAction =
actionId !== undefined &&
typeof actionId === 'string' &&
req.method === 'POST'
const isServerAction = isFetchAction || isMultipartAction
const hasGetInitialProps = !!components.Component?.getInitialProps
let isSSG = !!components.getStaticProps
@ -1725,6 +1734,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
// requests so ensure we respond with 405 for
// invalid requests
if (
!isServerAction &&
!is404Page &&
!is500Page &&
pathname !== '/_error' &&
@ -1879,8 +1889,8 @@ export default abstract class Server<ServerOptions extends Options = Options> {
}
let ssgCacheKey =
isPreviewMode || !isSSG || opts.supportsDynamicHTML
? null // Preview mode, on-demand revalidate, flight request can bypass the cache
isPreviewMode || !isSSG || opts.supportsDynamicHTML || isServerAction
? null // Preview mode, on-demand revalidate, server actions, flight request can bypass the cache
: `${locale ? `/${locale}` : ''}${
(pathname === '/' || resolvedUrlPathname === '/') && locale
? ''
@ -1996,6 +2006,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
supportsDynamicHTML,
isOnDemandRevalidate,
isDraftMode: isPreviewMode,
isServerAction,
}
// Legacy render methods will return a render result that needs to be

View file

@ -274,6 +274,7 @@ export type RenderOptsPartial = {
strictNextHead: boolean
isDraftMode?: boolean
deploymentId?: string
isServerAction?: boolean
}
export type RenderOpts = LoadComponentsReturnType & RenderOptsPartial

View file

@ -70,7 +70,7 @@ export function appendMutableCookies(
}
// Return a new response that extends the response with
// the modified cookies as fallbacks. `res`' cookies
// the modified cookies as fallbacks. `res` cookies
// will still take precedence.
const resCookies = new ResponseCookies(headers)
const returnedCookies = resCookies.getAll()

View file

@ -20,14 +20,6 @@ createNextDescribe(
},
},
({ next, isNextDev, isNextStart, isNextDeploy }) => {
if (isNextStart) {
it('should warn for server actions + ISR incompat', async () => {
expect(next.cliOutput).toContain(
'Using server actions on a page currently disables static generation for that page'
)
})
}
it('should handle basic actions correctly', async () => {
const browser = await next.browser('/server')
@ -499,6 +491,17 @@ createNextDescribe(
})
describe('fetch actions', () => {
it('should handle a fetch action initiated from a static page', async () => {
const browser = await next.browser('/client-static')
await check(() => browser.elementByCss('#count').text(), '0')
await browser.elementByCss('#increment').click()
await check(() => browser.elementByCss('#count').text(), '1')
await browser.elementByCss('#increment').click()
await check(() => browser.elementByCss('#count').text(), '2')
})
it('should handle redirect to a relative URL in a single pass', async () => {
const browser = await next.browser('/client')

View file

@ -0,0 +1,16 @@
import { Counter } from '../../../components/Counter'
import { incrementCounter } from '../actions'
export default function Page() {
return (
<div>
<Counter onClick={incrementCounter} />
</div>
)
}
export const revalidate = 60
export async function generateStaticParams() {
return [{ path: ['asdf'] }]
}

View file

@ -0,0 +1,10 @@
'use server'
let counter = 0
export async function incrementCounter() {
console.log('Button clicked!')
counter++
return counter
}

View file

@ -0,0 +1,3 @@
export default function Layout({ children }) {
return children
}

View file

@ -0,0 +1,20 @@
'use client'
import React from 'react'
export function Counter({ onClick }) {
const [count, setCount] = React.useState(0)
return (
<>
<h1 id="count">{count}</h1>
<button
id="increment"
onClick={async () => {
const newCount = await onClick()
setCount(newCount)
}}
>
+1
</button>
</>
)
}

File diff suppressed because it is too large Load diff