starred/vinext

Fork 0

mirror of https://github.com/cloudflare/vinext.git synced 2026-05-09 08:25:34 +02:00

[GH-ISSUE #588] Client-side navigation with search params: server component receives stale/default params #128

New issue

Closed

opened 2026-05-06 12:37:26 +02:00 by BreizhHardware · 2 comments

BreizhHardware commented

2026-05-06 12:37:26 +02:00

Owner

Originally created by @NathanDrake2406 on GitHub (Mar 19, 2026).
Original GitHub issue: https://github.com/cloudflare/vinext/issues/588

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) and wrangler dev (broken)

Root cause

In Next.js, accessing searchParams opts a page into dynamic rendering. The page should never be ISR-cached, it should render fresh on every request. vinext has the same intent with markDynamicUsage(), which fires inside buildPageElement() when the page has search params. The HTML response path checks this correctly via consumeDynamicUsage() and skips the ISR cache write. But the RSC response path (used for client-side navigation) returns before that check ever runs, so it writes to the ISR cache unconditionally.

In entries/app-rsc-entry.ts:

markDynamicUsage() fires at line ~1188 during buildPageElement() when search params are present
The HTML path checks consumeDynamicUsage() at line ~3170 and skips ISR if true. This works.
The RSC path returns at line ~3057, before line ~3170 ever executes. The ISR cache write at line ~3041 has no dynamicUsedDuringRender guard.

So what happens is:

First request to /top?genre=Romance&provider=1899 hits the RSC path, renders correctly, then writes the result to ISR cache under key /top (pathname only, which is the correct key format for ISR)
Next request to /top?genre=Romance&provider=8 hits the ISR cache, gets a HIT on /top, and returns stale content from request 1
This keeps happening because the page was never supposed to be cached in the first place

The ISR cache key being pathname-only is fine and matches Next.js. The bug is just that the RSC path doesn't respect the dynamic rendering signal before caching.

Fix

Move the consumeDynamicUsage() check before the RSC return path, or add it as a guard on the RSC cache write block:

if (isRscRequest) {
  // ... build response headers ...

  const __dynamicUsedInRsc = consumeDynamicUsage();
  if (process.env.NODE_ENV === "production" && __isrRscDataPromise && !__dynamicUsedInRsc) {
    // ... existing cache write ...
  }
  return new Response(__rscForResponse, { status: _mwCtx.status || 200, headers: responseHeaders });
}

Same guard should probably be added to the RSC stream tee setup at line ~2950 to avoid the unnecessary tee when we know we won't cache.

Originally created by @NathanDrake2406 on GitHub (Mar 19, 2026). Original GitHub issue: https://github.com/cloudflare/vinext/issues/588 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`: ```tsx // 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: ```tsx // /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): 1. Page loads correctly. Heading says "Highest Rated Romance on Max", film list shows romance films on Max. 2. Click Netflix radio, which calls `router.push("/top?limit=100&genre=Romance&provider=8")`. 3. URL updates correctly to `/top?limit=100&genre=Romance&provider=8`. 4. **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). 5. **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). 6. **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) and `wrangler dev` (broken) --- ## Root cause In Next.js, accessing `searchParams` opts a page into dynamic rendering. The page should never be ISR-cached, it should render fresh on every request. vinext has the same intent with `markDynamicUsage()`, which fires inside `buildPageElement()` when the page has search params. The HTML response path checks this correctly via `consumeDynamicUsage()` and skips the ISR cache write. But the RSC response path (used for client-side navigation) returns **before** that check ever runs, so it writes to the ISR cache unconditionally. In `entries/app-rsc-entry.ts`: - `markDynamicUsage()` fires at line ~1188 during `buildPageElement()` when search params are present - The HTML path checks `consumeDynamicUsage()` at line ~3170 and skips ISR if true. This works. - The RSC path returns at line ~3057, before line ~3170 ever executes. The ISR cache write at line ~3041 has no `dynamicUsedDuringRender` guard. So what happens is: 1. First request to `/top?genre=Romance&provider=1899` hits the RSC path, renders correctly, then writes the result to ISR cache under key `/top` (pathname only, which is the correct key format for ISR) 2. Next request to `/top?genre=Romance&provider=8` hits the ISR cache, gets a HIT on `/top`, and returns stale content from request 1 3. This keeps happening because the page was never supposed to be cached in the first place The ISR cache key being pathname-only is fine and matches Next.js. The bug is just that the RSC path doesn't respect the dynamic rendering signal before caching. ## Fix Move the `consumeDynamicUsage()` check before the RSC return path, or add it as a guard on the RSC cache write block: ```js if (isRscRequest) { // ... build response headers ... const __dynamicUsedInRsc = consumeDynamicUsage(); if (process.env.NODE_ENV === "production" && __isrRscDataPromise && !__dynamicUsedInRsc) { // ... existing cache write ... } return new Response(__rscForResponse, { status: _mwCtx.status || 200, headers: responseHeaders }); } ``` Same guard should probably be added to the RSC stream tee setup at line ~2950 to avoid the unnecessary tee when we know we won't cache.

BreizhHardware closed this issue

2026-05-06 12:37:27 +02:00

BreizhHardware commented

2026-05-06 12:37:27 +02:00

Author

Owner

@james-elicx commented on GitHub (Mar 19, 2026):

ISR cache key ignores search params

This is intentional and aligns with the behaviour in Next.js - you'll probably want to rewrite requests with query params to dynamic routes if they should form part of your ISR cache keys.

RSC write path skips the dynamicUsedDuringRender check

This one sound problematic and like an issue that needs addressing - if a route uses dynamic APIs, that should be accounted for before attempting to cache the response.

Background revalidation always uses empty search params

Similarly to the first one, this is also intentional due to search params not forming part of the ISR cache.

@james-elicx commented on GitHub (Mar 19, 2026): > ISR cache key ignores search params This is intentional and aligns with the behaviour in Next.js - you'll probably want to rewrite requests with query params to dynamic routes if they should form part of your ISR cache keys. > RSC write path skips the dynamicUsedDuringRender check This one sound problematic and like an issue that needs addressing - if a route uses dynamic APIs, that should be accounted for before attempting to cache the response. > Background revalidation always uses empty search params Similarly to the first one, this is also intentional due to search params not forming part of the ISR cache.

BreizhHardware commented

2026-05-06 12:37:27 +02:00

Author

Owner

@NathanDrake2406 commented on GitHub (Mar 20, 2026):

ISR cache key ignores search params

This is intentional and aligns with the behaviour in Next.js - you'll probably want to rewrite requests with query params to dynamic routes if they should form part of your ISR cache keys.

RSC write path skips the dynamicUsedDuringRender check

This one sound problematic and like an issue that needs addressing - if a route uses dynamic APIs, that should be accounted for before attempting to cache the response.

Background revalidation always uses empty search params

Similarly to the first one, this is also intentional due to search params not forming part of the ISR cache.

I've updated the issue :)

@NathanDrake2406 commented on GitHub (Mar 20, 2026): > > ISR cache key ignores search params > > This is intentional and aligns with the behaviour in Next.js - you'll probably want to rewrite requests with query params to dynamic routes if they should form part of your ISR cache keys. > > > RSC write path skips the dynamicUsedDuringRender check > > This one sound problematic and like an issue that needs addressing - if a route uses dynamic APIs, that should be accounted for before attempting to cache the response. > > > Background revalidation always uses empty search params > > Similarly to the first one, this is also intentional due to search params not forming part of the ISR cache. > > I've updated the issue :)

BreizhHardware referenced this issue

2026-05-06 12:39:16 +02:00

[PR #128] [MERGED] fix(shims): copy request headers into mutable Headers in headersContextFromRequest #335