Skip to content
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
de4bf6f
fix: server action redirects use soft RSC navigation instead of hard …
yunus25jmi1 Mar 27, 2026
7a94313
fix: use manual glob implementation for Node compatibility
yunus25jmi1 Mar 27, 2026
ed29480
fix: complete soft RSC navigation for server action redirects
yunus25jmi1 Mar 27, 2026
c714eb3
fix: improve file-matcher glob handling and update snapshots
yunus25jmi1 Mar 27, 2026
c16cae3
fix: address review feedback for soft RSC navigation
yunus25jmi1 Mar 28, 2026
0fb28ce
test: update entry-templates snapshots after review fixes
yunus25jmi1 Mar 28, 2026
7a249da
fix: refactor scanWithExtensions to use glob for file matching
yunus25jmi1 Mar 29, 2026
28750cd
fix(server-actions): address round-3 review feedback for soft redirects
yunus25jmi1 Mar 31, 2026
0730add
test: update entry-templates snapshots after round-3 review fixes
yunus25jmi1 Mar 31, 2026
f14713c
fix(rewrites): include middleware headers in static file responses
yunus25jmi1 Mar 31, 2026
de691d7
fix(server-actions): address code review feedback for soft redirects
yunus25jmi1 Apr 1, 2026
6953ee9
fix(server-actions): complete soft RSC navigation for action redirects
yunus25jmi1 Apr 1, 2026
3de9712
fix(server-actions): address final review feedback
yunus25jmi1 Apr 1, 2026
c81c2f5
fix(server-actions): cleanup fallback headers context and apply middl…
yunus25jmi1 Apr 1, 2026
a932a5e
fix(server-actions): harden redirect fallback context and headers
yunus25jmi1 Apr 1, 2026
b532645
fix(server-actions): make redirect navigation atomic
yunus25jmi1 Apr 2, 2026
c7c5bca
Merge upstream/main and resolve app browser/navigation conflicts
yunus25jmi1 Apr 2, 2026
7cd9bac
test: update entry-template snapshots after merge conflict resolution
yunus25jmi1 Apr 2, 2026
6ce2636
Merge upstream/main and resolve app-rsc-entry snapshot conflicts
yunus25jmi1 Apr 3, 2026
612720f
fix(server-actions): sync latestClientParams during redirect soft-nav
yunus25jmi1 Apr 4, 2026
29dde11
chore(server-actions): refresh stale non-redirect navigation comment
yunus25jmi1 Apr 4, 2026
43b446a
fix(prod-server): serve static files for beforeFiles rewrite targets
yunus25jmi1 Apr 5, 2026
b5e38fa
clarify(app-rsc-entry): clarify cookie collection timing in redirect …
yunus25jmi1 Apr 5, 2026
db1c8ba
test: update snapshots for cookie collection comment change
yunus25jmi1 Apr 5, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 96 additions & 7 deletions packages/vinext/src/entries/app-rsc-entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1877,24 +1877,113 @@ async function _handleRequest(request, __reqCtx, _mwCtx) {
// We can't use a real HTTP redirect (the fetch would follow it automatically
// and receive a page HTML instead of RSC stream). Instead, we return a 200
// with x-action-redirect header that the client entry detects and handles.
//
// For same-origin routes, we pre-render the redirect target's RSC payload
// so the client can perform a soft RSC navigation (SPA-style) instead of
// a hard page reload. This matches Next.js behavior.
//
// Note: Middleware is NOT executed for the redirect target pre-render.
// This is a known limitation — the redirect target is rendered directly
// without going through the middleware pipeline.
if (actionRedirect) {
const actionPendingCookies = getAndClearPendingCookies();
const actionDraftCookie = getDraftModeCookieHeader();
setHeadersContext(null);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: headersContext is cleared here, but the pre-render at lines 1915-1931 builds and renders the redirect target page. If any server component in that page calls headers() or cookies(), it will get null context.

Worse, renderToReadableStream returns a lazily-consumed stream. Async server components that run during stream consumption (after return redirectResponse) will also see null headersContext. Compare with the normal re-render path at lines 2015-2019, which explicitly documents: "Do NOT clear headers/navigation context here — the RSC stream is consumed lazily."

Don't clear headersContext before the pre-render. Either leave it alive (matching the normal action re-render path), or create a fresh context for the redirect target:

Suggested change
setHeadersContext(null);
setHeadersContext(headersContextFromRequest(request));
setNavigationContext(null);

setNavigationContext(null);
const redirectHeaders = new Headers({

// Try to pre-render the redirect target for soft RSC navigation.
// This is the Next.js parity fix for issue #654.
try {
const redirectUrl = new URL(actionRedirect.url, request.url);

// Only pre-render same-origin URLs. External URLs fall through to
// the empty-body response, which triggers a hard redirect on the client.
if (redirectUrl.origin === new URL(request.url).origin) {
const redirectMatch = matchRoute(redirectUrl.pathname);

if (redirectMatch) {
const { route: redirectRoute, params: redirectParams } = redirectMatch;

// Set navigation context for the redirect target
setNavigationContext({
pathname: redirectUrl.pathname,
searchParams: redirectUrl.searchParams,
params: redirectParams,
});
Comment on lines +1946 to +1951
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: navigation context is not cleaned up on failure. If the try block throws after this point, setNavigationContext is left pointing at the redirect target. The catch block at line 1956 doesn't reset it.

This should be wrapped in a try/finally, or the catch block should call setNavigationContext(null).

Comment on lines +1946 to +1951
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this setNavigationContext call is correct for the pre-render, but if buildPageElement or renderToReadableStream throws, the catch block at line 1961 resets it to null. However, if the renderToReadableStream call succeeds but the stream itself errors during lazy consumption (after the return redirectResponse), the navigation context will be left pointing at the redirect target for any subsequent requests in the same isolate.

This is the same pattern as the normal RSC render path (line 1995-1999), so it's not a regression — just worth noting as a pre-existing concern with streaming responses.


// Build and render the redirect target page
const redirectElement = buildPageElement(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider extracting the redirect pre-render logic (lines 1918-2052) into a runtime helper in server/ rather than keeping it inline in the generated entry template. Per AGENTS.md:

Generated entry modules should stay thin. Move real behavior into normal typed modules under packages/vinext/src/server/* whenever the code involves request/response orchestration, streaming or teeing streams, redirect / not-found / access-fallback handling.

This would also reduce the 4x snapshot duplication (~750 new lines in snapshots are this same block repeated for each test variant).

redirectRoute,
redirectParams,
undefined,
redirectUrl.searchParams,
);

const redirectOnError = createRscOnErrorHandler(
request,
redirectUrl.pathname,
redirectRoute.pattern,
);

const rscStream = renderToReadableStream(
{ root: redirectElement, returnValue },
{ temporaryReferences, onError: redirectOnError },
);

// Collect cookies after rendering (same as normal action response)
const redirectPendingCookies = getAndClearPendingCookies();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: getAndClearPendingCookies() is called synchronously here, but the rscStream above is lazily consumed. Async server components in the redirect target that call cookies().set() during stream consumption will execute after this point, so their cookies will be missed.

This is the same structural limitation as the normal re-render path further down, so it's not a regression. But a comment noting this would help future readers:

Suggested change
const redirectPendingCookies = getAndClearPendingCookies();
// Collect cookies set synchronously during rendering. Note: cookies
// set by async server components during lazy stream consumption
// will not be captured here (same limitation as the normal re-render
// path below).
const redirectPendingCookies = getAndClearPendingCookies();

const redirectDraftCookie = getDraftModeCookieHeader();

const redirectHeaders = {
"Content-Type": "text/x-component; charset=utf-8",
"Vary": "RSC, Accept",
"x-action-redirect": actionRedirect.url,
"x-action-redirect-type": actionRedirect.type,
"x-action-redirect-status": String(actionRedirect.status),
"x-action-rsc-prerender": "1",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing X-Vinext-Params header. The client-side code at app-browser-entry.ts:185 reads this header to call setClientParams(). Without it, useParams() will return {} after a soft redirect to a dynamic route.

Add the params header (same as buildAppPageRscResponse does):

Suggested change
const redirectHeaders = {
"Content-Type": "text/x-component; charset=utf-8",
"Vary": "RSC, Accept",
"x-action-redirect": actionRedirect.url,
"x-action-redirect-type": actionRedirect.type,
"x-action-redirect-status": String(actionRedirect.status),
"x-action-rsc-prerender": "1",
const redirectHeaders = {
"Content-Type": "text/x-component; charset=utf-8",
"Vary": "RSC, Accept",
"x-action-redirect": actionRedirect.url,
"x-action-redirect-type": actionRedirect.type,
"x-action-redirect-status": String(actionRedirect.status),
"x-action-rsc-prerender": "1",
...(Object.keys(redirectParams).length > 0 ? { "X-Vinext-Params": encodeURIComponent(JSON.stringify(redirectParams)) } : {}),
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug (from previous review, still present): missing X-Vinext-Params header. The client at app-browser-entry.ts:185 reads this to call setClientParams(). Without it, useParams() returns {} after soft redirect to a dynamic route (e.g., /posts/[slug]).

Compare with buildAppPageRscResponse at server/app-page-response.ts:169-172.

Suggested change
"x-action-rsc-prerender": "1",
const redirectHeaders = {
"Content-Type": "text/x-component; charset=utf-8",
"Vary": "RSC, Accept",
"x-action-redirect": actionRedirect.url,
"x-action-redirect-type": actionRedirect.type,
"x-action-redirect-status": String(actionRedirect.status),
"x-action-rsc-prerender": "1",
...(Object.keys(redirectParams).length > 0 ? { "X-Vinext-Params": encodeURIComponent(JSON.stringify(redirectParams)) } : {}),
};

};
const redirectResponse = new Response(rscStream, {
status: 200,
headers: redirectHeaders,
});

// Append cookies (collected after rendering, not duplicated)
if (redirectPendingCookies.length > 0 || redirectDraftCookie) {
for (const cookie of redirectPendingCookies) {
redirectResponse.headers.append("Set-Cookie", cookie);
}
if (redirectDraftCookie) redirectResponse.headers.append("Set-Cookie", redirectDraftCookie);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: actionPendingCookies (cookies set by the server action before calling redirect()) are collected at line 1889 but never appended to this response. Only redirectPendingCookies (from the pre-render) are appended.

This means cookies like cookies().set('session', token) called before redirect('/dashboard') in a server action will be silently dropped.

Both sets of cookies need to be included:

Suggested change
// Append cookies (collected after rendering, not duplicated)
if (redirectPendingCookies.length > 0 || redirectDraftCookie) {
for (const cookie of redirectPendingCookies) {
redirectResponse.headers.append("Set-Cookie", cookie);
}
if (redirectDraftCookie) redirectResponse.headers.append("Set-Cookie", redirectDraftCookie);
}
if (actionPendingCookies.length > 0 || actionDraftCookie || redirectPendingCookies.length > 0 || redirectDraftCookie) {
for (const cookie of actionPendingCookies) {
redirectResponse.headers.append("Set-Cookie", cookie);
}
if (actionDraftCookie) redirectResponse.headers.append("Set-Cookie", actionDraftCookie);
for (const cookie of redirectPendingCookies) {
redirectResponse.headers.append("Set-Cookie", cookie);
}
if (redirectDraftCookie) redirectResponse.headers.append("Set-Cookie", redirectDraftCookie);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug (from previous review, still present): actionPendingCookies are collected at line 1889 but never appended here. Only redirectPendingCookies are included. This drops cookies set by the action before redirect() was called (e.g., cookies().set('session', token)).

Both sets need to be included:

Suggested change
}
if (actionPendingCookies.length > 0 || actionDraftCookie || redirectPendingCookies.length > 0 || redirectDraftCookie) {
for (const cookie of actionPendingCookies) {
redirectResponse.headers.append("Set-Cookie", cookie);
}
if (actionDraftCookie) redirectResponse.headers.append("Set-Cookie", actionDraftCookie);
for (const cookie of redirectPendingCookies) {
redirectResponse.headers.append("Set-Cookie", cookie);
}
if (redirectDraftCookie) redirectResponse.headers.append("Set-Cookie", redirectDraftCookie);
}


return redirectResponse;
}
}
} catch (preRenderErr) {
// If pre-rendering fails (e.g., auth guard, missing data, unmatched route),
// clean up navigation context and fall through to hard redirect.
setNavigationContext(null);
console.error("[vinext] Failed to pre-render redirect target:", preRenderErr);
}

// Fallback: external URL or unmatched route — client will hard-navigate.
// Clean up navigation context before returning.
setNavigationContext(null);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this setNavigationContext(null) is redundant when falling through from the catch block, since line 1964 already resets it. Not harmful, but a bit confusing to read — it looks like it should be unreachable from that path.

Comment on lines +2036 to 2039
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: setHeadersContext(null) + setNavigationContext(null) here are redundant when falling through from the catch block at line 1990-1995, which already does the same cleanup. The code is correct (it also handles the case where the try block's inner if conditions fail and execution skips the catch entirely), but a comment would help readability:

Suggested change
// Fallback: external URL or unmatched route — client will hard-navigate.
// Clean up both contexts before returning.
setHeadersContext(null);
setNavigationContext(null);
// Fallback: external URL or unmatched route — client will hard-navigate.
// Clean up both contexts before returning. This also handles the case
// where the try block's conditions (same-origin, matched route) were
// not met and execution fell through without entering the catch.
setHeadersContext(null);
setNavigationContext(null);

Comment on lines +2036 to 2039
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: setHeadersContext(null) and setNavigationContext(null) here are redundant when falling through from the catch block (lines 2028-2029 already do the same cleanup). They're needed when the try block's inner conditions (same-origin, matched route) are not met and execution falls through without entering the catch. A brief comment would help readability:

Suggested change
// Fallback: external URL or unmatched route — client will hard-navigate.
// Clean up both contexts before returning.
setHeadersContext(null);
setNavigationContext(null);
// Fallback: external URL or unmatched route — client will hard-navigate.
// Clean up contexts (also handles the case where the try block's
// conditions were not met and execution fell through without catching).
setHeadersContext(null);
setNavigationContext(null);

const redirectHeaders = {
"Content-Type": "text/x-component; charset=utf-8",
"Vary": "RSC, Accept",
"x-action-redirect": actionRedirect.url,
"x-action-redirect-type": actionRedirect.type,
"x-action-redirect-status": String(actionRedirect.status),
});
for (const cookie of actionPendingCookies) {
redirectHeaders.append("Set-Cookie", cookie);
};
const fallbackResponse = new Response(null, { status: 200, headers: redirectHeaders });
// Append cookies for fallback case
if (actionPendingCookies.length > 0 || actionDraftCookie) {
for (const cookie of actionPendingCookies) {
fallbackResponse.headers.append("Set-Cookie", cookie);
}
if (actionDraftCookie) fallbackResponse.headers.append("Set-Cookie", actionDraftCookie);
}
if (actionDraftCookie) redirectHeaders.append("Set-Cookie", actionDraftCookie);
// Send an empty RSC-like body (client will navigate instead of parsing)
return new Response("", { status: 200, headers: redirectHeaders });
return fallbackResponse;
}

// After the action, re-render the current page so the client
Expand Down
70 changes: 66 additions & 4 deletions packages/vinext/src/server/app-browser-entry.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/// <reference types="vite/client" />

import type { ReactNode } from "react";
import { startTransition } from "react";
import type { Root } from "react-dom/client";
import {
createFromFetch,
Expand Down Expand Up @@ -144,11 +145,72 @@ function registerServerActionCallback(): void {
// Fall through to hard redirect below if URL parsing fails.
}

// Use hard redirect for all action redirects because vinext's server
// currently returns an empty body for redirect responses. RSC navigation
// requires a valid RSC payload. This is a known parity gap with Next.js,
// which pre-renders the redirect target's RSC payload.
// Check if the server pre-rendered the redirect target's RSC payload.
// The server sets x-action-rsc-prerender: 1 when it has pre-rendered the target.
// This is the fix for issue #654.
const hasRscPayload = fetchResponse.headers.get("x-action-rsc-prerender") === "1";
const redirectType = fetchResponse.headers.get("x-action-redirect-type") ?? "replace";

if (hasRscPayload && fetchResponse.body) {
// Server pre-rendered the redirect target — apply it as a soft SPA navigation.
// This matches how Next.js handles action redirects internally.
try {
const result = await createFromFetch(Promise.resolve(fetchResponse), {
temporaryReferences,
});

if (isServerActionResult(result)) {
// Update the React tree with the redirect target's RSC payload
startTransition(() => {
getReactRoot().render(result.root);
});

// Update the browser URL without a reload
if (redirectType === "push") {
window.history.pushState(null, "", actionRedirect);
} else {
window.history.replaceState(null, "", actionRedirect);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: client-side navigation context is not updated. After soft-navigating to the redirect target, usePathname(), useSearchParams(), and useParams() will return stale values from the previous page.

Compare with the navigateRsc function (lines 285-294) which calls setClientParams() after navigation. This code path needs equivalent updates:

// After startTransition + history update:
setNavigationContext({
  pathname: new URL(actionRedirect, window.location.origin).pathname,
  searchParams: new URL(actionRedirect, window.location.origin).searchParams,
  params: {}, // or parse from X-Vinext-Params header
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After pushState/replaceState, the normal navigation path in navigation.ts:608 calls notifyListeners() to trigger useSyncExternalStore re-renders for usePathname(), useSearchParams(), and useParams(). This code path skips that notification.

In practice, render(result.root) replaces the entire tree so most components remount with correct values. But any persistent layout components that use these hooks won't re-render with updated values.

Consider importing and calling notifyListeners (it would need to be exported from navigation.ts), or dispatching a popstate event after the URL update.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: after pushState/replaceState, the normal navigation path in navigation.ts:608 calls notifyListeners() to trigger useSyncExternalStore re-renders for usePathname(), useSearchParams(), etc. This code path skips that.

In practice, render(result.root) replaces the tree so most components remount correctly. But persistent layout components that use these hooks won't re-render with the updated URL.

Since notifyListeners isn't currently exported, one option is to dispatch a synthetic popstate event after the URL update:

window.dispatchEvent(new PopStateEvent("popstate"));

(Though this would also trigger the popstate listener at line 326 which calls navigateRsc — so it needs care. Exporting notifyListeners from navigation.ts is probably cleaner.)


// Update client-side navigation context so usePathname(), useSearchParams(),
// and useParams() return the correct values for the redirect target.
const redirectUrl = new URL(actionRedirect, window.location.origin);
setNavigationContext({
pathname: redirectUrl.pathname,
searchParams: redirectUrl.searchParams,
params: {}, // params will be populated by the RSC stream consumption
});

// Read params from response header (same as normal RSC navigation)
const paramsHeader = fetchResponse.headers.get("X-Vinext-Params");
if (paramsHeader) {
try {
setClientParams(JSON.parse(decodeURIComponent(paramsHeader)));
} catch {
setClientParams({});
}
} else {
setClientParams({});
}

// Handle return value if present
if (result.returnValue) {
if (!result.returnValue.ok) throw result.returnValue.data;
return result.returnValue.data;
}
return undefined;
}
} catch (rscParseErr) {
// RSC parse failed — fall through to hard redirect below.
console.error(
"[vinext] RSC navigation failed, falling back to hard redirect:",
rscParseErr,
);
}
}

// Fallback: empty body (external URL, unmatched route, or parse error).
// Use hard redirect to ensure the navigation still completes.
if (redirectType === "push") {
window.location.assign(actionRedirect);
} else {
Expand Down
Loading
Loading