fix: default redirect() to "push" in Server Action context#779
Conversation
redirect() called inside a Server Action always used "replace" navigation, breaking the Back button after form submissions. Next.js defaults to "push" in action context via actionAsyncStorage. Instead of adding a new AsyncLocalStorage, use an empty string sentinel in the digest when no explicit type is provided. The catch site resolves the default based on context: "push" for Server Actions, "replace" for SSR renders. This matches Next.js behavior. Also adds an optional `type` parameter to permanentRedirect() matching the Next.js signature (defaults to "replace").
commit: |
|
/bigbonk review |
There was a problem hiding this comment.
Good fix. The approach of deferring type resolution to the catch site via an empty sentinel is a pragmatic alternative to adding a new AsyncLocalStorage (which is what Next.js uses with actionAsyncStorage). The SSR catch sites (app-route-handler-policy.ts, app-page-execution.ts) only extract location and statusCode from the digest and ignore parts[1], so the empty sentinel is harmless there. The action handler correctly resolves empty to "push", and the browser entry already falls back to "replace" when the header is absent.
Tests are thorough — they cover the empty sentinel, explicit types, semicolon injection, permanentRedirect with optional type, and the generated template assertion. The permanentRedirect change correctly matches Next.js, which also defaults to "replace" without isAction context-dependence.
One minor observation and one question below, but neither blocks merging.
| throw new VinextNavigationError( | ||
| `NEXT_REDIRECT:${url}`, | ||
| `NEXT_REDIRECT;${type ?? "replace"};${encodeURIComponent(url)}`, | ||
| `NEXT_REDIRECT;${type ?? ""};${encodeURIComponent(url)}`, |
There was a problem hiding this comment.
Nit: worth noting a subtle difference from Next.js here. Next.js resolves the type at call time using actionAsyncStorage.getStore()?.isAction, so the digest always carries an explicit "push" or "replace". Vinext defers it to the catch site using an empty sentinel.
This works correctly today because the only catch site that cares about the type is the Server Action handler (which defaults empty → "push"). The SSR catch sites (app-page-execution.ts, app-route-handler-policy.ts) ignore the type field entirely.
If a future catch site needs the type and doesn't know whether it's in an action context, the sentinel approach would need revisiting. Not a blocker — just documenting the trade-off for future readers.
| export function permanentRedirect(url: string): never { | ||
| export function permanentRedirect( | ||
| url: string, | ||
| type: "replace" | "push" | RedirectType = "replace", |
There was a problem hiding this comment.
Next.js also does not use isAction-dependent defaulting for permanentRedirect — it always defaults to "replace". This matches. Good.
|
Review submitted. Here's a summary of my assessment: Approved. The PR correctly fixes What the PR does well:
One observation left as a comment: The sentinel approach differs from Next.js's call-time resolution. If a future catch site needs the type without knowing whether it's in an action context, this would need revisiting. Not blocking — just documented for future readers. |
Summary
redirect()called inside a Server Action always used"replace"navigation, breaking the Back button after form submissions. Next.js defaults to"push"in action context (viaactionAsyncStorage.getStore()?.isAction)."push", while SSR render contexts only extractlocation/statusCode(no type needed at HTTP level).typeparameter topermanentRedirect()matching the Next.js signature (defaults to"replace").How it works
redirect("/foo")now produces digestNEXT_REDIRECT;;%2Ffoo(empty type field)redirect("/foo", "push")producesNEXT_REDIRECT;push;%2Ffoo(explicit, always respected)app-rsc-entry.tsresolves empty to"push"(parts[1] || "push")x-action-redirect-typeheader and useswindow.location.assign()for push /window.location.replace()for replaceNext.js reference
packages/next/src/client/components/redirect.ts— line 43:type ??= actionAsyncStorage?.getStore()?.isAction ? 'push' : 'replace'packages/next/src/client/components/redirect.ts— line 62:permanentRedirectaccepts optionaltypeparameterTest plan
redirect()without explicit type produces empty sentinel in digestredirect()with explicit"push"or"replace"preserves type in digestpermanentRedirect()accepts optionaltypeparameterpermanentRedirect()defaults to"replace"when no type given"push"(template assertion)