fix: resolve relative server action redirects#1579
Open
NathanDrake2406 wants to merge 3 commits into
Open
Conversation
commit: |
Contributor
There was a problem hiding this comment.
Pull request overview
Aligns vinext’s Server Action redirect handling with Next.js semantics for dot-relative locations, ensuring redirects resolve against the action initiation route and that same-origin redirects can stay on the App Router client navigation path (avoiding document reloads).
Changes:
- Added
resolveServerActionRedirectLocation()to resolve dot-relative redirect targets against the server action initiation URL. - Updated browser server-action redirect handling to route same-origin redirects through the navigation runtime when available, falling back to hard navigations otherwise.
- Added unit + Playwright E2E coverage and fixture pages for relative/absolute/multi-level redirects.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/fixtures/app-basic/app/nextjs-compat/server-actions-relative-redirect/subpage/page.tsx | Adds a target page for root-level relative redirect assertions. |
| tests/fixtures/app-basic/app/nextjs-compat/server-actions-relative-redirect/subdir/subpage/page.tsx | Adds a nested target page for subdir relative redirect assertions. |
| tests/fixtures/app-basic/app/nextjs-compat/server-actions-relative-redirect/subdir/page.tsx | Adds client UI buttons to trigger server actions from a nested route. |
| tests/fixtures/app-basic/app/nextjs-compat/server-actions-relative-redirect/page.tsx | Adds client UI buttons to trigger server actions from the root route. |
| tests/fixtures/app-basic/app/nextjs-compat/server-actions-relative-redirect/actions.ts | Adds server actions that redirect via ./, ../, and absolute targets. |
| tests/e2e/app-router/nextjs-compat/server-actions-relative-redirect.spec.ts | Adds Playwright regression coverage for Next.js-compatible relative redirect behavior and “no reload” expectation. |
| tests/app-browser-entry.test.ts | Adds unit tests for redirect URL resolution + internal/external classification. |
| packages/vinext/src/server/app-browser-entry.ts | Updates server action redirect handling to resolve dot-relative targets and use navigation runtime for same-origin redirects. |
| packages/vinext/src/server/app-browser-action-result.ts | Introduces helper to resolve action redirect locations with Next.js directory-style dot-relative semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
67478ab to
7e44827
Compare
7e44827 to
f148fb9
Compare
Server action redirects could carry dot-relative locations from the action response. Treating those locations as raw browser redirects resolved them from the current document URL and forced a reload, so nested App Router actions could land on the wrong route or lose client state. Resolve server action redirect locations against the action initiation pathname using Next.js directory-style semantics, keep same-origin redirects on the App Router navigation path, and hard-navigate only for external destinations or parse fallback. Adds browser-level coverage ported from the Next.js relative server action redirect test plus focused helper assertions for the URL resolution contract.
f148fb9 to
68250cd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
packages/vinext/src/server/app-browser-action-result.ts,packages/vinext/src/server/app-browser-entry.ts,tests/e2e/app-router/nextjs-compat/server-actions-relative-redirect.spec.tsredirect("./...")orredirect("../...")land on the same routes as Next.js and do not force a document reload for same-origin App Router navigation.Why
Server action redirect locations are relative to the route that initiated the action, not to an arbitrary later browser document URL, and same-origin action redirects should remain in the App Router client navigation lifecycle. The previous path treated the raw
x-action-redirectvalue as a hard navigation target, which could resolve nested./subpageredirects incorrectly and drop client state.What changed
redirect("./subpage")from a nested routeredirect("../subpage")from a nested routeValidation
PLAYWRIGHT_PROJECT=app-router pnpm run test:e2e -- tests/e2e/app-router/nextjs-compat/server-actions-relative-redirect.spec.tsvp test run tests/app-browser-entry.test.ts -t "server action"vp fmt --check packages/vinext/src/server/app-browser-action-result.ts packages/vinext/src/server/app-browser-entry.ts tests/app-browser-entry.test.ts tests/e2e/app-router/nextjs-compat/server-actions-relative-redirect.spec.ts tests/fixtures/app-basic/app/nextjs-compat/server-actions-relative-redirect/actions.ts tests/fixtures/app-basic/app/nextjs-compat/server-actions-relative-redirect/page.tsx tests/fixtures/app-basic/app/nextjs-compat/server-actions-relative-redirect/subdir/page.tsx tests/fixtures/app-basic/app/nextjs-compat/server-actions-relative-redirect/subdir/subpage/page.tsx tests/fixtures/app-basic/app/nextjs-compat/server-actions-relative-redirect/subpage/page.tsxvp env exec --node 24 ./scripts/run-nextjs-deploy-suite.sh /Users/nathan/Projects/vinext/.refs/nextjs-v16.2.6 --retries 0 -c 1 --debug test/e2e/app-dir/server-actions-relative-redirect/server-actions-relative-redirect.test.tsvp checkknip --no-progressRisk / compatibility
References