From 24e3e6ad55142bf83c85e7a3730f2da4855139aa Mon Sep 17 00:00:00 2001 From: Sheraff Date: Sat, 18 Jul 2026 09:34:18 +0200 Subject: [PATCH 1/2] fix(router-core): isolate client loading lane ownership --- docs/router/guide/preloading.md | 2 +- packages/router-core/INTERNALS.md | 183 +++-- packages/router-core/src/load-client.ts | 672 ++++++++++-------- packages/router-core/src/router.ts | 10 +- packages/router-core/src/utils.ts | 9 +- .../tests/background-assets-stale.test.ts | 71 +- .../tests/background-trim-abort.test.ts | 48 ++ .../tests/client-loading-contract.test.ts | 205 ++++++ packages/router-core/tests/load.test.ts | 12 +- .../tests/preload-adoption.test.ts | 377 +++++++--- ...reload-background-parent-coherence.test.ts | 15 +- .../tests/preload-beforeload-reuse.test.ts | 94 +-- .../tests/preload-navigation-adoption.test.ts | 13 +- .../preload-public-signal-lifetime.test.ts | 211 ++++++ packages/router-core/tests/utils.test.ts | 7 + 15 files changed, 1323 insertions(+), 606 deletions(-) create mode 100644 packages/router-core/tests/client-loading-contract.test.ts diff --git a/docs/router/guide/preloading.md b/docs/router/guide/preloading.md index 092a869685..d15611de94 100644 --- a/docs/router/guide/preloading.md +++ b/docs/router/guide/preloading.md @@ -130,7 +130,7 @@ export const Route = createFileRoute('/posts/$postId')({ }) ``` -Client-side preloading also runs each new route's `beforeLoad` with `preload: true`. When a completed successful preload is still fresh under the built-in freshness and invalidation policy used for preloaded loader data, the client router can reuse the context returned by that invocation instead of calling `beforeLoad` again with `preload: false`. Reuse follows route match identity, including `loaderDeps`, and requires a reusable parent context. The `shouldReload` option remains loader-only. Pending, failed, invalidated, or stale preloads do not donate their `beforeLoad` context to a client navigation. +Client-side preloading also runs each new route's `beforeLoad` with `preload: true`. If navigation begins while an identical preload lane is still running, the navigation can adopt that complete lane, including its `beforeLoad` context and loaders. Lane identity includes the complete route sequence, params, `loaderDeps`, and search. After a preload finishes, its `beforeLoad` context is discarded and navigation calls `beforeLoad` again with `preload: false`; successful loader data remains independently cacheable according to the route's preload freshness settings. The `shouldReload` option remains loader-only. ## Preloading with External Libraries diff --git a/packages/router-core/INTERNALS.md b/packages/router-core/INTERNALS.md index 3275325b44..14946f603c 100644 --- a/packages/router-core/INTERNALS.md +++ b/packages/router-core/INTERNALS.md @@ -135,9 +135,8 @@ length, not a zero-based boundary index. `stores.cachedMatches`. The cache is neither active presentation nor a registry of in-progress work. -**Flight** is one loader generation and its outcome. Registry membership means a -flight may be joined; a lease means a particular match owns that generation and -its `AbortSignal`. +**Flight** is one loader generation and its outcome. A lease means a particular +match owns that generation and its `AbortSignal`. **Projection** computes non-loader route output such as head metadata, scripts, styles, and, on the server, headers after semantic outcomes have been reduced. @@ -161,7 +160,7 @@ There are intentionally few independent authorities: | `stores.resolvedLocation` | Last location whose load/hydration settled | The requested location | | `router._pending` | One pending reveal/minimum-duration session | A second navigation transaction | | `router._preflight` | Latest synchronous foreground planning controller | A second writer | -| `router._flights` | Joinable loader work by match id | Ownership of every flight | +| `router._preloads` | Active complete preload lanes | Per-match loader sharing | | `route._lazy` | Current lazy-route import owner or loaded marker | A component-module cache | The most important separation is: @@ -191,7 +190,7 @@ Promises also have deliberately narrow ownership: | Promise or acknowledgement | Scope | | -------------------------- | ------------------------------------------------------ | -| Flight outcome | Shareable loader work | +| Flight outcome | One loader invocation | | `tx.done` | One private foreground transaction | | Pending/transition ack | Settlement and render confirmation for one publication | | `commitLocationPromise` | History/navigation completion | @@ -329,8 +328,7 @@ adopt flight leases No phase publishes the terminal semantic lane. Foreground task readiness may offer pending presentation through its `onReady` callback. Background lanes -reuse the phases with mixed ownership; their current projection caveat is -documented below. +reuse the phases with mixed semantic ownership and detached projection output. ## Contextualization and concurrent work @@ -338,8 +336,7 @@ documented below. 1. handles param/search validation failures, 2. combines the completed parent context with route context, -3. reuses an accepted preloaded result only while the contiguous parent prefix - remains reusable, otherwise awaits `beforeLoad`, and +3. awaits `beforeLoad` unless the prefix was already resolved by hydration, and 4. merges the selected result before advancing to the child. This guarantees that child `beforeLoad`, child loader context, and @@ -350,11 +347,10 @@ This guarantees that child `beforeLoad`, child loader context, and Matching first decides whether a semantic match object can be reused by id. Execution then makes two independent decisions: -- `beforeLoad` normally reruns for navigation. It is skipped only for an adopted - hydration prefix or a fresh, completed client-preload donor. The donor's - `beforeLoad` completion timestamp is its own freshness and cache-GC origin; - custom `shouldReload` remains loader-only. Donation is prefix-closed, so a - parent that cannot donate forces every descendant `beforeLoad` to rerun. +- `beforeLoad` reruns for every independently executed navigation lane. An + adopted active preload supplies its complete already-contextualized lane; + completed preload context is never cached. Hydration may skip the accepted + server-rendered prefix. Custom `shouldReload` remains loader-only. - Loader execution considers status, invalidation, configured `shouldReload`, freshness, navigation cause/forced reload, and blocking versus background reload mode. Reusing a match therefore does not imply reusing its loader data. @@ -364,14 +360,13 @@ the match is invalid or `shouldReload` says otherwise, the stale-time branch als requires a forced stale reload, an entering match, or another active id for the same route. -An accepted `beforeLoad` donor may have run with `preload: true`. Navigation -intentionally reuses that context without rerunning the hook merely to call it -with `preload: false`. Pending or failed `beforeLoad` work is never donated. A -pending loader flight may still be shared while navigation runs its own serial -`beforeLoad` chain. +An adopted active preload ran `beforeLoad` with `preload: true`, and navigation +intentionally accepts that context as part of the complete lane. A navigation +that does not adopt the complete lane runs its own serial `beforeLoad` chain and +its own loader work. `preload: false` does not suppress speculative `beforeLoad`: the hook still runs -with `preload: true`, but its context is not donated. The loader is skipped and +with `preload: true`, but its completed context is not cached. The loader is skipped and the match remains invalid, so navigation reruns `beforeLoad` and performs the loader work. Normal component and pending-component readiness may still run for the preload. @@ -617,42 +612,29 @@ rendered output, not `_rendered` itself. ## Loader flights and resource ownership A `LoaderFlight` contains one outcome promise, its own abort controller, and a -lease count. The controller belongs to the shared work rather than any one -transaction. +lease count. Each loader execution creates its own flight; there is no global +per-match join registry. `match._flight === flight` means that match owns one +lease. Copies accepted from active or cached semantic generations acquire a +lease before execution. -Two facts must remain separate: - -- `_flights.get(match.id) === flight` means the flight is joinable. -- `match._flight === flight` means that match owns one lease. - -`closeFlight` removes joinability but does not release the owner's lease. -`releaseFlight` removes one lease; only the last release aborts the loader and -removes the registry entry. `transferMatchResources` performs bulk ownership -transfers based on match object identity. +`releaseFlight` removes one lease and aborts the loader only after the last +owner releases it. `transferMatchResources` performs bulk ownership transfers +based on match object identity. The lease intentionally may outlive promise settlement. Once successful data is -accepted, closing registry membership prevents new joins while the accepted -match keeps the loader generation's public `AbortSignal` alive. That signal is -aborted only when the last owner is replaced, unloaded, expired, or rejected from -cache. Promise state therefore cannot replace the lease count. - -Every semantic publication closes joinability for the published matches. A -later navigation cannot join already-published work through `_flights`, even -though the accepted matches continue to own their leases and signals. - -A navigation may join a preload's same-id flight. It adopts only a successful -outcome. If shared work yields an error, not-found, redirect, or cancellation, -the joining lane closes/releases that flight and runs its own loader. This lets -work be shared without allowing a preload's control outcome or failure context to -govern a navigation. - -When a lane adopts leases, the registry's current joinable flight takes -precedence over a copied, closed `_flight`. An accepted old generation therefore -cannot hide newer joinable work for the same match id. - -Transaction cancellation races the shared promise through `waitFor`. Losing the -race releases that lane's lease; it does not abort work still leased elsewhere. -The flight also checks its own controller after both loader resolution and +accepted, the accepted match keeps the loader generation's public `AbortSignal` +alive. That signal is aborted only when the last owner is replaced, unloaded, +expired, or rejected from cache. Promise state therefore cannot replace the +lease count. + +Active work sharing occurs only by adopting an entire identical preload lane, +not by joining individual loader flights. A failed, not-found, redirected, or +canceled adopted lane is discarded and the navigation executes its own complete +lane. Transaction cancellation races the preload promise through `waitFor`, so a +superseded navigation settles without forcing an otherwise independent preload +to stop. + +Every loader flight checks its own controller after both loader resolution and rejection. A loader that ignores `AbortSignal` therefore still normalizes to canceled and cannot resurrect obsolete cache state. @@ -663,11 +645,13 @@ flags. ## Client preloading and the terminal cache Client preloading uses the same matcher, contextualizer, task builder, reducer, -projector, and flight protocol as navigation, but it never becomes `_tx`. +projector, and loader-flight ownership protocol as navigation, but it never +becomes `_tx`. -`beforeLoad` donation, loader reload, and `preload: false` behavior are defined in -the contextualization section above. Preloading adds speculative ownership, -redirect handling, and cache publication; it does not add another reuse policy. +Loader reload and `preload: false` behavior are defined in the contextualization +section above. Preloading adds speculative ownership, redirect handling, active +complete-lane sharing, and cache publication; it does not add another per-match +reuse policy. A preload snapshots the current writer and plans with an `_isCurrent` guard. It derives the already-resolved active prefix, executes the lane, follows internal @@ -677,11 +661,16 @@ The planning cancellation/error distinction is described above. Routes with as described in the reuse section. The `_isCurrent` guard protects synchronous planning. Once a preload lane exists, -a new foreground transaction does not blindly abort it: useful loader flights may -still be shared, while active-id and cache-entry identity checks prevent obsolete -cache publication. Unexpected post-plan exceptions belong to the preload lane. -It aborts its controller; while still current, it follows eligible redirects, -absorbs not-found, logs other unexpected failures, and releases its resources. A +it is registered in `_preloads`. Another preload or a navigation may adopt it +only when the complete route sequence, match ids, params, and search are equal. +A navigation claim is accepted only if the captured committed base and cache +array remain current. Successful adoption transfers cancellation ownership to +the navigation; failed adoption retries the navigation's complete lane in +blocking mode. HMR may abort all active preload lanes. + +Unexpected post-plan exceptions belong to the preload lane. It aborts its +controller; while still current, it follows eligible redirects, absorbs +not-found, logs other unexpected failures, and releases its resources. A superseded lane must likewise release its resources before returning. Cache publication is an identity compare-and-swap. A preload candidate is @@ -697,12 +686,12 @@ against an absent entry may later publish whenever the entry is also absent at publication. The slot need not have remained continuously empty between those two snapshots. -Foreground commits retain successful exiting matches only when they have either -fresh loader-backed data within the applicable normal/preload GC window or -reusable preloaded `beforeLoad` context within `preloadGcTime`. Active ids are -removed from the cache; failed, not-found, expired, and loaderless entries -without reusable context provenance are discarded. Clearing the cache releases -leases before publishing the retained cache entries. GC is opportunistic: +Completed preloads discard `beforeLoad` context before caching. Foreground +commits retain successful exiting matches only when they have fresh loader-backed +data within the applicable normal/preload GC window. Active ids are removed from +the cache; failed, not-found, expired, and loaderless entries are discarded. +Clearing the cache releases leases before publishing the retained cache entries. +GC is opportunistic: foreground commits apply these age checks; there is no eviction timer, and preload publication does not sweep unrelated entries by age. @@ -716,8 +705,11 @@ candidate reloads in the background. This is used only for loader-backed, non-preload, non-sync work whose effective stale reload mode is not blocking. Foreground reduction and commit proceed with the existing successful match. The -background candidate uses the same loader outcome and projection rules. It may -publish only if both conditions still hold: +background candidate uses the same loader outcome and projection rules. A +successful background reload does not wait for the normal route chunk, which is +already loaded for the committed match; error and not-found reduction still +loads the selected boundary chunk. The candidate may publish only if both +conditions still hold: ```text router._tx === owningTransaction @@ -735,8 +727,8 @@ handoff. Losing background candidates release their resources. Background publication updates matches through the normal publication primitive rather than inventing a second commit protocol. -A background lane intentionally has mixed ownership. Reload task indexes contain -private candidates, while untouched indexes still reference the committed base. +A background lane intentionally begins with mixed ownership. Reload task indexes +contain private candidates, while untouched indexes reference the committed base. An ordinary failure can mutate its private candidate directly. If a not-found bubbles to an untouched ancestor, reduction first clones that boundary and removes `_flight`; the committed generation retains its object and lease until @@ -744,13 +736,13 @@ the guarded publication succeeds. Trimming a background lane likewise leaves shared suffix resources alone. The final transfer or discard of the background batch is the single resource authority. -The two guards above protect final lane publication, not every pre-commit effect. -`projectLane` currently runs user `head`/`scripts` hooks before those guards over -the mixed lane, and it can write projection fields onto an untouched shared base -match even if the final compare-and-swap loses. This is a current sharp edge, not -an authority to copy. Strengthening stale-publication isolation requires -detached projection output or resource-aware, flight-free clones; an ordinary -clone must not accidentally duplicate a flight lease. +Projection runs on detached, flight-free match copies so asynchronous +`head`/`scripts` hooks cannot mutate the committed base before the two guards are +rechecked. On successful publication, loader-flight ownership moves from the +mixed semantic lane into the projected copies. On rejection, candidates are +released and the committed base retains its leases. Projection awaits are raced +against the lane controller, and projection-only copies preserve untouched +matches' `fetchCount`. Background publication replaces an already-resolved presentation directly. It does not enter the pending/renderer-acknowledgement protocol, change @@ -967,14 +959,15 @@ Refresh requires it, installs new route options, rebuilds route indexes, syncs the hot module's route export, clears the lazy owner and path cache, and calls `router._refreshRoute(routeId)`. Core then: -1. aborts and retires joinable flights, +1. aborts all preload lanes, releasing their leases, 2. clears cached match leases, 3. plans synchronously while refusing semantic reuse for the changed route and - its descendants, and -4. commits through the ordinary foreground transaction protocol without pending - presentation. + its descendants, then +4. installs and commits through the ordinary foreground transaction protocol + without pending presentation; installation retires the previous foreground + transaction and releases its leases. -Flight/cache retirement is deliberately global because old route code may own +Preload/cache retirement is deliberately global because old route code may own work outside the visible subtree. Semantic rematerialization remains scoped to the changed route and descendants; ancestors stay reusable. @@ -1024,17 +1017,16 @@ When modifying this system, keep these invariants explicit: 7. Concurrent loader and normal-chunk selection is structural, not promise-settlement ordering. 8. Pending UI is a flight-free presentation clone and never semantic authority. -9. Flight registry membership and flight lease ownership are different facts; - an accepted generation's lease and public signal may outlive promise - settlement. +9. An accepted loader generation's lease and public signal may outlive promise + settlement; a lease is resource ownership, not promise state. 10. Every discarded match releases its resources exactly once. -11. A shared preload result may donate successful work, not control or failure - authority. -12. Preloaded `beforeLoad` provenance is independent from loader-data provenance, - and its reuse is prefix-closed. +11. Only an identical complete active preload lane may be adopted. Control or + failure results cause the navigation to execute its own lane. +12. Completed preload `beforeLoad` context is discarded independently from + cacheable loader data. 13. Final background lane publication requires both current-writer identity and - exact-base identity. Untouched matches are currently shared during - pre-publication projection, as noted above. + exact-base identity. Projection runs on detached matches before those guards + transfer resources and publish. 14. Hydration builds its committed resolved prefix privately, publishes that prefix only after currentness checks, and delegates the remainder to the normal client transaction. @@ -1070,8 +1062,9 @@ Unless a path is shown, core filenames below are relative to for planning, rollback, and waiter ownership, - `preload-adoption.test.ts`, `preload-navigation-adoption.test.ts`, `preload-public-cache-behavior.test.ts`, and - `preload-background-parent-coherence.test.ts` for shared work and cache - publication; `preload-beforeload-reuse.test.ts` for serial context donation, + `preload-background-parent-coherence.test.ts` for complete-lane sharing and + cache publication; `preload-beforeload-reuse.test.ts` for active adoption and + completed-context lifetime, - `background-assets-stale.test.ts` and `background-trim-abort.test.ts` for background projection, trimming, ownership, and writer supersession; `invalidate-pre-rematch-failure.test.ts` for generation replacement across a diff --git a/packages/router-core/src/load-client.ts b/packages/router-core/src/load-client.ts index 578db7f737..c45990f134 100644 --- a/packages/router-core/src/load-client.ts +++ b/packages/router-core/src/load-client.ts @@ -4,6 +4,7 @@ import { isNotFound } from './not-found' import { isRedirect } from './redirect' import { loadRouteChunk } from './route-chunks' import { getLocationChangeInfo } from './router' +import { deepEqual } from './utils' import type { ParsedLocation } from './location' import type { AnyRouteMatch } from './Matches' import type { NotFoundError } from './not-found' @@ -47,7 +48,7 @@ type LoaderOutcome = type IndexedOutcome = [index: number, outcome: LoaderOutcome] -export type LoaderFlight = [ +type LoaderFlight = [ outcome: Promise, controller: AbortController, leases: number, @@ -55,8 +56,6 @@ export type LoaderFlight = [ type WorkMatch = AnyRouteMatch & { _flight?: LoaderFlight - _preloadContext?: number - _preloadBeforeLoad?: AnyRoute['options']['beforeLoad'] } export type LoadTransaction = { @@ -68,6 +67,13 @@ export type LoadTransaction = { redirecting?: true } +type PreloadFlight = [ + matches: Array, + controller: AbortController, + promise: Promise, + claim?: LoadTransaction, +] + export type PendingSession = { owner: LoadTransaction boundary: number @@ -80,6 +86,8 @@ export type PendingSession = { type CoordinatorRouter = AnyRouter & { /** Cancels reentrant synchronous planning without replacing the current writer. */ _preflight?: AbortController + /** Full preload lanes that can be claimed by an identical navigation. */ + _preloads?: Set } type LoaderTask = { @@ -115,11 +123,11 @@ function waitFor( signal: AbortSignal, ): Promise { if (signal.aborted) { - return Promise.reject(signal) + throw signal } return new Promise((resolve, reject) => { const abort = () => reject(signal) - signal.addEventListener('abort', abort, { once: true }) + signal.addEventListener('abort', abort) Promise.resolve(value) .then(resolve, reject) .finally(() => { @@ -173,7 +181,6 @@ async function contextualize( lane: MatchedLane, options: ExecuteLaneOptions, ): Promise { - let reusePreloadContext = !options.preload const signal = options.controller.signal for ( let index = options.resolvedPrefix ?? 0; @@ -183,8 +190,6 @@ async function contextualize( const match = lane.matches[index]! const route = getRoute(router, match) - // Fresh matches already own this lane's controller; cached matches do not. - reusePreloadContext &&= match.abortController !== options.controller match.abortController = options.controller // Contextualization is serial, so the previous match already contains the // complete parent context for this route. @@ -199,37 +204,17 @@ async function contextualize( if (validationError !== undefined) { match.__beforeLoadContext = {} - releaseFlight(router, match) + releaseFlight(match) return [index, normalizeError(route, validationError)] } const beforeLoad = route.options.beforeLoad + match.__beforeLoadContext = {} if (!beforeLoad) { - match.__beforeLoadContext = {} continue } const preload = !!options.preload - // A child can only reuse context from the same parent generation. - const donor = match._preloadContext - const reuse = - reusePreloadContext && - donor != null && - !shouldReloadMatch(router, match, route, options, undefined, donor) - match._preloadContext = undefined - if (process.env.NODE_ENV !== 'production') { - match._preloadBeforeLoad = undefined - } - if (reuse) { - match.context = { - ...context, - ...match.__beforeLoadContext, - } - continue - } - reusePreloadContext = false - match.__beforeLoadContext = {} - const beforeLoadContext: BeforeLoadContextOptions< any, any, @@ -261,7 +246,7 @@ async function contextualize( match.isFetching = false const outcome = normalize(result, false, route.id) if (outcome[0] !== SUCCESS) { - releaseFlight(router, match) + releaseFlight(match) return [index, outcome] } match.__beforeLoadContext = result @@ -269,12 +254,6 @@ async function contextualize( ...context, ...result, } - if (preload && route.options.preload !== false) { - match._preloadContext = Date.now() - if (process.env.NODE_ENV !== 'production') { - match._preloadBeforeLoad = beforeLoad - } - } } catch (cause) { match.isFetching = false if (cause === signal && signal.aborted) { @@ -283,7 +262,7 @@ async function contextualize( if (cause instanceof Promise) { throw cause } - releaseFlight(router, match) + releaseFlight(match) return [index, normalizeError(route, cause)] } } @@ -291,7 +270,7 @@ async function contextualize( return } -function releaseFlight(router: AnyRouter, match: WorkMatch): void { +function releaseFlight(match: WorkMatch): void { const flight = match._flight if (!flight) { return @@ -299,37 +278,24 @@ function releaseFlight(router: AnyRouter, match: WorkMatch): void { match._flight = undefined if (!--flight[2]) { flight[1].abort() - if (router._flights?.get(match.id) === flight) { - router._flights.delete(match.id) - } } } + /** * Not passing in a `next` ownership recipient * is equivalent to discarding the match resources */ export function transferMatchResources( - router: AnyRouter, previous: Array, next?: Array, ): void { for (const match of previous as Array) { if (!next?.includes(match)) { - releaseFlight(router, match) + releaseFlight(match) } } } -function closeFlight(router: AnyRouter, match: WorkMatch): void { - const flight = match._flight - if (!flight) { - return - } - if (router._flights?.get(match.id) === flight) { - router._flights.delete(match.id) - } -} - function getLoaderContext( router: AnyRouter, lane: ContextualizedLane, @@ -373,96 +339,48 @@ async function loadResource( return [SUCCESS, undefined] } - let flight = match._flight - let joined = !!flight && router._flights?.get(match.id) === flight - for (;;) { - if (!joined) { - releaseFlight(router, match) - const controller = new AbortController() - flight = [ - Promise.resolve() - .then(() => - loader( - getLoaderContext( - router, - lane, - match, - route, - controller, - parentMatchPromise, - preload, - ), - ), - ) - .then( - (value) => - controller.signal.aborted - ? [CANCELED] - : normalize(value, false, route.id), - (cause) => - controller.signal.aborted - ? [CANCELED] - : normalizeError(route, cause), + releaseFlight(match) + const controller = new AbortController() + const flight: LoaderFlight = [ + Promise.resolve() + .then(() => + loader( + getLoaderContext( + router, + lane, + match, + route, + controller, + parentMatchPromise, + preload, ), - controller, - 1, - ] - ;(router._flights ??= new Map()).set(match.id, flight) - } - match._flight = flight - match.abortController = flight![1] - match.isFetching = 'loader' - try { - const outcome = await waitFor(flight![0], signal) - if (!joined || outcome[0] === SUCCESS) { - return outcome - } - } catch (cause) { - if (cause === signal) { - releaseFlight(router, match) - return [CANCELED] - } - throw cause + ), + ) + .then( + (value) => + controller.signal.aborted + ? [CANCELED] + : normalize(value, false, route.id), + (cause) => + controller.signal.aborted ? [CANCELED] : normalizeError(route, cause), + ), + controller, + 1, + ] + match._flight = flight + match.abortController = controller + match.isFetching = 'loader' + try { + return await waitFor(flight[0], signal) + } catch (cause) { + if (cause === signal) { + releaseFlight(match) + return [CANCELED] } - closeFlight(router, match) - releaseFlight(router, match) - joined = false + throw cause } } -function shouldReloadMatch( - router: AnyRouter, - match: WorkMatch, - route: AnyRoute, - options: ExecuteLaneOptions, - configured?: any, - donor?: number, -): boolean { - if (match.status !== 'success') { - return true - } - - const preload = !!options.preload - const preloadStaleTime = route.options.preloadStaleTime - const staleAge = - preload || - ((donor != null || match.preload) && preloadStaleTime !== undefined) - ? (preloadStaleTime ?? router.options.defaultPreloadStaleTime ?? 30_000) - : (route.options.staleTime ?? router.options.defaultStaleTime ?? 0) - return !!( - match.invalid || - configured || - (configured === undefined && - Date.now() - (donor ?? match.updatedAt) >= staleAge && - (options.forceStaleReload || - match.cause === 'enter' || - options.base?.some( - (candidate) => - candidate.routeId === match.routeId && candidate.id !== match.id, - ))) - ) -} - function applySuccess(match: WorkMatch, data: unknown): void { match.loaderData = data match.error = undefined @@ -483,15 +401,15 @@ function createLoaderTask( const match = lane.matches[index]! const route = getRoute(router, match) const preload = !!options.preload + const signal = options.controller.signal let reload = false let reloadFailure: LoaderOutcome | undefined try { - if (index < (options.resolvedPrefix ?? 0)) { - reload = !!router._flights?.get(match.id) - } else { - let configured - if (match.status === 'success') { - configured = route.options.shouldReload + if (index >= (options.resolvedPrefix ?? 0)) { + if (match.status !== 'success') { + reload = true + } else { + let configured = route.options.shouldReload if (typeof configured === 'function') { configured = configured( getLoaderContext( @@ -505,25 +423,42 @@ function createLoaderTask( ), ) } + const staleAge = + preload || match.preload + ? (route.options.preloadStaleTime ?? + router.options.defaultPreloadStaleTime ?? + 30_000) + : (route.options.staleTime ?? router.options.defaultStaleTime ?? 0) + reload = !!( + match.invalid || + configured || + (configured === undefined && + Date.now() - match.updatedAt >= staleAge && + (options.forceStaleReload || + match.cause === 'enter' || + options.base?.some( + (candidate) => + candidate.routeId === match.routeId && + candidate.id !== match.id, + ))) + ) } - reload = shouldReloadMatch(router, match, route, options, configured) } } catch (cause) { - releaseFlight(router, match) + releaseFlight(match) reloadFailure = normalizeError(route, cause) } const routeLoader = route.options.loader - const background = !!( + const background = routeLoader && reload && match.status === 'success' && !preload && !options.sync && ((typeof routeLoader === 'function' - ? undefined - : routeLoader?.staleReloadMode) ?? + ? undefined + : routeLoader?.staleReloadMode) ?? router.options.defaultStaleReloadMode) !== 'blocking' - ) const skippedPreload = preload && route.options.preload === false const loaded = reload && !skippedPreload if (skippedPreload) { @@ -542,7 +477,7 @@ function createLoaderTask( route, tasks[index - 1]?.match, preload, - options.controller.signal, + signal, ) const outcome = rawOutcome.then((result) => { if (loaded && !background && result[0] === SUCCESS) { @@ -552,12 +487,14 @@ function createLoaderTask( return result }) - const chunkFailure = Promise.resolve() - .then(() => loadRouteChunk(route)) - .then( - () => undefined, - (cause): IndexedOutcome => [index, normalizeError(route, cause)], - ) + const chunkFailure = background + ? undefined + : Promise.resolve() + .then(() => loadRouteChunk(route)) + .then( + () => undefined, + (cause): IndexedOutcome => [index, normalizeError(route, cause)], + ) const ready = outcome .then((value) => (value[0] === SUCCESS ? chunkFailure : undefined)) .then((value) => { @@ -565,14 +502,10 @@ function createLoaderTask( return value }) - const task = { - outcome, - ready, - match: outcome.then(() => match), - } - tasks.push(task) + const matchPromise = outcome.then(() => match) + tasks.push({ outcome, ready, match: matchPromise }) if (!background) { - return task.match + return matchPromise } const candidate: WorkMatch = { ...match, @@ -585,22 +518,20 @@ function createLoaderTask( route, semanticParent, false, - options.controller.signal, + signal, ).then((result) => { if (result[0] === SUCCESS) { applySuccess(candidate, result[1]) } return result }) - const backgroundMatch = backgroundOutcome.then(() => candidate) - const backgroundTask = { + ;(lane.background ??= []).push({ index, candidate, outcome: backgroundOutcome, - ready: task.ready, - } - ;(lane.background ??= []).push(backgroundTask) - return backgroundMatch + ready, + }) + return backgroundOutcome.then(() => candidate) } function getNotFoundBoundary( @@ -672,7 +603,9 @@ async function reduceLane( if (((task as BackgroundLoaderTask).index ?? index) >= readinessEnd) { break } - const chunkFailure = await task.ready + const chunkFailure = await (process.env.NODE_ENV !== 'production' + ? waitFor(task.ready, options.controller.signal) + : task.ready) if (chunkFailure) { if (chunkFailure[1][0] === REDIRECTED) { if ( @@ -696,10 +629,7 @@ async function reduceLane( if (control) { if (lane.background) { - transferMatchResources( - router, - lane.background.map((task) => task.candidate), - ) + transferMatchResources(lane.background.map((task) => task.candidate)) lane.background = undefined } return control[1] as ControlOutcome @@ -742,43 +672,39 @@ async function reduceLane( kind === ERROR ? 'errorComponent' : 'notFoundComponent', ) } catch {} - trimLane(router, lane, boundary + 1, background) + const length = boundary + 1 + if (!background) { + transferMatchResources(lane.matches.slice(length)) + if (lane.background) { + let write = 0 + for (const task of lane.background) { + if (task.index < length) { + lane.background[write++] = task + } else { + transferMatchResources([task.candidate]) + } + } + lane.background.length = write + } + } + lane.matches.length = length return lane as ReducedLane } return lane as ReducedLane } -function trimLane( - router: AnyRouter, - lane: ContextualizedLane, - length: number, - background: boolean, -): void { - if (!background) { - transferMatchResources(router, lane.matches.slice(length)) - } - lane.matches.length = length - if (!lane.background || background) { - return - } - let write = 0 - for (const task of lane.background) { - if (task.index < length) { - lane.background[write++] = task - } else { - transferMatchResources(router, [task.candidate]) - } - } - lane.background.length = write -} - async function projectLane( router: AnyRouter, lane: ReducedLane, + signal: AbortSignal, start = 0, ): Promise { - for (let index = start; index < lane.matches.length; index++) { + for ( + let index = start; + index < lane.matches.length && !signal.aborted; + index++ + ) { const match = lane.matches[index]! const routeOptions = getRoute(router, match).options if (routeOptions.head || routeOptions.scripts) { @@ -790,17 +716,22 @@ async function projectLane( params: match.params, loaderData: match.loaderData, } - const [head, scripts] = await Promise.all([ - routeOptions.head?.(context), - routeOptions.scripts?.(context), - ]) + const [head, scripts] = await waitFor( + Promise.all([ + routeOptions.head?.(context), + routeOptions.scripts?.(context), + ]), + signal, + ) match.meta = head?.meta match.links = head?.links match.headScripts = head?.scripts match.styles = head?.styles match.scripts = scripts } catch (cause) { - console.error(cause) + if (!signal.aborted) { + console.error(cause) + } } } } @@ -818,9 +749,8 @@ async function executeClientLane( matches: matches as Array, } for (const match of matched.matches) { - const flight = router._flights?.get(match.id) ?? match._flight + const flight = match._flight if (flight) { - match._flight = flight flight[2]++ } } @@ -866,6 +796,7 @@ async function executeClientLane( return projectLane( router, reduced, + options.controller.signal, Math.min(options.resolvedPrefix ?? 0, reduced.matches.length - 1), ) } @@ -874,6 +805,21 @@ function semanticMatches(router: CoordinatorRouter): Array { return router._committedMatches ?? router.stores.matches.get() } +function sameLane( + left: Array, + right: Array, +): boolean { + return ( + left.length === right.length && + left.every( + (match, index) => + match.id === right[index]!.id && + deepEqual(match.params, right[index]!.params, false) && + deepEqual(match.search, right[index]!.search, false), + ) + ) +} + /** * Finds the first route that should show pending UI and its two timing values. * A fallback already on screen remains selected after its route loads, so we @@ -1003,7 +949,6 @@ function publishMatches( match.fetchCount = (oldMatch?.routeId === match.routeId ? oldMatch!.fetchCount : 0) + 1 } - closeFlight(router, match as WorkMatch) } router.stores.setMatches(matches) router._committedMatches = matches @@ -1026,6 +971,8 @@ function commitMatches( if (!nextIds.has(match.id)) { cached.push({ ...match, + __beforeLoadContext: undefined, + context: {}, _flight: undefined, } as WorkMatch) } @@ -1034,23 +981,16 @@ function commitMatches( const now = Date.now() let write = 0 for (const match of cached) { - const work = match as WorkMatch - const route = getRoute(router, work) - const preloadGcTime = - route.options.preloadGcTime ?? - router.options.defaultPreloadGcTime ?? - 300_000 - const donor = work._preloadContext + const route = getRoute(router, match) + const gcTime = match.preload + ? (route.options.preloadGcTime ?? + router.options.defaultPreloadGcTime ?? + 300_000) + : (route.options.gcTime ?? router.options.defaultGcTime ?? 300_000) if ( match.status === 'success' && - ((donor != null && now - donor < preloadGcTime) || - (route.options.loader && - now - match.updatedAt < - (match.preload - ? preloadGcTime - : (route.options.gcTime ?? - router.options.defaultGcTime ?? - 300_000)))) + route.options.loader && + now - match.updatedAt < gcTime ) { cached[write++] = match } @@ -1062,7 +1002,6 @@ function commitMatches( stores.setCached(cached) }) transferMatchResources( - router, [...previousCached, ...previous], [...matches, ...cached], ) @@ -1111,9 +1050,10 @@ async function runBackground( base: Array, tasks: Array, ): Promise { - const backgroundMatches = tasks.map((task) => task.candidate) + const backgroundMatches: Array = [] const next = base.slice() as Array for (const task of tasks) { + backgroundMatches.push(task.candidate) next[task.index] = task.candidate } const lane = { @@ -1126,7 +1066,7 @@ async function runBackground( } const reduced = await reduceLane(router, lane, tasks, options) if (Array.isArray(reduced)) { - transferMatchResources(router, backgroundMatches) + transferMatchResources(backgroundMatches) if ( reduced[0] === REDIRECTED && router._tx === tx && @@ -1143,19 +1083,38 @@ async function runBackground( } return } - const projected = await projectLane(router, reduced) - if (router._tx !== tx || router._committedMatches !== base) { - transferMatchResources(router, backgroundMatches) + const projected = await projectLane( + router, + { + ...reduced, + matches: reduced.matches.map((match) => ({ + ...match, + _flight: undefined, + })), + }, + tx.controller.signal, + ) + if ( + tx.controller.signal.aborted || + router._tx !== tx || + router._committedMatches !== base + ) { + transferMatchResources(backgroundMatches) return } + for (let index = 0; index < projected.matches.length; index++) { + const source = reduced.matches[index]! + const target = projected.matches[index]! + target._flight = source._flight + source._flight = undefined + if (source === base[index]) { + base[index] = target + } + } router.batch(() => { publishMatches(router, projected.matches, base) }) - transferMatchResources( - router, - [...base, ...backgroundMatches], - projected.matches, - ) + transferMatchResources([...base, ...backgroundMatches], projected.matches) } async function runClientTransaction( @@ -1165,26 +1124,40 @@ async function runClientTransaction( onReady?: () => void, sync?: boolean, resolvedPrefix?: number, + preload?: PreloadFlight, ): Promise { const options: ExecuteLaneOptions = { controller: tx.controller, forceStaleReload, - sync, + sync: !!preload || sync, base: semanticMatches(router), resolvedPrefix, redirects: tx.redirects, onReady, } - const result = await executeClientLane( - router, - tx.location, - tx.matches, - options, - ) + let result = preload + ? await waitFor(preload[2], tx.controller.signal) + : undefined + if (router._tx !== tx) { + preload?.[1].abort() + transferMatchResources(tx.matches) + return + } + if (result && !Array.isArray(result) && tx.matches === result.matches) { + tx.controller.abort() + tx.controller = preload![1] + } else { + if (preload) { + for (const match of tx.matches) { + match.invalid = true + } + } + result = await executeClientLane(router, tx.location, tx.matches, options) + } if (Array.isArray(result)) { finishPending(router, tx) - transferMatchResources(router, tx.matches) + transferMatchResources(tx.matches) if (result[0] === REDIRECTED && router._tx === tx) { tx.redirects = (tx.redirects ?? 0) + 1 tx.redirecting = true @@ -1217,9 +1190,9 @@ async function runClientTransaction( } } } - if (router._tx !== tx) { + if (tx.controller.signal.aborted || router._tx !== tx) { finishPending(router, tx) - transferMatchResources(router, result.matches) + transferMatchResources(result.matches) return } const toLocation = tx.location @@ -1228,8 +1201,8 @@ async function runClientTransaction( router.stores.resolvedLocation.get(), ) await router.startViewTransition(async () => { - if (router._tx !== tx) { - transferMatchResources(router, result.matches) + if (tx.controller.signal.aborted || router._tx !== tx) { + transferMatchResources(result.matches) return } const commit = () => { @@ -1359,6 +1332,17 @@ export async function loadClientRoute( } } + const preload = + !refreshRouteId && resolvedPrefix === undefined + ? Array.from(router._preloads ?? []).find( + (flight) => + !flight[0].some( + (match) => + getRoute(router, match as WorkMatch).options.preload === false, + ) && sameLane(flight[0], matches), + ) + : undefined + const tx: LoadTransaction = { location, controller, @@ -1372,13 +1356,14 @@ export async function loadClientRoute( refreshRouteId ? undefined : () => offerPending(router, tx), opts?.sync, resolvedPrefix, + preload, ), ) .catch(() => { if (router._tx === tx) { finishPending(router, tx) - controller.abort() - transferMatchResources(router, tx.matches) + tx.controller.abort() + transferMatchResources(tx.matches) router.batch(() => { router.stores.status.set('idle') router.stores.setMatches(router._committedMatches ?? []) @@ -1393,9 +1378,15 @@ export async function loadClientRoute( redirects: previousOwner?.redirecting ? previousOwner.redirects : undefined, } router._tx = tx + if (preload) { + for (const match of tx.matches as Array) { + match._flight = undefined + } + preload[3] = tx + } previousOwner?.controller.abort() if (previousOwner) { - transferMatchResources(router, previousOwner.matches) + transferMatchResources(previousOwner.matches) } router.batch(() => { @@ -1417,17 +1408,103 @@ export function refreshClientRoute( router: CoordinatorRouter, routeId: string, ): Promise { - if (router._flights) { - for (const flight of router._flights.values()) { - flight[1].abort() + if (router._preloads) { + for (const preload of router._preloads) { + preload[1].abort() } - router._flights.clear() + router._preloads.clear() } router.clearCache() return loadClientRoute(router, { sync: true, _refreshRouteId: routeId }) } +async function runPreloadFlight( + router: CoordinatorRouter, + flight: PreloadFlight, + location: ParsedLocation, + base: Array, + plannedCache: Array, +): Promise { + try { + let resolvedPrefix = 0 + while ( + base[resolvedPrefix]?.status === 'success' && + base[resolvedPrefix]?.id === flight[0][resolvedPrefix]?.id + ) { + resolvedPrefix++ + } + const result = await executeClientLane(router, location, flight[0], { + controller: flight[1], + preload: true, + base, + resolvedPrefix, + }) + if (flight[1].signal.aborted || Array.isArray(result)) { + transferMatchResources(flight[0]) + return flight[1].signal.aborted ? [CANCELED] : result + } + + const matches = result.matches + if (matches.some((match) => match.status !== 'success')) { + transferMatchResources(matches) + return result + } + + const claim = flight[3] + const previous = router.stores.cachedMatches.get() + if ( + claim && + router._tx === claim && + previous === plannedCache && + semanticMatches(router) === base && + !matches.some((match) => match.invalid || match.globalNotFound) + ) { + claim.matches = matches + return result + } + + const active = new Set(semanticMatches(router).map((match) => match.id)) + const candidates: Array = [] + for (const match of matches) { + if ( + match.invalid || + !getRoute(router, match).options.loader || + active.has(match.id) || + previous.find((candidate) => candidate.id === match.id) !== + plannedCache.find((candidate) => candidate.id === match.id) + ) { + releaseFlight(match) + continue + } + candidates.push({ + ...match, + __beforeLoadContext: undefined, + context: {}, + }) + match._flight = undefined + } + const ids = new Set(candidates.map((match) => match.id)) + const cached = previous + .filter((match) => !ids.has(match.id)) + .concat(candidates) + router.stores.setCached(cached) + transferMatchResources(previous, [...cached, ...semanticMatches(router)]) + return result + } catch (cause) { + transferMatchResources(flight[0]) + if (flight[1].signal.aborted) { + return [CANCELED] + } + throw cause + } finally { + router._preloads?.delete(flight) + if (flight[3]?.matches !== flight[0]) { + flight[1].abort() + } + } +} + export async function preloadClientRoute( router: CoordinatorRouter, opts: any, @@ -1449,21 +1526,30 @@ export async function preloadClientRoute( _controller: controller, _isCurrent: () => router._tx === owner, }) - let resolvedPrefix = 0 - while ( - base[resolvedPrefix]?.status === 'success' && - base[resolvedPrefix]?.id === matches[resolvedPrefix]?.id - ) { - resolvedPrefix++ + if (controller.signal.aborted || router._tx !== owner) { + controller.abort() + return } - const result = await executeClientLane(router, location, matches, { + const existing = Array.from(router._preloads ?? []).find((flight) => + sameLane(flight[0], matches!), + ) + const flight: PreloadFlight = existing ?? [ + matches, controller, - preload: true, - base, - resolvedPrefix, - }) + Promise.resolve().then(() => + runPreloadFlight(router, flight, location, base, plannedCache), + ), + ] + if (existing) { + for (const match of matches as Array) { + match._flight = undefined + } + controller.abort() + } else { + ;(router._preloads ??= new Set()).add(flight) + } + const result = await flight[2] if (Array.isArray(result)) { - transferMatchResources(router, matches) if (result[0] === REDIRECTED && !result[1].options.reloadDocument) { return preloadClientRoute( router, @@ -1476,58 +1562,18 @@ export async function preloadClientRoute( } return } - - matches = result.matches - if (matches.some((match) => match.status !== 'success')) { - transferMatchResources(router, matches) - return - } - - const active = new Set(semanticMatches(router).map((match) => match.id)) - const previous = router.stores.cachedMatches.get() - const candidates: Array = [] - let obsoleteContext = false - for (const match of matches as Array) { - if (process.env.NODE_ENV !== 'production') { - obsoleteContext ||= - match._preloadContext != null && - match._preloadBeforeLoad !== - getRoute(router, match).options.beforeLoad - if (obsoleteContext) { - releaseFlight(router, match) - continue - } - } - if ( - active.has(match.id) || - previous.find((candidate) => candidate.id === match.id) !== - plannedCache.find((candidate) => candidate.id === match.id) - ) { - releaseFlight(router, match) - continue - } - closeFlight(router, match) - candidates.push(match) - } - const ids = new Set(candidates.map((match) => match.id)) - const cached = previous - .filter((match) => !ids.has(match.id)) - .concat(candidates) - router.stores.setCached(cached) - transferMatchResources(router, previous, [ - ...cached, - ...semanticMatches(router), - ]) - return matches + return result.matches } catch (cause) { if (!matches) { - if (controller.signal.aborted) { + const stale = controller.signal.aborted || router._tx !== owner + controller.abort() + if (stale) { return } throw cause } controller.abort() - transferMatchResources(router, matches) + transferMatchResources(matches) if (router._tx !== owner) { return } diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 8101228406..842e611b96 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -56,11 +56,7 @@ import type { } from './new-process-route-tree' import type { SearchParser, SearchSerializer } from './searchParams' import type { AnyRedirect, ResolvedRedirect } from './redirect' -import type { - LoadTransaction, - LoaderFlight, - PendingSession, -} from './load-client' +import type { LoadTransaction, PendingSession } from './load-client' import type { ServerLoadResult } from './load-server' import type { HistoryAction, @@ -1019,8 +1015,6 @@ export class RouterCore< subscribers = new Set>() /** Current client load transaction and owner of navigation writes. */ declare _tx?: LoadTransaction - /** Shared in-flight route loaders keyed by match ID. */ - declare _flights?: Map /** Last terminal matches, excluding temporary pending presentation. */ declare _committedMatches?: Array /** Pending-boundary reveal and minimum-visible timing state. */ @@ -2622,7 +2616,7 @@ export class RouterCore< if (filter && !filter(match as MakeRouteMatchUnion)) { retained.push(match) } else { - transferMatchResources(this, [match]) + transferMatchResources([match]) } } this.stores.setCached(retained) diff --git a/packages/router-core/src/utils.ts b/packages/router-core/src/utils.ts index 91011bfa7f..631bbea7a5 100644 --- a/packages/router-core/src/utils.ts +++ b/packages/router-core/src/utils.ts @@ -361,12 +361,13 @@ export function isPlainArray(value: unknown): value is Array { /** * Perform a deep equality check with options for partial comparison and - * ignoring `undefined` values. Optimized for router state comparisons. + * ignoring `undefined` values. Passing `false` is shorthand for exact object + * keys, including keys whose value is `undefined`. */ export function deepEqual( a: any, b: any, - opts?: { partial?: boolean; ignoreUndefined?: boolean }, + opts?: { partial?: boolean; ignoreUndefined?: boolean } | false, ): boolean { if (a === b) { return true @@ -385,9 +386,9 @@ export function deepEqual( } if (isPlainObject(a) && isPlainObject(b)) { - const ignoreUndefined = opts?.ignoreUndefined ?? true + const ignoreUndefined = opts !== false && opts?.ignoreUndefined !== false - if (opts?.partial) { + if ((opts as any)?.partial) { for (const k in b) { if (!ignoreUndefined || b[k] !== undefined) { if (!deepEqual(a[k], b[k], opts)) return false diff --git a/packages/router-core/tests/background-assets-stale.test.ts b/packages/router-core/tests/background-assets-stale.test.ts index 1c39969185..f5ca5faa6b 100644 --- a/packages/router-core/tests/background-assets-stale.test.ts +++ b/packages/router-core/tests/background-assets-stale.test.ts @@ -1,6 +1,6 @@ import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest' import { createMemoryHistory } from '@tanstack/history' -import { BaseRootRoute, BaseRoute } from '../src' +import { BaseRootRoute, BaseRoute, createControlledPromise } from '../src' import { createTestRouter } from './routerTestUtils' describe('background decorative asset failure', () => { @@ -67,6 +67,7 @@ describe('background decorative asset failure', () => { await vi.advanceTimersByTimeAsync(1) await router.load() await vi.waitFor(() => expect(loaderCalls).toBe(2)) + const rootFetchCount = router.state.matches[0]!.fetchCount resolveStaleReload({ title: 'fresh' }) await vi.waitFor(() => @@ -75,5 +76,73 @@ describe('background decorative asset failure', () => { expect(getMatch()?.meta).toEqual([{ title: 'old' }]) expect(log).toHaveBeenCalledWith(projectionError) + expect(router.state.matches[0]!.fetchCount).toBe(rootFetchCount) + }) + + test('a superseded background projection does not mutate committed matches', async () => { + const backgroundLoaderGate = createControlledPromise() + const backgroundHeadStarted = createControlledPromise() + const backgroundHeadGate = createControlledPromise<{ + meta: Array<{ name: string; content: string }> + }>() + const navigationGate = createControlledPromise() + let fooLoaderCalls = 0 + let rootHeadCalls = 0 + + const rootRoute = new BaseRootRoute({ + head: async () => { + rootHeadCalls++ + if (rootHeadCalls === 3) { + backgroundHeadStarted.resolve() + return backgroundHeadGate + } + return { + meta: [{ name: 'generation', content: `head-${rootHeadCalls}` }], + } + }, + }) + const fooRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/foo', + staleTime: 0, + loader: () => { + fooLoaderCalls++ + return fooLoaderCalls === 1 ? 'initial' : backgroundLoaderGate + }, + }) + const otherRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/other', + loader: () => navigationGate, + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([fooRoute, otherRoute]), + history: createMemoryHistory({ initialEntries: ['/foo'] }), + }) + + await router.load() + await vi.advanceTimersByTimeAsync(1) + await router.load() + await vi.waitFor(() => expect(fooLoaderCalls).toBe(2)) + const committedRoot = router.state.matches[0]! + expect(committedRoot.meta).toEqual([ + { name: 'generation', content: 'head-2' }, + ]) + backgroundLoaderGate.resolve('background') + await backgroundHeadStarted + + const navigation = router.navigate({ to: '/other' }) + await Promise.resolve() + backgroundHeadGate.resolve({ + meta: [{ name: 'generation', content: 'stale-background' }], + }) + await Promise.resolve() + + expect(committedRoot.meta).toEqual([ + { name: 'generation', content: 'head-2' }, + ]) + + navigationGate.resolve('other') + await navigation }) }) diff --git a/packages/router-core/tests/background-trim-abort.test.ts b/packages/router-core/tests/background-trim-abort.test.ts index b680460120..c0b53c8604 100644 --- a/packages/router-core/tests/background-trim-abort.test.ts +++ b/packages/router-core/tests/background-trim-abort.test.ts @@ -173,3 +173,51 @@ test('foreground supersession aborts every loader in a background batch', async expect(backgroundSignals.every((signal) => signal.aborted)).toBe(true) expect(router.state.matches.at(-1)?.routeId).toBe(otherRoute.id) }) + +test('background reload does not wait for an already-loaded route chunk', async () => { + let chunkLoads = 0 + let loaderCalls = 0 + let backgroundSignal: AbortSignal | undefined + const RouteComponent = Object.assign(() => null, { + preload: () => { + chunkLoads++ + return Promise.resolve() + }, + }) + + const rootRoute = new BaseRootRoute({}) + const targetRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/target', + component: RouteComponent, + loader: ({ abortController }) => { + loaderCalls++ + if (loaderCalls > 1) { + backgroundSignal = abortController.signal + } + return `generation ${loaderCalls}` + }, + }) + const otherRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/other', + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([targetRoute, otherRoute]), + history: createMemoryHistory({ initialEntries: ['/target'] }), + }) + + await router.load() + await router.invalidate() + await vi.waitFor(() => expect(backgroundSignal).toBeDefined()) + await vi.waitFor(() => + expect(router.state.matches.at(-1)?.loaderData).toBe('generation 2'), + ) + + expect(chunkLoads).toBe(1) + expect(backgroundSignal?.aborted).toBe(false) + + await router.navigate({ to: '/other' }) + + expect(backgroundSignal?.aborted).toBe(true) +}) diff --git a/packages/router-core/tests/client-loading-contract.test.ts b/packages/router-core/tests/client-loading-contract.test.ts new file mode 100644 index 0000000000..385a83f5a0 --- /dev/null +++ b/packages/router-core/tests/client-loading-contract.test.ts @@ -0,0 +1,205 @@ +import { describe, expect, test, vi } from 'vitest' +import { createMemoryHistory } from '@tanstack/history' +import { BaseRootRoute, BaseRoute, createControlledPromise } from '../src' +import { createTestRouter } from './routerTestUtils' + +describe('client loading contracts', () => { + test('completed preloads cache loader data but not beforeLoad context', async () => { + const beforeLoad = vi.fn(({ preload }: { preload: boolean }) => ({ + source: preload ? 'preload' : 'navigation', + })) + const loader = vi.fn( + ({ context }: { context: { source: string } }) => context.source, + ) + const rootRoute = new BaseRootRoute({}) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const targetRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/target', + staleTime: Infinity, + preloadStaleTime: Infinity, + beforeLoad, + loader, + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([indexRoute, targetRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + await router.load() + await router.preloadRoute({ to: '/target' }) + + const cached = router.stores.cachedMatches + .get() + .find((match) => match.routeId === targetRoute.id) + expect(cached?.loaderData).toBe('preload') + expect(cached?.__beforeLoadContext).toBeUndefined() + expect(cached?.context).not.toMatchObject({ source: 'preload' }) + + await router.navigate({ to: '/target' }) + + expect(beforeLoad.mock.calls.map(([context]) => context.preload)).toEqual([ + true, + false, + ]) + expect(loader).toHaveBeenCalledTimes(1) + expect(router.state.matches.at(-1)?.context).toEqual({ + source: 'navigation', + }) + expect(router.state.matches.at(-1)?.loaderData).toBe('preload') + }) + + test('navigation can adopt an identical preload while beforeLoad is running', async () => { + const beforeLoadGate = createControlledPromise() + const beforeLoad = vi.fn(async ({ preload }: { preload: boolean }) => { + await beforeLoadGate + return { source: preload ? 'preload' : 'navigation' } + }) + const loader = vi.fn(() => 'loader data') + const rootRoute = new BaseRootRoute({}) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const targetRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/target', + beforeLoad, + loader, + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([indexRoute, targetRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + await router.load() + const preload = router.preloadRoute({ to: '/target' }) + await vi.waitFor(() => expect(beforeLoad).toHaveBeenCalledTimes(1)) + + const navigation = router.navigate({ to: '/target' }) + await Promise.resolve() + expect(beforeLoad).toHaveBeenCalledTimes(1) + + beforeLoadGate.resolve() + await Promise.all([preload, navigation]) + + expect(beforeLoad).toHaveBeenCalledTimes(1) + expect(loader).toHaveBeenCalledTimes(1) + expect(router.state.matches.at(-1)?.context).toEqual({ source: 'preload' }) + expect(router.state.matches.at(-1)?.loaderData).toBe('loader data') + }) + + test('navigation does not adopt beforeLoad from a different preload lane', async () => { + const beforeLoadGate = createControlledPromise() + const beforeLoad = vi.fn(async ({ preload }: { preload: boolean }) => { + await beforeLoadGate + return { preload } + }) + const rootRoute = new BaseRootRoute({}) + const parentRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/parent', + beforeLoad, + }) + const firstRoute = new BaseRoute({ + getParentRoute: () => parentRoute, + path: '/first', + }) + const secondRoute = new BaseRoute({ + getParentRoute: () => parentRoute, + path: '/second', + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([ + parentRoute.addChildren([firstRoute, secondRoute]), + ]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + await router.load() + const preload = router.preloadRoute({ to: '/parent/first' }) + await vi.waitFor(() => expect(beforeLoad).toHaveBeenCalledTimes(1)) + + const navigation = router.navigate({ to: '/parent/second' }) + await vi.waitFor(() => expect(beforeLoad).toHaveBeenCalledTimes(2)) + + beforeLoadGate.resolve() + await Promise.all([preload, navigation]) + + expect(beforeLoad.mock.calls.map(([context]) => context.preload)).toEqual([ + true, + false, + ]) + expect(router.state.matches.at(-1)?.routeId).toBe(secondRoute.id) + }) + + test('a beforeLoad-only completed preload does not create a cache entry', async () => { + const beforeLoad = vi.fn(() => ({ guarded: true })) + const rootRoute = new BaseRootRoute({}) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const targetRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/target', + beforeLoad, + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([indexRoute, targetRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + await router.load() + await router.preloadRoute({ to: '/target' }) + + expect( + router.stores.cachedMatches + .get() + .some((match) => match.routeId === targetRoute.id), + ).toBe(false) + }) + + test('preload false performs a blocking navigation load', async () => { + const loaderGate = createControlledPromise() + const loader = vi.fn(() => loaderGate) + const rootRoute = new BaseRootRoute({}) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const targetRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/target', + preload: false, + loader, + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([indexRoute, targetRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + await router.load() + await router.preloadRoute({ to: '/target' }) + expect(loader).not.toHaveBeenCalled() + + let settled = false + const navigation = router.navigate({ to: '/target' }).then(() => { + settled = true + }) + await vi.waitFor(() => expect(loader).toHaveBeenCalledTimes(1)) + expect(settled).toBe(false) + + loaderGate.resolve('target data') + await navigation + + expect(router.state.matches.at(-1)).toMatchObject({ + routeId: targetRoute.id, + status: 'success', + loaderData: 'target data', + }) + }) +}) diff --git a/packages/router-core/tests/load.test.ts b/packages/router-core/tests/load.test.ts index 2c805cce65..1130441e85 100644 --- a/packages/router-core/tests/load.test.ts +++ b/packages/router-core/tests/load.test.ts @@ -215,7 +215,7 @@ describe('beforeLoad skip or exec', () => { await router.navigate({ to: '/foo' }) expect(router.state.location.pathname).toBe('/foo') - expect(beforeLoad).toHaveBeenCalledTimes(2) + expect(beforeLoad).toHaveBeenCalledTimes(1) }) test('exec if rejected preload (notFound)', async () => { @@ -409,7 +409,7 @@ describe('loader skip or exec', () => { expect(loader).toHaveBeenCalledTimes(1) }) - test('exec if resolved preload (success)', async () => { + test('reuses a resolved preload within the default preload stale time', async () => { const loader = vi.fn() const router = setup({ loader }) await router.preloadRoute({ to: '/foo' }) @@ -417,7 +417,7 @@ describe('loader skip or exec', () => { await router.navigate({ to: '/foo' }) expect(router.state.location.pathname).toBe('/foo') - expect(loader).toHaveBeenCalledTimes(2) + expect(loader).toHaveBeenCalledTimes(1) }) test('skip if resolved preload (success) within staleTime duration', async () => { @@ -832,7 +832,7 @@ describe('stale loader reload triggers', () => { expect(loader).toHaveBeenCalledTimes(2) }) - test('reloads a stale preloaded loader when switching to a different match id of the same route', async () => { + test('reuses fresh preloaded loaders when switching match ids', async () => { const rootRoute = new BaseRootRoute({}) const rootLoader = vi.fn(() => ({ ok: true })) const childLoader = vi.fn(() => ({ ok: true })) @@ -890,8 +890,8 @@ describe('stale loader reload triggers', () => { search: { page: '2' }, }) - expect(rootLoader).toHaveBeenCalledTimes(3) - expect(childLoader).toHaveBeenCalledTimes(3) + expect(rootLoader).toHaveBeenCalledTimes(2) + expect(childLoader).toHaveBeenCalledTimes(2) }) test('skips stale ancestor loader when only a child path param changes', async () => { diff --git a/packages/router-core/tests/preload-adoption.test.ts b/packages/router-core/tests/preload-adoption.test.ts index 557753e668..16ff038fb5 100644 --- a/packages/router-core/tests/preload-adoption.test.ts +++ b/packages/router-core/tests/preload-adoption.test.ts @@ -1,6 +1,11 @@ import { afterEach, describe, expect, test, vi } from 'vitest' import { createMemoryHistory } from '@tanstack/history' -import { BaseRootRoute, BaseRoute, createControlledPromise } from '../src' +import { + BaseRootRoute, + BaseRoute, + createControlledPromise, + notFound, +} from '../src' import { createTestRouter } from './routerTestUtils' afterEach(() => { @@ -11,10 +16,9 @@ afterEach(() => { * Preload adoption edge cases. The happy path (navigation adopts an * in-flight preload's successful loader run) and the control-flow * non-leakage path are pinned in load.test.ts; this file pins the - * adoption boundary: a donor must have its loader genuinely in flight. A - * preload still in its serial phase can itself be waiting on the navigation - * through the borrow protocol, while a stale successful donor can still have - * fresh loader work pending behind its cached snapshot. + * complete-lane adoption boundary. Identical lanes can be shared during either + * serial contextualization or loader execution; different lanes never join + * individual route loaders. */ describe('preload adoption', () => { @@ -26,7 +30,6 @@ describe('preload adoption', () => { notifications: Array }>() const revalidationStarted = createControlledPromise() - const navigationReachedReloadDecision = createControlledPromise() let loaderCalls = 0 const rootRoute = new BaseRootRoute({}) @@ -40,11 +43,7 @@ describe('preload adoption', () => { staleTime: 0, preloadStaleTime: 0, gcTime: 60_000, - shouldReload: ({ preload }) => { - if (!preload) { - navigationReachedReloadDecision.resolve() - } - }, + shouldReload: () => undefined, loader: { staleReloadMode: 'blocking', handler: () => { @@ -82,7 +81,7 @@ describe('preload adoption', () => { const navigation = router.navigate({ to: '/notifications' }).then(() => { navigationSettled = true }) - await navigationReachedReloadDecision + await Promise.resolve() // The navigation may share the revalidation, but it must not treat the // stale cached snapshot as the completed result while fresh work runs. @@ -102,7 +101,7 @@ describe('preload adoption', () => { ).toEqual({ notifications: ['fresh'] }) }) - test('navigating during the preload serial phase does not deadlock (adoption declined)', async () => { + test('navigation adopts an identical preload during its serial phase', async () => { const beforeLoadGate = createControlledPromise() const preloadSerialStarted = createControlledPromise() let beforeLoadCalls = 0 @@ -140,23 +139,19 @@ describe('preload adoption', () => { await preloadSerialStarted expect(beforeLoadCalls).toBe(1) - // The navigation must not join a donor whose loader is not in flight; - // it runs its own loader and completes without waiting on the preload. - await router.navigate({ to: '/foo' }) - expect(loader).toHaveBeenCalledTimes(1) - expect( - router.state.matches.find((match) => match.routeId === fooRoute.id) - ?.status, - ).toBe('success') + const navigation = router.navigate({ to: '/foo' }) + await Promise.resolve() + expect(beforeLoadCalls).toBe(1) + expect(loader).not.toHaveBeenCalled() beforeLoadGate.resolve() - await preload + await Promise.all([preload, navigation]) + expect(loader).toHaveBeenCalledTimes(1) }) - test("a sibling preload adopts another preload lane's in-flight loader", async () => { + test('identical concurrent preloads share one full lane', async () => { const loaderGate = createControlledPromise() const loaderStarted = createControlledPromise() - const secondPreloadStarted = createControlledPromise() let preloadBeforeLoadCalls = 0 const loader = vi.fn(() => { loaderStarted.resolve() @@ -173,8 +168,8 @@ describe('preload adoption', () => { path: '/foo', preloadStaleTime: Infinity, beforeLoad: ({ preload }) => { - if (preload && ++preloadBeforeLoadCalls === 2) { - secondPreloadStarted.resolve() + if (preload) { + preloadBeforeLoadCalls++ } }, loader, @@ -191,8 +186,8 @@ describe('preload adoption', () => { await loaderStarted const second = router.preloadRoute({ to: '/foo' } as any) - await secondPreloadStarted - expect(preloadBeforeLoadCalls).toBe(2) + await Promise.resolve() + expect(preloadBeforeLoadCalls).toBe(1) expect(loader).toHaveBeenCalledTimes(1) loaderGate.resolve('once') @@ -211,7 +206,6 @@ describe('preload adoption', () => { test('a fulfilled undefined loader result is adopted as success', async () => { const loaderGate = createControlledPromise() const loaderStarted = createControlledPromise() - const navigationStarted = createControlledPromise() const loader = vi.fn(() => { loaderStarted.resolve() return loaderGate @@ -225,11 +219,7 @@ describe('preload adoption', () => { const fooRoute = new BaseRoute({ getParentRoute: () => rootRoute, path: '/foo', - beforeLoad: ({ preload }) => { - if (!preload) { - navigationStarted.resolve() - } - }, + beforeLoad: () => undefined, loader, }) @@ -243,7 +233,7 @@ describe('preload adoption', () => { const preload = router.preloadRoute({ to: '/foo' }) await loaderStarted const navigation = router.navigate({ to: '/foo' }) - await navigationStarted + await Promise.resolve() expect(loaderGate.status).toBe('pending') expect(loader).toHaveBeenCalledTimes(1) @@ -258,80 +248,294 @@ describe('preload adoption', () => { expect(match?.loaderData).toBeUndefined() }) - test('a non-joinable earlier lane does not hide a later lane with its loader in flight', async () => { - const beforeLoadGate = createControlledPromise() - const firstPreloadSerialStarted = createControlledPromise() - const loaderStarted = createControlledPromise() - const navigationSerialStarted = createControlledPromise() - let beforeLoadCalls = 0 - const loaderGate = createControlledPromise() - const loader = vi.fn(() => { - loaderStarted.resolve() - return loaderGate + test('navigation does not adopt a preload with different loader deps', async () => { + const preloadBeforeLoadStarted = createControlledPromise() + const preloadBeforeLoadGate = createControlledPromise() + let generation = 1 + const loader = vi.fn(({ deps }) => deps.generation) + + const rootRoute = new BaseRootRoute({}) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', }) + const targetRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/target', + loaderDeps: () => ({ generation }), + beforeLoad: async ({ preload }) => { + if (preload) { + preloadBeforeLoadStarted.resolve() + await preloadBeforeLoadGate + } + }, + loader, + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([indexRoute, targetRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + await router.load() + const preload = router.preloadRoute({ to: '/target' }) + await preloadBeforeLoadStarted + + generation = 2 + await router.navigate({ to: '/target' }) + + expect(loader).toHaveBeenCalledTimes(1) + expect(router.state.matches.at(-1)?.loaderData).toBe(2) + + preloadBeforeLoadGate.resolve() + await preload + expect(loader).toHaveBeenCalledTimes(2) + }) + + test('navigation does not adopt a preload with different parsed params', async () => { + const preloadBeforeLoadStarted = createControlledPromise() + const preloadBeforeLoadGate = createControlledPromise() + let generation = 1 + const loader = vi.fn(({ params }) => params.generation) const rootRoute = new BaseRootRoute({}) const indexRoute = new BaseRoute({ getParentRoute: () => rootRoute, path: '/', }) - const fooRoute = new BaseRoute({ + const targetRoute = new BaseRoute({ getParentRoute: () => rootRoute, - path: '/foo', + path: '/target/$id', + params: { + parse: ({ id }) => ({ id, generation }), + stringify: ({ id }) => ({ id }), + }, beforeLoad: async ({ preload }) => { - beforeLoadCalls++ - if (!preload) { - navigationSerialStarted.resolve() - } else if (beforeLoadCalls === 1) { - // Keep the FIRST preload's serial phase in flight so its lane - // holds a non-joinable donor; later calls proceed immediately. - firstPreloadSerialStarted.resolve() - await beforeLoadGate + if (preload) { + preloadBeforeLoadStarted.resolve() + await preloadBeforeLoadGate } }, loader, }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([indexRoute, targetRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + await router.load() + const preload = router.preloadRoute({ + to: '/target/$id', + params: { id: 'one', generation: 1 }, + }) + await preloadBeforeLoadStarted + + generation = 2 + await router.navigate({ + to: '/target/$id', + params: { id: 'one', generation: 2 }, + }) + + expect(loader).toHaveBeenCalledTimes(1) + expect(router.state.matches.at(-1)?.loaderData).toBe(2) + + preloadBeforeLoadGate.resolve() + await preload + expect(loader).toHaveBeenCalledTimes(2) + }) + test('navigation retries a root not-found preload result', async () => { + const preloadLoaderStarted = createControlledPromise() + const preloadLoaderGate = createControlledPromise() + const loader = vi.fn(async ({ preload }) => { + if (preload) { + preloadLoaderStarted.resolve() + await preloadLoaderGate + throw notFound() + } + return 'navigation data' + }) + + const rootRoute = new BaseRootRoute({}) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const targetRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/target', + loader, + }) const router = createTestRouter({ - routeTree: rootRoute.addChildren([indexRoute, fooRoute]), + routeTree: rootRoute.addChildren([indexRoute, targetRoute]), history: createMemoryHistory({ initialEntries: ['/'] }), }) await router.load() + const preload = router.preloadRoute({ to: '/target' }) + await preloadLoaderStarted + const navigation = router.navigate({ to: '/target' }) - // First lane: registered first, stuck in its serial phase. - const first = router.preloadRoute({ to: '/foo' } as any) - await firstPreloadSerialStarted - expect(beforeLoadCalls).toBe(1) + preloadLoaderGate.resolve() + await Promise.all([preload, navigation]) - // Second lane: its loader is in flight and therefore joinable. - const second = router.preloadRoute({ to: '/foo' } as any) - await loaderStarted + expect(loader).toHaveBeenCalledTimes(2) + expect(router.state.matches.at(-1)).toMatchObject({ + routeId: targetRoute.id, + loaderData: 'navigation data', + }) + expect(router.state.matches.some((match) => match.globalNotFound)).toBe( + false, + ) + }) - // The navigation must scan PAST the first (serial-phase) lane and adopt - // the second lane's in-flight loader run instead of re-running it. - const navigation = router.navigate({ to: '/foo' }) - await navigationSerialStarted - expect(beforeLoadCalls).toBe(3) - expect(loader).toHaveBeenCalledTimes(1) + test('a superseded navigation stops waiting for an independent preload', async () => { + const beforeLoadStarted = createControlledPromise() + const beforeLoadGate = createControlledPromise() + const rootRoute = new BaseRootRoute({}) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const slowRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/slow', + beforeLoad: async () => { + beforeLoadStarted.resolve() + await beforeLoadGate + }, + loader: () => 'slow data', + }) + const otherRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/other', + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([indexRoute, slowRoute, otherRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) - loaderGate.resolve('once') - await Promise.all([navigation, second]) + await router.load() + let preloadSettled = false + const preload = router.preloadRoute({ to: '/slow' }).then(() => { + preloadSettled = true + }) + await beforeLoadStarted + + let navigationSettled = false + const navigation = router.navigate({ to: '/slow' }).then(() => { + navigationSettled = true + }) + await Promise.resolve() + + await router.navigate({ to: '/other' }) + await navigation + + expect(navigationSettled).toBe(true) + expect(preloadSettled).toBe(false) + expect(beforeLoadGate.status).toBe('pending') + + beforeLoadGate.resolve() + await preload + }) + + test('clearing cache prevents a captured preload generation from being claimed', async () => { + const stalePreloadStarted = createControlledPromise() + const stalePreloadGate = createControlledPromise() + const freshNavigationGate = createControlledPromise() + let loaderCalls = 0 + const loader = vi.fn(() => { + loaderCalls++ + if (loaderCalls === 1) { + return 'initial' + } + if (loaderCalls === 2) { + stalePreloadStarted.resolve() + return stalePreloadGate + } + return freshNavigationGate + }) + + const rootRoute = new BaseRootRoute({}) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const targetRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/target', + shouldReload: true, + loader, + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([indexRoute, targetRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + await router.load() + await router.preloadRoute({ to: '/target' }) expect(loader).toHaveBeenCalledTimes(1) - expect( - router.state.matches.find((match) => match.routeId === fooRoute.id) - ?.loaderData, - ).toBe('once') + + const preload = router.preloadRoute({ to: '/target' }) + await stalePreloadStarted + + let navigationSettled = false + const navigation = router.navigate({ to: '/target' }).then(() => { + navigationSettled = true + }) + await Promise.resolve() + expect(loader).toHaveBeenCalledTimes(2) + router.clearCache() + + stalePreloadGate.resolve('stale') + await vi.waitFor(() => expect(loader).toHaveBeenCalledTimes(3)) + expect(navigationSettled).toBe(false) + + freshNavigationGate.resolve('fresh') + await Promise.all([preload, navigation]) + + expect(router.state.matches.at(-1)?.loaderData).toBe('fresh') + }) + + test('an adopted beforeLoad signal is aborted when its navigation unloads', async () => { + const beforeLoadStarted = createControlledPromise() + const beforeLoadGate = createControlledPromise() + let signal: AbortSignal | undefined + const rootRoute = new BaseRootRoute({}) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const targetRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/target', + beforeLoad: async ({ abortController }) => { + signal = abortController.signal + beforeLoadStarted.resolve() + await beforeLoadGate + }, + loader: () => 'target data', + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([indexRoute, targetRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + await router.load() + const preload = router.preloadRoute({ to: '/target' }) + await beforeLoadStarted + const navigation = router.navigate({ to: '/target' }) beforeLoadGate.resolve() - await first + await Promise.all([preload, navigation]) + expect(signal?.aborted).toBe(false) + + await router.navigate({ to: '/' }) + expect(signal?.aborted).toBe(true) }) - test('navigation retries a failed joined preload without starving sibling work', async () => { + test('navigation retries the full lane after an adopted preload fails', async () => { const preloadParentStarted = createControlledPromise() const preloadFailureGate = createControlledPromise() - const navigationStarted = createControlledPromise() const navigationParentRetryStarted = createControlledPromise() const childStarted = createControlledPromise() const childGate = createControlledPromise() @@ -345,11 +549,7 @@ describe('preload adoption', () => { const parentRoute = new BaseRoute({ getParentRoute: () => rootRoute, path: '/parent', - beforeLoad: ({ preload }) => { - if (!preload) { - navigationStarted.resolve() - } - }, + beforeLoad: () => undefined, loader: async ({ preload }) => { parentLoads++ if (preload) { @@ -385,21 +585,24 @@ describe('preload adoption', () => { expect(childLoads).toBe(1) const navigation = router.navigate({ to: '/parent/child' }) - await navigationStarted + await Promise.resolve() expect(preloadFailureGate.status).toBe('pending') expect(parentLoads).toBe(1) expect(childLoads).toBe(1) preloadFailureGate.resolve() + await Promise.resolve() + expect(parentLoads).toBe(1) + childGate.resolve('child data') + await navigationParentRetryStarted expect(parentLoads).toBe(2) - expect(childLoads).toBe(1) + expect(childLoads).toBe(2) - childGate.resolve('child data') await Promise.all([navigation, preload]) expect(parentLoads).toBe(2) - expect(childLoads).toBe(1) + expect(childLoads).toBe(2) expect(router.state.location.pathname).toBe('/parent/child') expect( router.state.matches.find((match) => match.routeId === parentRoute.id) diff --git a/packages/router-core/tests/preload-background-parent-coherence.test.ts b/packages/router-core/tests/preload-background-parent-coherence.test.ts index cec904a16f..64d6de6c8d 100644 --- a/packages/router-core/tests/preload-background-parent-coherence.test.ts +++ b/packages/router-core/tests/preload-background-parent-coherence.test.ts @@ -4,12 +4,11 @@ import { BaseRootRoute, BaseRoute, createControlledPromise } from '../src' import { createTestRouter } from './routerTestUtils' /** - * A preload that borrows an active parent while that parent is revalidating in - * the background must derive descendant data from the revalidated generation. - * Otherwise the preload can cache a child snapshot based on stale parent data, - * then combine it with the freshly committed parent on navigation. + * A preload only adopts an identical active lane. A child preload therefore + * uses completed cached parent data instead of borrowing a parent from a + * different background-revalidation lane. */ -test('child preload stays coherent with an overlapping parent background reload', async () => { +test('child preload does not borrow an overlapping parent background reload', async () => { const backgroundResponse = createControlledPromise<{ revision: number }>() const childLoaderStarted = createControlledPromise() let parentLoadCount = 0 @@ -58,8 +57,8 @@ test('child preload stays coherent with an overlapping parent background reload' const childPreload = router.preloadRoute({ to: '/parent/child' }) - // The child loader is waiting on its parent while revision 2 is still - // pending, so the two loader generations genuinely overlap. + // The child loader and background parent generation genuinely overlap, but + // the child can use the completed cached parent generation immediately. await childLoaderStarted expect(backgroundResponse.status).toBe('pending') expect(parentLoader).toHaveBeenCalledTimes(2) @@ -81,5 +80,5 @@ test('child preload stays coherent with an overlapping parent background reload' expect(parentLoader).toHaveBeenCalledTimes(2) expect(childLoader).toHaveBeenCalledTimes(1) expect(parentMatch?.loaderData).toEqual({ revision: 2 }) - expect(childMatch?.loaderData).toEqual({ parentRevision: 2 }) + expect(childMatch?.loaderData).toEqual({ parentRevision: 1 }) }) diff --git a/packages/router-core/tests/preload-beforeload-reuse.test.ts b/packages/router-core/tests/preload-beforeload-reuse.test.ts index 2cbd3988f5..bed34511d5 100644 --- a/packages/router-core/tests/preload-beforeload-reuse.test.ts +++ b/packages/router-core/tests/preload-beforeload-reuse.test.ts @@ -7,8 +7,8 @@ afterEach(() => { vi.useRealTimers() }) -describe('preloaded beforeLoad context reuse', () => { - test('reuses fresh nested context for navigation and child loaders', async () => { +describe('preloaded beforeLoad context lifetime', () => { + test('reruns nested beforeLoad while reusing completed loader data', async () => { const parentBeforeLoad = vi.fn(({ preload }: { preload: boolean }) => ({ parent: preload ? 'preloaded parent' : 'loaded parent', })) @@ -62,24 +62,19 @@ describe('preloaded beforeLoad context reuse', () => { await router.preloadRoute({ to: '/parent/child' }) await router.navigate({ to: '/parent/child' }) - expect(parentBeforeLoad).toHaveBeenCalledTimes(1) - expect(parentBeforeLoad).toHaveBeenCalledWith( - expect.objectContaining({ preload: true }), - ) - expect(childBeforeLoad).toHaveBeenCalledTimes(1) - expect(childBeforeLoad).toHaveBeenCalledWith( - expect.objectContaining({ - context: expect.objectContaining({ parent: 'preloaded parent' }), - preload: true, - }), - ) + expect( + parentBeforeLoad.mock.calls.map(([context]) => context.preload), + ).toEqual([true, false]) + expect( + childBeforeLoad.mock.calls.map(([context]) => context.preload), + ).toEqual([true, false]) expect(childLoader).toHaveBeenCalledTimes(1) const match = router.state.matches.find( (candidate) => candidate.routeId === childRoute.id, ) expect(match?.context).toEqual({ - parent: 'preloaded parent', - child: 'preloaded parent:true', + parent: 'loaded parent', + child: 'loaded parent:false', }) expect(match?.loaderData).toEqual({ context: { @@ -90,7 +85,7 @@ describe('preloaded beforeLoad context reuse', () => { }) }) - test('keeps beforeLoad independent while joining a pending preload loader', async () => { + test('adopts beforeLoad with an identical active preload lane', async () => { const loaderGate = createControlledPromise() const beforeLoad = vi.fn(({ preload }: { preload: boolean }) => ({ guard: preload ? 'preloaded' : 'loaded', @@ -118,10 +113,9 @@ describe('preloaded beforeLoad context reuse', () => { await vi.waitFor(() => expect(loader).toHaveBeenCalledTimes(1)) const navigation = router.navigate({ to: '/guarded' }) - await vi.waitFor(() => expect(beforeLoad).toHaveBeenCalledTimes(2)) + await Promise.resolve() expect(beforeLoad.mock.calls.map(([context]) => context.preload)).toEqual([ true, - false, ]) expect(loader).toHaveBeenCalledTimes(1) @@ -129,60 +123,13 @@ describe('preloaded beforeLoad context reuse', () => { await Promise.all([preload, navigation]) expect(loader).toHaveBeenCalledTimes(1) - expect(router.state.matches.at(-1)?.context).toEqual({ guard: 'loaded' }) + expect(router.state.matches.at(-1)?.context).toEqual({ + guard: 'preloaded', + }) expect(router.state.matches.at(-1)?.loaderData).toBe('shared loader data') }) - test.each([ - { age: 50, expected: [false, true], guard: 'preloaded' }, - { age: 100, expected: [false, true, false], guard: 'loaded' }, - ])( - 'uses the beforeLoad completion time when loader data remains navigation-owned at age $age', - async ({ age, expected, guard }) => { - vi.useFakeTimers() - vi.setSystemTime(1_000) - const beforeLoad = vi.fn(({ preload }: { preload: boolean }) => ({ - guard: preload ? 'preloaded' : 'loaded', - })) - const loader = vi.fn(({ preload }: { preload: boolean }) => preload) - const rootRoute = new BaseRootRoute({}) - const indexRoute = new BaseRoute({ - getParentRoute: () => rootRoute, - path: '/', - }) - const guardedRoute = new BaseRoute({ - getParentRoute: () => rootRoute, - path: '/guarded', - staleTime: Infinity, - preloadStaleTime: 100, - beforeLoad, - shouldReload: false, - loader, - }) - const router = createTestRouter({ - routeTree: rootRoute.addChildren([indexRoute, guardedRoute]), - history: createMemoryHistory({ initialEntries: ['/'] }), - }) - - await router.load() - await router.navigate({ to: '/guarded' }) - await router.navigate({ to: '/' }) - vi.setSystemTime(5_000) - await router.preloadRoute({ to: '/guarded' }) - vi.setSystemTime(5_000 + age) - await router.navigate({ to: '/guarded' }) - - expect(beforeLoad.mock.calls.map(([context]) => context.preload)).toEqual( - expected, - ) - expect(loader.mock.calls.map(([context]) => context.preload)).toEqual([ - false, - ]) - expect(router.state.matches.at(-1)?.context).toEqual({ guard }) - }, - ) - - test('keeps a fresh beforeLoad-only preload across an unrelated navigation', async () => { + test('does not cache a beforeLoad-only preload across navigation', async () => { const beforeLoad = vi.fn(({ preload }: { preload: boolean }) => ({ guard: preload ? 'preloaded' : 'loaded', })) @@ -214,9 +161,10 @@ describe('preloaded beforeLoad context reuse', () => { expect(beforeLoad.mock.calls.map(([context]) => context.preload)).toEqual([ true, + false, ]) expect(router.state.matches.at(-1)?.context).toEqual({ - guard: 'preloaded', + guard: 'loaded', }) }) @@ -371,7 +319,7 @@ describe('preloaded beforeLoad context reuse', () => { expect(oldBeforeLoad).toHaveBeenCalledTimes(1) expect(newBeforeLoad).toHaveBeenCalledTimes(1) - expect(childLoader).toHaveBeenCalledTimes(2) + expect(childLoader).toHaveBeenCalledTimes(1) expect(router.state.matches.at(-1)?.loaderData).toBe('new') }) @@ -557,7 +505,7 @@ describe('preloaded beforeLoad context reuse', () => { await router.preloadRoute({ to: '/guarded' }) await router.navigate({ to: '/guarded' }) - expect(beforeLoad).toHaveBeenCalledTimes(1) + expect(beforeLoad).toHaveBeenCalledTimes(2) expect(loader).toHaveBeenCalledTimes(2) expect(loader.mock.calls.map(([context]) => context.preload)).toEqual([ true, @@ -566,7 +514,7 @@ describe('preloaded beforeLoad context reuse', () => { expect(shouldReload).toHaveBeenCalledTimes(1) expect(shouldReload).toHaveBeenCalledWith( expect.objectContaining({ - context: expect.objectContaining({ guard: 'preloaded' }), + context: expect.objectContaining({ guard: 'loaded' }), preload: false, }), ) diff --git a/packages/router-core/tests/preload-navigation-adoption.test.ts b/packages/router-core/tests/preload-navigation-adoption.test.ts index 1fbf00ea40..b7b8a080fb 100644 --- a/packages/router-core/tests/preload-navigation-adoption.test.ts +++ b/packages/router-core/tests/preload-navigation-adoption.test.ts @@ -11,12 +11,7 @@ describe('navigation adopting an in-flight preload', () => { test('adopted preload loader runs once and its signal is not aborted', async () => { const loaderGate = createControlledPromise() const loaderStarted = createControlledPromise() - const navigationStarted = createControlledPromise() - const beforeLoad = vi.fn(({ preload }: { preload: boolean }) => { - if (!preload) { - navigationStarted.resolve() - } - }) + const beforeLoad = vi.fn() let preloadSignal: AbortSignal | undefined const loader = vi.fn( ({ abortController }: { abortController: AbortController }) => { @@ -55,13 +50,11 @@ describe('navigation adopting an in-flight preload', () => { expect(preloadSignal).toBeDefined() expect(preloadSignal?.aborted).toBe(false) - // Wait until the navigation has run its own public beforeLoad while the - // preload loader remains pending, making the overlap explicit. + // The identical navigation claims the whole active preload lane. const navigation = router.navigate({ to: '/foo' }) - await navigationStarted + await Promise.resolve() expect(beforeLoad.mock.calls.map(([context]) => context.preload)).toEqual([ true, - false, ]) expect(loaderGate.status).toBe('pending') expect(loader).toHaveBeenCalledTimes(1) diff --git a/packages/router-core/tests/preload-public-signal-lifetime.test.ts b/packages/router-core/tests/preload-public-signal-lifetime.test.ts index b1bc87e65c..4e1e63e0e8 100644 --- a/packages/router-core/tests/preload-public-signal-lifetime.test.ts +++ b/packages/router-core/tests/preload-public-signal-lifetime.test.ts @@ -122,3 +122,214 @@ test('a superseded preload releases its borrowed loader signal lease', async () expect(router.state.location.pathname).toBe('/') expect(parentSignal?.aborted).toBe(true) }) + +test('HMR aborts a preload waiting in asset projection', async () => { + const headStarted = createControlledPromise() + const headGate = createControlledPromise() + let signal: AbortSignal | undefined + + const rootRoute = new BaseRootRoute({}) + const homeRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const targetRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/target', + loader: ({ abortController }) => { + signal = abortController.signal + return 'target data' + }, + head: async () => { + headStarted.resolve() + await headGate + return { meta: [{ title: 'obsolete' }] } + }, + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([homeRoute, targetRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + await router.load() + const preload = router.preloadRoute({ to: '/target' }) + await headStarted + expect(signal?.aborted).toBe(false) + + expect(router._refreshRoute).toBeDefined() + await router._refreshRoute!(targetRoute.id) + await preload + + expect(signal?.aborted).toBe(true) + expect(headGate.status).toBe('pending') + expect( + router.stores.cachedMatches + .get() + .some((match) => match.routeId === targetRoute.id), + ).toBe(false) +}) + +test('HMR aborts a preload waiting for its route chunk', async () => { + const chunkStarted = createControlledPromise() + const chunkGate = createControlledPromise() + let signal: AbortSignal | undefined + const RouteComponent = Object.assign(() => null, { + preload: () => { + chunkStarted.resolve() + return chunkGate + }, + }) + + const rootRoute = new BaseRootRoute({}) + const homeRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const targetRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/target', + component: RouteComponent, + loader: ({ abortController }) => { + signal = abortController.signal + return 'target data' + }, + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([homeRoute, targetRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + await router.load() + const preload = router.preloadRoute({ to: '/target' }) + await chunkStarted + expect(signal?.aborted).toBe(false) + + expect(router._refreshRoute).toBeDefined() + await router._refreshRoute!(targetRoute.id) + await preload + + expect(signal?.aborted).toBe(true) + expect(chunkGate.status).toBe('pending') +}) + +test('HMR does not release an inherited flight before preload execution', async () => { + let parentSignal: AbortSignal | undefined + + const rootRoute = new BaseRootRoute({}) + const homeRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const parentRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/parent', + loader: ({ abortController }) => { + parentSignal = abortController.signal + return 'parent data' + }, + }) + const childRoute = new BaseRoute({ + getParentRoute: () => parentRoute, + path: '/child', + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([ + homeRoute, + parentRoute.addChildren([childRoute]), + ]), + history: createMemoryHistory({ initialEntries: ['/parent'] }), + }) + + await router.load() + expect(parentSignal?.aborted).toBe(false) + + const preload = router.preloadRoute({ to: '/parent/child' }) + expect(router._refreshRoute).toBeDefined() + await Promise.all([router._refreshRoute!(childRoute.id), preload]) + + expect(parentSignal?.aborted).toBe(false) + await router.navigate({ to: '/' }) + expect(parentSignal?.aborted).toBe(true) +}) + +test('a superseded claim does not retain the preload controller', async () => { + const loaderStarted = createControlledPromise() + const loaderGate = createControlledPromise() + let beforeLoadSignal: AbortSignal | undefined + + const rootRoute = new BaseRootRoute({}) + const homeRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const targetRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/target', + beforeLoad: ({ abortController }) => { + beforeLoadSignal = abortController.signal + }, + loader: () => { + loaderStarted.resolve() + return loaderGate + }, + }) + const otherRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/other', + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([homeRoute, targetRoute, otherRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + await router.load() + const preload = router.preloadRoute({ to: '/target' }) + await loaderStarted + const navigation = router.navigate({ to: '/target' }) + const superseding = preload.then(() => router.navigate({ to: '/other' })) + + loaderGate.resolve('target data') + await Promise.all([navigation, superseding]) + + expect(router.state.location.pathname).toBe('/other') + expect(beforeLoadSignal?.aborted).toBe(true) +}) + +test('fatal adopted-lane cleanup aborts the transferred controller', async () => { + const loaderStarted = createControlledPromise() + const loaderGate = createControlledPromise() + let beforeLoadSignal: AbortSignal | undefined + + const rootRoute = new BaseRootRoute({}) + const homeRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const targetRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/target', + beforeLoad: ({ abortController }) => { + beforeLoadSignal = abortController.signal + }, + loader: () => { + loaderStarted.resolve() + return loaderGate + }, + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([homeRoute, targetRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + await router.load() + router.startTransition = () => Promise.reject(new Error('render failed')) + + const preload = router.preloadRoute({ to: '/target' }) + await loaderStarted + const navigation = router.navigate({ to: '/target' }) + + loaderGate.resolve('target data') + await Promise.all([preload, navigation]) + + expect(beforeLoadSignal?.aborted).toBe(true) +}) diff --git a/packages/router-core/tests/utils.test.ts b/packages/router-core/tests/utils.test.ts index fff911df37..65b18612cd 100644 --- a/packages/router-core/tests/utils.test.ts +++ b/packages/router-core/tests/utils.test.ts @@ -419,6 +419,13 @@ describe('deepEqual', () => { expect(deepEqual([a], [b], { partial, ignoreUndefined })).toEqual(false) expect(deepEqual([b], [a], { partial, ignoreUndefined })).toEqual(false) }) + + it('supports false as exact object-key shorthand', () => { + const a = { nested: { value: undefined } } + const b = { nested: {} } + expect(deepEqual(a, b, false)).toEqual(false) + expect(deepEqual(b, a, false)).toEqual(false) + }) }) describe('partial = true', () => { const partial = true From 20906b66b578d7c478f54965b526e9db0d0cf0fe Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sat, 18 Jul 2026 07:39:29 +0000 Subject: [PATCH 2/2] ci: apply automated fixes --- packages/router-core/src/load-client.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/router-core/src/load-client.ts b/packages/router-core/src/load-client.ts index c45990f134..eabafcf24a 100644 --- a/packages/router-core/src/load-client.ts +++ b/packages/router-core/src/load-client.ts @@ -456,8 +456,8 @@ function createLoaderTask( !preload && !options.sync && ((typeof routeLoader === 'function' - ? undefined - : routeLoader?.staleReloadMode) ?? + ? undefined + : routeLoader?.staleReloadMode) ?? router.options.defaultStaleReloadMode) !== 'blocking' const skippedPreload = preload && route.options.preload === false const loaded = reload && !skippedPreload