bugfix: actions that self-redirect should be handled gracefully

packages/next/src/client/components/app-router.tsx

@@ -58,6 +58,12 @@ import type { FlightRouterState } from '../../server/app-render/types'
 import { useNavFailureHandler } from './nav-failure-handler'
 import { useServerActionDispatcher } from '../app-call-server'
 import type { AppRouterActionQueue } from '../../shared/lib/router/action-queue'
+import {
+  getRedirectTypeFromError,
+  getURLFromRedirectError,
+  isRedirectError,
+  RedirectType,
+} from './redirect'
 
 const globalMutable: {
   pendingMpaPath?: string
@@ -369,6 +375,31 @@ function Router({
     }
   }, [dispatch])
 
+  useEffect(() => {
+    // Ensure that any redirect errors that bubble up outside of the RedirectBoundary
+    // are caught and handled by the router.
+    function handle(event: ErrorEvent | PromiseRejectionEvent) {
+      const error = 'reason' in event ? event.reason : event.error
+      if (isRedirectError(error)) {
+        event.preventDefault()
+        const url = getURLFromRedirectError(error)
+        const redirectType = getRedirectTypeFromError(error)
+        if (redirectType === RedirectType.push) {
+          appRouter.push(url, {})
+        } else {
+          appRouter.replace(url, {})
+        }
+      }
+    }
+    window.addEventListener('error', handle)
+    window.addEventListener('unhandledrejection', handle)
+
+    return () => {
+      window.removeEventListener('error', handle)
+      window.removeEventListener('unhandledrejection', handle)
+    }
+  }, [appRouter])
+
   // When mpaNavigation flag is set do a hard navigation to the new url.
   // Infinitely suspend because we don't actually want to rerender any child
   // components with the new URL and any entangled state updates shouldn't

packages/next/src/client/components/react-dev-overlay/internal/helpers/use-error-handler.ts

@@ -58,6 +58,11 @@ if (typeof window !== 'undefined') {
     'unhandledrejection',
     (ev: WindowEventMap['unhandledrejection']): void => {
       const reason = ev?.reason
+      if (isNextRouterError(reason)) {
+        ev.preventDefault()
+        return
+      }
+
       if (
         !reason ||
         !(reason instanceof Error) ||

packages/next/src/client/components/router-reducer/create-initial-router-state.ts

@@ -4,8 +4,8 @@ import type { FlightDataPath } from '../../../server/app-render/types'
 import { createHrefFromUrl } from './create-href-from-url'
 import { fillLazyItemsTillLeafWithHead } from './fill-lazy-items-till-leaf-with-head'
 import { extractPathFromFlightRouterState } from './compute-changed-path'
-import { createPrefetchCacheEntryForInitialLoad } from './prefetch-cache-utils'
-import type { PrefetchCacheEntry } from './router-reducer-types'
+import { createSeededPrefetchCacheEntry } from './prefetch-cache-utils'
+import { PrefetchKind, type PrefetchCacheEntry } from './router-reducer-types'
 import { addRefreshMarkerToActiveParallelSegments } from './refetch-inactive-parallel-segments'
 import { getFlightDataPartsFromPath } from '../../flight-data-helpers'
 
@@ -112,7 +112,7 @@ export function createInitialRouterState({
       location.origin
     )
 
-    createPrefetchCacheEntryForInitialLoad({
+    createSeededPrefetchCacheEntry({
       url,
       data: {
         flightData: [normalizedFlightData],
@@ -125,6 +125,7 @@ export function createInitialRouterState({
       tree: initialState.tree,
       prefetchCache: initialState.prefetchCache,
       nextUrl: initialState.nextUrl,
+      kind: PrefetchKind.AUTO,
     })
   }
 

packages/next/src/client/components/router-reducer/prefetch-cache-utils.ts

@@ -249,19 +249,20 @@ function prefixExistingPrefetchCacheEntry({
 /**
  * Use to seed the prefetch cache with data that has already been fetched.
  */
-export function createPrefetchCacheEntryForInitialLoad({
+export function createSeededPrefetchCacheEntry({
   nextUrl,
   tree,
   prefetchCache,
   url,
   data,
+  kind,
 }: Pick<ReadonlyReducerState, 'nextUrl' | 'tree' | 'prefetchCache'> & {
   url: URL
   data: FetchServerResponseResult
+  kind: PrefetchKind
 }) {
   // The initial cache entry technically includes full data, but it isn't explicitly prefetched -- we just seed the
   // prefetch cache so that we can skip an extra prefetch request later, since we already have the data.
-  const kind = PrefetchKind.AUTO
   // if the prefetch corresponds with an interception route, we use the nextUrl to prefix the cache key
   const prefetchCacheKey = data.couldBeIntercepted
     ? createPrefetchCacheKey(url, kind, nextUrl)

packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts

@@ -21,11 +21,12 @@ const { createFromFetch, encodeReply } = (
       require('react-server-dom-webpack/client')
 ) as typeof import('react-server-dom-webpack/client')
 
-import type {
-  ReadonlyReducerState,
-  ReducerState,
-  ServerActionAction,
-  ServerActionMutable,
+import {
+  PrefetchKind,
+  type ReadonlyReducerState,
+  type ReducerState,
+  type ServerActionAction,
+  type ServerActionMutable,
 } from '../router-reducer-types'
 import { addBasePath } from '../../../add-base-path'
 import { createHrefFromUrl } from '../create-href-from-url'
@@ -43,6 +44,10 @@ import {
   normalizeFlightData,
   type NormalizedFlightData,
 } from '../../../flight-data-helpers'
+import { getRedirectError, RedirectType } from '../../redirect'
+import { createSeededPrefetchCacheEntry } from '../prefetch-cache-utils'
+import { removeBasePath } from '../../../remove-base-path'
+import { hasBasePath } from '../../../has-base-path'
 
 type FetchServerActionResult = {
   redirectLocation: URL | undefined
@@ -221,7 +226,41 @@ export function serverActionReducer(
 
       if (redirectLocation) {
         const newHref = createHrefFromUrl(redirectLocation, false)
-        mutable.canonicalUrl = newHref
+
+        // 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.
+        createSeededPrefetchCacheEntry({
+          url: redirectLocation,
+          data: {
+            flightData,
+            canonicalUrl: undefined,
+            couldBeIntercepted: false,
+            isPrerender: false,
+            postponed: false,
+          },
+          tree: state.tree,
+          prefetchCache: state.prefetchCache,
+          nextUrl: state.nextUrl,
+          kind: PrefetchKind.FULL,
+        })
+
+        mutable.prefetchCache = state.prefetchCache
+        // If the action triggered a redirect, we reject the action promise with a redirect
+        // so that it's handled by RedirectBoundary as we won't have a valid
+        // action result to resolve the promise with. This will effectively reset the state of
+        // the component that called the action as the error boundary will remount the tree.
+        // The status code doesn't matter here as the action handler will have already sent
+        // a response with the correct status code.
+
+        reject(
+          getRedirectError(
+            hasBasePath(newHref) ? removeBasePath(newHref) : newHref,
+            RedirectType.push
+          )
+        )
+
+        return handleMutable(state, mutable)
       }
 
       for (const normalizedFlightData of flightData) {

packages/next/src/shared/lib/router/action-queue.ts

@@ -3,8 +3,6 @@ import {
   type AppRouterState,
   type ReducerActions,
   type ReducerState,
-  ACTION_REFRESH,
-  ACTION_SERVER_ACTION,
   ACTION_NAVIGATE,
   ACTION_RESTORE,
 } from '../../../client/components/router-reducer/router-reducer-types'
@@ -20,7 +18,6 @@ export type AppRouterActionQueue = {
   dispatch: (payload: ReducerActions, setState: DispatchStatePromise) => void
   action: (state: AppRouterState, action: ReducerActions) => ReducerState
   pending: ActionQueueNode | null
-  needsRefresh?: boolean
   last: ActionQueueNode | null
 }
 
@@ -45,18 +42,6 @@ function runRemainingActions(
         action: actionQueue.pending,
         setState,
       })
-    } else {
-      // No more actions are pending, check if a refresh is needed
-      if (actionQueue.needsRefresh) {
-        actionQueue.needsRefresh = false
-        actionQueue.dispatch(
-          {
-            type: ACTION_REFRESH,
-            origin: window.location.origin,
-          },
-          setState
-        )
-      }
     }
   }
 }
@@ -160,11 +145,6 @@ function dispatchAction(
     // Mark this action as the last in the queue
     actionQueue.last = newAction
 
-    // if the pending action was a server action, mark the queue as needing a refresh once events are processed
-    if (actionQueue.pending.payload.type === ACTION_SERVER_ACTION) {
-      actionQueue.needsRefresh = true
-    }
-
     runAction({
       actionQueue,
       action: newAction,

test/e2e/app-dir/actions/app-action.test.ts

@@ -564,6 +564,64 @@ describe('app-dir action handling', () => {
     await check(() => browser.elementByCss('h1').text(), 'Transition is: idle')
   })
 
+  it('should reset the form state when the action redirects to a page that contains the same form', async () => {
+    const browser = await next.browser('/redirect')
+    const input = await browser.elementByCss('input[name="name"]')
+    const submit = await browser.elementByCss('button')
+
+    expect(await browser.hasElementByCssSelector('#error')).toBe(false)
+
+    await input.fill('foo')
+    await submit.click()
+
+    // The server action will fail validation and will return error state
+    // verify that the error state is displayed
+    await retry(async () => {
+      expect(await browser.hasElementByCssSelector('#error')).toBe(true)
+      expect(await browser.elementByCss('#error').text()).toBe(
+        "Only 'justputit' is accepted."
+      )
+    })
+
+    // The server action won't return an error state, it will just call redirect to itself
+    // Validate that the form state is reset
+    await input.fill('justputit')
+    await submit.click()
+
+    await retry(async () => {
+      expect(await browser.hasElementByCssSelector('#error')).toBe(false)
+    })
+  })
+
+  it('should reset the form state when the action redirects to itself', async () => {
+    const browser = await next.browser('/self-redirect')
+    const input = await browser.elementByCss('input[name="name"]')
+    const submit = await browser.elementByCss('button')
+
+    expect(await browser.hasElementByCssSelector('#error')).toBe(false)
+
+    await input.fill('foo')
+    await submit.click()
+
+    // The server action will fail validation and will return error state
+    // verify that the error state is displayed
+    await retry(async () => {
+      expect(await browser.hasElementByCssSelector('#error')).toBe(true)
+      expect(await browser.elementByCss('#error').text()).toBe(
+        "Only 'justputit' is accepted."
+      )
+    })
+
+    // The server action won't return an error state, it will just call redirect to itself
+    // Validate that the form state is reset
+    await input.fill('justputit')
+    await submit.click()
+
+    await retry(async () => {
+      expect(await browser.hasElementByCssSelector('#error')).toBe(false)
+    })
+  })
+
   // This is disabled when deployed because the 404 page will be served as a static route
   // which will not support POST requests, and will return a 405 instead.
   if (!isNextDeploy) {

test/e2e/app-dir/actions/app/redirect/actions.ts

@@ -0,0 +1,17 @@
+'use server'
+
+import { redirect } from 'next/navigation'
+
+type State = {
+  errors: Record<string, string>
+}
+
+export async function action(previousState: State, formData: FormData) {
+  const name = formData.get('name')
+
+  if (name !== 'justputit') {
+    return { errors: { name: "Only 'justputit' is accepted." } }
+  }
+
+  redirect('/redirect/other')
+}

test/e2e/app-dir/actions/app/redirect/layout.tsx

@@ -0,0 +1,21 @@
+'use client'
+
+import React, { useActionState } from 'react'
+import { action } from './actions'
+
+export default function Page({ children }) {
+  const [{ errors }, dispatch] = useActionState(action, {
+    errors: { name: '' },
+  })
+
+  return (
+    <div>
+      <form action={dispatch}>
+        <input type="text" name="name" />
+        <button type="submit">Submit</button>
+        {errors.name && <p id="error">{errors.name}</p>}
+      </form>
+      {children}
+    </div>
+  )
+}

test/e2e/app-dir/actions/app/redirect/other/page.tsx

@@ -0,0 +1,3 @@
+export default function Page() {
+  return <div>Other Page</div>
+}

test/e2e/app-dir/actions/app/redirect/page.tsx

@@ -0,0 +1,3 @@
+export default function Page() {
+  return <div>Main Page</div>
+}

test/e2e/app-dir/actions/app/self-redirect/actions.ts

@@ -0,0 +1,17 @@
+'use server'
+
+import { redirect } from 'next/navigation'
+
+type State = {
+  errors: Record<string, string>
+}
+
+export async function action(previousState: State, formData: FormData) {
+  const name = formData.get('name')
+
+  if (name !== 'justputit') {
+    return { errors: { name: "Only 'justputit' is accepted." } }
+  }
+
+  redirect('/self-redirect')
+}

test/e2e/app-dir/actions/app/self-redirect/layout.tsx

@@ -0,0 +1,21 @@
+'use client'
+
+import React, { useActionState } from 'react'
+import { action } from './actions'
+
+export default function Page({ children }) {
+  const [{ errors }, dispatch] = useActionState(action, {
+    errors: { name: '' },
+  })
+
+  return (
+    <div>
+      <form action={dispatch}>
+        <input type="text" name="name" />
+        <button type="submit">Submit</button>
+        {errors.name && <p id="error">{errors.name}</p>}
+      </form>
+      {children}
+    </div>
+  )
+}

test/e2e/app-dir/actions/app/self-redirect/other/page.tsx

@@ -0,0 +1,3 @@
+export default function Page() {
+  return <div>Other Page</div>
+}

test/e2e/app-dir/actions/app/self-redirect/page.tsx

@@ -0,0 +1,3 @@
+export default function Page() {
+  return <div>Main Page</div>
+}