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

Honor redirect type in server actions #70279

Merged
merged 8 commits into from
Sep 20, 2024
7 changes: 5 additions & 2 deletions packages/next/src/client/components/redirect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,21 @@ export function getRedirectError(
*
* - In a Server Component, this will insert a meta tag to redirect the user to the target page.
* - In a Route Handler or Server Action, it will serve a 307/303 to the caller.
* - In a Server Action, type defaults to 'push' and 'replace' elsewhere.
*
* Read more: [Next.js Docs: `redirect`](https://nextjs.org/docs/app/api-reference/functions/redirect)
*/
export function redirect(
/** The URL to redirect to */
url: string,
type: RedirectType = RedirectType.replace
type?: RedirectType
): never {
const actionStore = actionAsyncStorage.getStore()
const redirectType =
type || (actionStore?.isAction ? RedirectType.push : RedirectType.replace)
ztanner marked this conversation as resolved.
Show resolved Hide resolved
throw getRedirectError(
url,
type,
redirectType,
// If we're in an action, we want to use a 303 redirect
// as we don't want the POST request to follow the redirect,
// as it could result in erroneous re-submissions.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import { hasBasePath } from '../../../has-base-path'

type FetchServerActionResult = {
redirectLocation: URL | undefined
redirectType: RedirectType | undefined
actionResult?: ActionResult
actionFlightData?: NormalizedFlightData[] | string
isPrerender: boolean
Expand Down Expand Up @@ -91,7 +92,20 @@ async function fetchServerAction(
body,
})

const location = res.headers.get('x-action-redirect')
const redirectHeader = res.headers.get('x-action-redirect')
const [location, _redirectType] = redirectHeader?.split(';') || []
let redirectType: RedirectType | undefined
switch (_redirectType) {
case 'push':
redirectType = RedirectType.push
break
case 'replace':
redirectType = RedirectType.replace
break
default:
redirectType = undefined
}

const isPrerender = !!res.headers.get(NEXT_IS_PRERENDER_HEADER)
let revalidatedParts: FetchServerActionResult['revalidatedParts']
try {
Expand Down Expand Up @@ -134,6 +148,7 @@ async function fetchServerAction(
return {
actionFlightData: normalizeFlightData(response.f),
redirectLocation,
redirectType,
revalidatedParts,
isPrerender,
}
Expand All @@ -143,6 +158,7 @@ async function fetchServerAction(
actionResult: response.a,
actionFlightData: normalizeFlightData(response.f),
redirectLocation,
redirectType,
revalidatedParts,
isPrerender,
}
Expand All @@ -162,6 +178,7 @@ async function fetchServerAction(

return {
redirectLocation,
redirectType,
revalidatedParts,
isPrerender,
}
Expand Down Expand Up @@ -197,13 +214,18 @@ export function serverActionReducer(
actionResult,
actionFlightData: flightData,
redirectLocation,
redirectType,
isPrerender,
}) => {
// Make sure the redirection is a push instead of a replace.
// Issue: https://github.com/vercel/next.js/issues/53911
// honor the redirect type instead of defaulting to push in case of server actions.
if (redirectLocation) {
state.pushRef.pendingPush = true
mutable.pendingPush = true
if (redirectType === RedirectType.replace) {
state.pushRef.pendingPush = false
mutable.pendingPush = false
} else {
state.pushRef.pendingPush = true
mutable.pendingPush = true
}
}

if (!flightData) {
Expand Down Expand Up @@ -263,7 +285,7 @@ export function serverActionReducer(
reject(
getRedirectError(
hasBasePath(newHref) ? removeBasePath(newHref) : newHref,
RedirectType.push
redirectType || RedirectType.push
)
)

Expand Down
7 changes: 6 additions & 1 deletion packages/next/src/server/app-render/action-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ import {
import { isNotFoundError } from '../../client/components/not-found'
import {
getRedirectStatusCodeFromError,
getRedirectTypeFromError,
getURLFromRedirectError,
isRedirectError,
type RedirectType,
} from '../../client/components/redirect'
import RenderResult from '../render-result'
import type { StaticGenerationStore } from '../../client/components/static-generation-async-storage.external'
Expand Down Expand Up @@ -276,10 +278,11 @@ async function createRedirectRenderResult(
res: BaseNextResponse,
originalHost: Host,
redirectUrl: string,
redirectType: RedirectType,
basePath: string,
staticGenerationStore: StaticGenerationStore
) {
res.setHeader('x-action-redirect', redirectUrl)
res.setHeader('x-action-redirect', `${redirectUrl};${redirectType}`)

// If we're redirecting to another route of this Next.js application, we'll
// try to stream the response from the other worker path. When that works,
Expand Down Expand Up @@ -841,6 +844,7 @@ export async function handleAction({
if (isRedirectError(err)) {
const redirectUrl = getURLFromRedirectError(err)
const statusCode = getRedirectStatusCodeFromError(err)
const redirectType = getRedirectTypeFromError(err)

await addRevalidationHeader(res, {
staticGenerationStore,
Expand All @@ -859,6 +863,7 @@ export async function handleAction({
res,
host,
redirectUrl,
redirectType,
ctx.renderOpts.basePath,
staticGenerationStore
),
Expand Down
14 changes: 14 additions & 0 deletions test/e2e/app-dir/actions/app-action.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,20 @@ describe('app-dir action handling', () => {
}, 'setCookieAndRedirect')
})

it('should replace current route when redirecting with type set to replace', async () => {
const browser = await next.browser('/header')

await browser.elementByCss('#setCookieAndRedirectReplace').click()
await check(async () => {
return (await browser.elementByCss('#redirected').text()) || ''
}, 'redirected')

// Ensure we cannot navigate back
const historyLen = await browser.eval('window.history.length')
// chromium's about:blank page is the first item in history
expect(historyLen).toBe(2)
abhi12299 marked this conversation as resolved.
Show resolved Hide resolved
})

it('should support headers in client imported actions', async () => {
const logs: string[] = []
next.on('stdout', (log) => {
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/app-dir/actions/app/header/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export async function setCookie(name, value) {
return cookies().get(name)
}

export async function setCookieAndRedirect(name, value, path) {
export async function setCookieAndRedirect(name, value, path, type) {
cookies().set(name, value)
redirect(path)
redirect(path, type)
}
15 changes: 15 additions & 0 deletions test/e2e/app-dir/actions/app/header/ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,21 @@ export default function UI({
setCookieAndRedirect
</button>
</form>
<form>
<button
id="setCookieAndRedirectReplace"
formAction={async () => {
await setCookieAndRedirect(
'redirect',
Math.random().toString(36).substring(7),
'/redirect-target',
'replace'
)
}}
>
setCookieAndRedirectReplace
</button>
</form>
</div>
)
}
Loading