Open
Conversation
⚡ Benchmark Results
Legend🟢 > 1% improvement | 🔴 > 1% regression | ⚪ within noise margin Benchmarks run on GitHub Actions runners (shared infrastructure) — expect ~5% variance between runs. Consistent directional changes across multiple routes are more meaningful than any single number. |
❌ 14 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
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
2 participants
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.
PR #372 ("enhanced router") introduced a regression where every
<Route>in a layout caused the SSR worker to import the target page module and the browser to preload its chunk, even for non-matching siblings. On large layouts this dominatedrequest latency and forced every client chunk into the initial download. This PR removes that cost without giving up the correctness or DX of the new router.
The core idea is to keep live client references out of the Flight payload for routes that don't match. The file router now emits
<Route componentId="…" componentLoader={() => Page} />instead of<Route element={<Page/>} />. The livereference only exists inside the closure body, so React's RSC encoder walks past it for non-matching siblings and never registers the chunk. Only the matching route calls
componentLoader()and instantiates the page, which produces exactlyone client-reference registration per request. For non-matching routes we resolve the source-relative
$idto the built chunk URL viaclientReferenceMap(with a try/catch fallback to the source module so dev still works without the.react-server/build output) and pass the chunk id to the client.On the client,
ClientRouteRegistrationbuilds a smallLazyChunkComponentwrapper around the deferred chunk. It deliberately does not useReact.lazy, becauselazyalways schedules a microtask before re-rendering and causes a one-framefallback flash even when the module is already in the
__webpack_require__cache. Instead the wrapper readsp.valuesynchronously on cache hits and falls through to React 19'suse(p)hook only when the import is genuinely in flight. Italso patches
.value/.statusonto the import promise itself, since the prod polyfill inrender-rsc.jsxdoes this on the server but Vite's dev__webpack_require__does not. There is a load-bearing invariant: in lazy mode the wrapper isonly instantiated when the route is active, because Activity hidden subtrees still render and would otherwise eagerly fire the dynamic import for every sibling. The lazy render path also intentionally has no local Suspense boundary, so an
active route's suspension propagates to the navigation transition and React keeps the previous page visible until the new chunk resolves, instead of flashing a blank fallback.
The second half of this PR is an unrelated scroll-restoration bug surfaced by the new lazy navigation timing.
ScrollRestorationinitialised itslastYsnapshot fromwindow.scrollYat effect setup time, but onpopstatethe browsercarries the previous page's scroll position over because we set
history.scrollRestoration = "manual". If a fast follow-up navigation ran the cleanup before any real scroll event refreshed the snapshot, the cleanup would write the previousroute's scroll value under the current route's key, silently corrupting saved positions. The fix introduces a module-level
scrollObservedflag that the window scroll listener and every container scroll listener flip on first event, and thecleanup only persists if a real scroll has been observed for that route. If nothing scrolled, the existing storage entry is correct and we leave it alone. This affects real users on any quick back/forward navigation, not just tests.
The scroll restoration test file was also rewritten to boot the dev server once via
beforeAll(the previous per-testawait server(...)was rebuilding the dev server before every test and dominating the suite duration — 63s down to 17s),with a
beforeEachthat navigates to the fixture origin first and then clearssessionStorage, since storage is per-origin and clearing onabout:blankis a no-op for the test origin.Finally, the benchmark example was restructured into
(rsc),(ssr), and(hybrid)route groups, andbench.mjsnow exposes a--filterflag for local iteration plus a new set of hybrid benchmarks that exercise thelayout-with-many-client-siblings shape this PR is designed to make fast. Original benchmark URLs are unchanged (route groups are transparent), so the CI baseline comparison still works without modification.