Skip to content

Commit

Permalink
Honor redirect type in server actions (#70279)
Browse files Browse the repository at this point in the history
Fixes #70216 

### What?
The `redirect` function from `next/navigation` does not honor the
redirect type when used in server actions. When setting `type` to
`replace`, the server action pushes to history instead of replacing the
route. This stems from #54458

### Why?
The server action should honor the redirect type.

### How?
By setting `x-action-redirect` header to `<url>;<redirectType>`, we
extract the type of redirect to be performed on the client. This is
handled in a way that if the type is absent, we fallback to `push`.

Demo:


https://github.com/user-attachments/assets/3c84c6e3-2b08-4df3-89b2-5f9d290cc5ff

Observe that once the form is submitted and the action replaced the URL
with `/`, we didn't go back to the `/child` route.

---------

Co-authored-by: Zack Tanner <1939140+ztanner@users.noreply.github.com>
  • Loading branch information
abhi12299 and ztanner authored Sep 20, 2024
1 parent 496ff01 commit a840410
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 11 deletions.
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)
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
18 changes: 18 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,24 @@ 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')

let historyLen = await browser.eval('window.history.length')
// chromium's about:blank page is the first item in history
expect(historyLen).toBe(2)

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

// Ensure we cannot navigate back
historyLen = await browser.eval('window.history.length')
// chromium's about:blank page is the first item in history
expect(historyLen).toBe(2)
})

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>
)
}

0 comments on commit a840410

Please sign in to comment.