-
Notifications
You must be signed in to change notification settings - Fork 251
Description
Migrating my movie site to vinext from OpenNext and while it's a lot faster, it's also buggy in the client-side navigation department. Specifically, router.push() with search param changes causes the server component to re-render with stale or default params instead of the updated ones. This only happens in the production build (via wrangler dev or vinext deploy). vinext dev works correctly.
Repro
I have a /top page (server component) that reads searchParams for filtering (genre, provider, limit, etc.), and a client component that updates filters via useRouter().push() inside useTransition:
// useFilterParams.ts ("use client")
export function useFilterParams(basePath: string) {
const router = useRouter();
const searchParams = useSearchParams();
const [isPending, startTransition] = useTransition();
const pushWithMutator = useCallback(
(mutate: (params: URLSearchParams) => void) => {
const params = new URLSearchParams(searchParams.toString());
mutate(params);
const qs = params.toString();
startTransition(() => {
router.push(qs ? `${basePath}?${qs}` : basePath, { scroll: false });
});
},
[basePath, router, searchParams, startTransition],
);
return { isPending, searchParams, pushWithMutator };
}The server component uses these params to query a database and render a filtered list:
// /top/page.tsx (server component)
export default async function TopPage({ searchParams }: { searchParams: Promise<{ limit?: string; genre?: string; provider?: string }> }) {
const params = await searchParams;
const movies = await getTopMovies(params); // queries DB with filters
return <TopMovieList movies={movies} />;
}What happens
Starting from /top?limit=100&genre=Romance&provider=1899 (HBO Max):
- Page loads correctly. Heading says "Highest Rated Romance on Max", film list shows romance films on Max.
- Click Netflix radio, which calls
router.push("/top?limit=100&genre=Romance&provider=8"). - URL updates correctly to
/top?limit=100&genre=Romance&provider=8. - Bug: Server component re-renders with default/unfiltered data. Heading reverts to "Top 10", film list shows The Godfather, 12 Angry Men, Seven Samurai (not romance, not filtered by provider).
- Bug: The page renders duplicate content. The accessibility tree shows the heading, filters, and film list twice (stale + fresh RSC payloads both in the DOM).
- Bug: Browser back button always returns to the initial page state (top 10 defaults) regardless of navigation history. Suggests history entries aren't being pushed correctly, or the RSC cache/state isn't keyed by URL.
Works in dev, broken in prod
This works perfectly in vinext dev (Node). Filters update correctly, back button preserves history, no duplicate rendering. The bug only shows up in the production build via wrangler dev or vinext deploy. So the issue is likely in the production RSC request handler or the bundled router's RSC fetch path, not the dev server.
Expected behavior
After router.push with new search params, the server component should receive the updated searchParams and re-render with the correct filtered data (matching Next.js behavior).
Environment
- vinext 0.0.31
- Vite 8.0.1
- React 19.2.4
- App Router only
- Tested in
vinext dev(works) andwrangler dev(broken)
Root cause
The ISR cache keys pages by pathname only. Search params are excluded entirely. This means /top?genre=Romance&provider=1899 and /top?genre=Romance&provider=8 share the same cache key (app:<buildId>:/top:rsc).
For any page that exports revalidate and varies content by search params, the ISR cache returns stale content rendered from a previous request's params (or empty params from background revalidation).
This is production-only because the ISR cache read is gated by process.env.NODE_ENV === "production".
Three compounding bugs in entries/app-rsc-entry.ts
1. ISR cache key ignores search params
// line 478-489: only uses pathname, never search params
function __isrCacheKey(pathname, suffix) {
const normalized = pathname === "/" ? "/" : pathname.replace(/\\/$/, "");
const prefix = buildId ? "app:" + buildId : "app";
return prefix + ":" + normalized + ":" + suffix;
}The cache read at line ~2535 uses __isrRscKey(cleanPathname) / __isrHtmlKey(cleanPathname), so different search params hit the same cache entry.
2. RSC write path skips the dynamicUsedDuringRender check
markDynamicUsage() is correctly called inside buildPageElement() when search params are present (line ~1188). The HTML path checks dynamicUsedDuringRender at line ~3208 and skips the cache write. But the RSC path returns at line ~3057 before that check, so it writes to the ISR cache unconditionally. The safety net that should prevent caching dynamic pages doesn't work for RSC requests.
3. Background revalidation always uses empty search params (partially addressed in #542)
// line ~2588-2591
setNavigationContext({ pathname: cleanPathname, searchParams: new URLSearchParams(), params });
const __revalElement = await buildPageElement(route, params, undefined, new URLSearchParams());Background ISR revalidation always re-renders with new URLSearchParams(), so cached content is always the default/unfiltered view. This is exactly the "Top 10" result reported above.
Potential fix
The right approach matches what Next.js does: pages that access searchParams should be fully dynamic and bypass ISR entirely.
- Short-circuit ISR cache read for requests with search params. Before the ISR cache read block (~line 2530), check if
url.searchParamshas entries. If it does, skip the cache read. The page content will vary by query string and pathname-only keys will always be wrong:
const __requestHasSearchParams = url.search && url.search.length > 1;
if (
process.env.NODE_ENV === "production" &&
!isForceDynamic &&
!__requestHasSearchParams && // skip ISR for requests with search params
revalidateSeconds !== null && revalidateSeconds > 0 && revalidateSeconds !== Infinity
) {
// ... existing ISR cache read ...
}- Guard the RSC ISR cache write with
dynamicUsedDuringRender. Move theconsumeDynamicUsage()call before the RSC return path, or check it inside the RSC write block (~line 3041). This prevents caching RSC responses for pages that calledmarkDynamicUsage():
if (isRscRequest) {
// ... build response headers ...
// Only write to ISR cache if the page didn't use dynamic APIs
const __dynamicUsedInRsc = consumeDynamicUsage();
if (process.env.NODE_ENV === "production" && __isrRscDataPromise && !__dynamicUsedInRsc) {
// ... existing cache write ...
}
return new Response(__rscForResponse, { status: _mwCtx.status || 200, headers: responseHeaders });
}- Skip ISR tee setup for requests with search params. The RSC stream tee at line ~2950 should also be guarded. No point capturing rscData if we know we won't cache it:
if (
process.env.NODE_ENV === "production" &&
revalidateSeconds !== null && revalidateSeconds > 0 && revalidateSeconds !== Infinity &&
!isForceDynamic &&
!__requestHasSearchParams // don't tee if we won't cache
) {
// ... tee logic ...
}This is the simplest correct fix. A more ambitious approach would include search params in the cache key (like Next.js does for its Data Cache), but that requires careful thought about cache cardinality and eviction, probably better as a follow-up.