Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: server actions initiated from static pages #51534

Merged
merged 14 commits into from
Sep 14, 2023
102 changes: 50 additions & 52 deletions packages/next/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import loadCustomRoutes, {
normalizeRouteRegex,
Redirect,
Rewrite,
RouteHas,
RouteType,
} from '../lib/load-custom-routes'
import { getRedirectStatus, modifyRouteRegex } from '../lib/redirect-status'
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -160,13 +162,15 @@ export type SsgRoute = {
dataRoute: string | null
initialStatus?: number
initialHeaders?: Record<string, string>
experimentalBypassFor?: RouteHas[]
}

export type DynamicSsgRoute = {
routeRegex: string
fallback: string | null | false
dataRoute: string | null
dataRouteRegex: string | null
experimentalBypassFor?: RouteHas[]
}

export type PrerenderManifest = {
Expand Down Expand Up @@ -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
Expand All @@ -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
}
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -2748,6 +2744,7 @@ export default async function build(

finalPrerenderRoutes[route] = {
...routeMeta,
experimentalBypassFor: bypassFor,
initialRevalidateSeconds: revalidate,
srcRoute: page,
dataRoute,
Expand All @@ -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
),
Expand Down
10 changes: 10 additions & 0 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
17 changes: 14 additions & 3 deletions packages/next/src/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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 =
ztanner marked this conversation as resolved.
Show resolved Hide resolved
actionId !== undefined &&
typeof actionId === 'string' &&
req.method === 'POST'
const isServerAction = isFetchAction || isMultipartAction
const hasGetInitialProps = !!components.Component?.getInitialProps
let isSSG = !!components.getStaticProps

Expand Down Expand Up @@ -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' &&
Expand Down Expand Up @@ -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
? ''
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions packages/next/src/server/render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ export type RenderOptsPartial = {
strictNextHead: boolean
isDraftMode?: boolean
deploymentId?: string
isServerAction?: boolean
}

export type RenderOpts = LoadComponentsReturnType & RenderOptsPartial
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
19 changes: 11 additions & 8 deletions test/e2e/app-dir/actions/app-action.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down Expand Up @@ -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')

Expand Down
16 changes: 16 additions & 0 deletions test/e2e/app-dir/actions/app/client-static/[[...path]]/page.js
Original file line number Diff line number Diff line change
@@ -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'] }]
}
10 changes: 10 additions & 0 deletions test/e2e/app-dir/actions/app/client-static/actions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
'use server'

let counter = 0

export async function incrementCounter() {
console.log('Button clicked!')

counter++
return counter
}
3 changes: 3 additions & 0 deletions test/e2e/app-dir/actions/app/client-static/layout.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Layout({ children }) {
return children
}
20 changes: 20 additions & 0 deletions test/e2e/app-dir/actions/components/Counter.tsx
Original file line number Diff line number Diff line change
@@ -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>
</>
)
}