From a39cc14bf117b3128d3a886c91c8026ba67d667e Mon Sep 17 00:00:00 2001
From: Zack Tanner <1939140+ztanner@users.noreply.github.com>
Date: Thu, 21 Nov 2024 14:55:43 -0500
Subject: [PATCH] fix: server action redirects between multiple root layouts
(#73063)
When handling a redirect through a server action, we pass in the current
href rather than the destination href to `handleExternalUrl`, which is
incorrect since that won't reflect the updated URL from a server
action's redirect response.
While looking into this, I noticed a handful of server action branches
that weren't properly rejecting or resolving the action promises. This
would cause the transitions to stall until they were fulfilled, so I
updated all of the existing branches where we early return to also
resolve the promise.
Fixes #72842
---
.../reducers/server-action-reducer.ts | 29 ++++++++++++-------
.../root-layout-redirect/app/actions.ts | 7 +++++
.../(root-a)/root-layout-a/page.js | 19 +++++++++---
.../app-dir/root-layout/root-layout.test.ts | 12 ++++++++
4 files changed, 53 insertions(+), 14 deletions(-)
create mode 100644 test/e2e/app-dir/root-layout-redirect/app/actions.ts
diff --git a/packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts b/packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts
index 1f52c038bf29bb..40567f37ea2467 100644
--- a/packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts
+++ b/packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts
@@ -206,7 +206,6 @@ export function serverActionReducer(
): ReducerState {
const { resolve, reject } = action
const mutable: ServerActionMutable = {}
- const href = state.canonicalUrl
let currentTree = state.tree
@@ -230,6 +229,8 @@ export function serverActionReducer(
isPrerender,
revalidatedParts,
}) => {
+ let redirectHref: string | undefined
+
// honor the redirect type instead of defaulting to push in case of server actions.
if (redirectLocation) {
if (redirectType === RedirectType.replace) {
@@ -239,6 +240,9 @@ export function serverActionReducer(
state.pushRef.pendingPush = true
mutable.pendingPush = true
}
+
+ redirectHref = createHrefFromUrl(redirectLocation, false)
+ mutable.canonicalUrl = redirectHref
}
if (!flightData) {
@@ -258,6 +262,8 @@ export function serverActionReducer(
if (typeof flightData === 'string') {
// Handle case when navigating to page in `pages` from `app`
+ resolve(actionResult)
+
return handleExternalUrl(
state,
mutable,
@@ -282,6 +288,8 @@ export function serverActionReducer(
if (!isRootRender) {
// TODO-APP: handle this case better
console.log('SERVER ACTION APPLY FAILED')
+ resolve(actionResult)
+
return state
}
@@ -291,20 +299,22 @@ export function serverActionReducer(
[''],
currentTree,
treePatch,
- redirectLocation
- ? createHrefFromUrl(redirectLocation)
- : state.canonicalUrl
+ redirectHref ? redirectHref : state.canonicalUrl
)
if (newTree === null) {
+ resolve(actionResult)
+
return handleSegmentMismatch(state, action, treePatch)
}
if (isNavigatingToNewRootLayout(currentTree, newTree)) {
+ resolve(actionResult)
+
return handleExternalUrl(
state,
mutable,
- href,
+ redirectHref || state.canonicalUrl,
state.pushRef.pendingPush
)
}
@@ -343,10 +353,7 @@ export function serverActionReducer(
currentTree = newTree
}
- if (redirectLocation) {
- const newHref = createHrefFromUrl(redirectLocation, false)
- mutable.canonicalUrl = newHref
-
+ if (redirectLocation && redirectHref) {
// Because the RedirectBoundary will trigger a navigation, we need to seed the prefetch cache
// with the FlightData that we got from the server action for the target page, so that it's
// available when the page is navigated to and doesn't need to be re-fetched.
@@ -381,7 +388,9 @@ export function serverActionReducer(
// a response with the correct status code.
reject(
getRedirectError(
- hasBasePath(newHref) ? removeBasePath(newHref) : newHref,
+ hasBasePath(redirectHref)
+ ? removeBasePath(redirectHref)
+ : redirectHref,
redirectType || RedirectType.push
)
)
diff --git a/test/e2e/app-dir/root-layout-redirect/app/actions.ts b/test/e2e/app-dir/root-layout-redirect/app/actions.ts
new file mode 100644
index 00000000000000..5503157fd1880c
--- /dev/null
+++ b/test/e2e/app-dir/root-layout-redirect/app/actions.ts
@@ -0,0 +1,7 @@
+'use server'
+
+import { redirect } from 'next/navigation'
+
+export async function redirectAction() {
+ redirect('/result')
+}
diff --git a/test/e2e/app-dir/root-layout/app/(multiple-root)/(root-a)/root-layout-a/page.js b/test/e2e/app-dir/root-layout/app/(multiple-root)/(root-a)/root-layout-a/page.js
index 6c5a6eefe3dbf2..48137a0517c2f1 100644
--- a/test/e2e/app-dir/root-layout/app/(multiple-root)/(root-a)/root-layout-a/page.js
+++ b/test/e2e/app-dir/root-layout/app/(multiple-root)/(root-a)/root-layout-a/page.js
@@ -1,10 +1,21 @@
import React from 'react'
import Link from 'next/link'
+import { redirect } from 'next/navigation'
+
+export default async function Page() {
+ async function redirectAction() {
+ 'use server'
+ redirect('/root-layout-b')
+ }
-export default function Page() {
return (
-
- To b
-
+ <>
+
+ To b
+
+
+
)
}
diff --git a/test/e2e/app-dir/root-layout/root-layout.test.ts b/test/e2e/app-dir/root-layout/root-layout.test.ts
index d2d4d866c8e403..45c413e7b8c257 100644
--- a/test/e2e/app-dir/root-layout/root-layout.test.ts
+++ b/test/e2e/app-dir/root-layout/root-layout.test.ts
@@ -224,4 +224,16 @@ describe('app-dir root layout', () => {
.waitForElementByCss('#root-b')
expect(await browser.hasElementByCssSelector('#root-a')).toBeFalse()
})
+
+ it('should correctly handle navigation between multiple root layouts when redirecting in a server action', async () => {
+ const browser = await next.browser('/root-layout-a')
+
+ await browser.waitForElementByCss('#action-redirect-to-b')
+ expect(await browser.hasElementByCssSelector('#root-b')).toBeFalse()
+ await browser
+ .elementById('action-redirect-to-b')
+ .click()
+ .waitForElementByCss('#root-b')
+ expect(await browser.hasElementByCssSelector('#root-a')).toBeFalse()
+ })
})