Skip to content

Commit 3d51361

Browse files
yanghuidongclaude
andcommitted
fix: prevent stale head() re-runs from polluting state
Problem: When async loaders complete, we schedule a detached head() re-run to execute head functions with fresh loaderData. However, if a new navigation or invalidation starts before this re-run executes, it would update state with stale data. The previous location-based check only caught navigation to different locations, but missed same-location invalidations (e.g., manual reload, scheduled refetch). Solution: Implement a generation counter pattern to track load operations: 1. Add `router._loadGeneration` counter (excludes preloads) 2. Each non-preload `loadMatches()` call increments the counter 3. Store the generation in `inner.loadGeneration` when load starts 4. Before executing head re-runs or updates, check if generation matches This detects ALL cases where a load has been superseded: - Navigation to different location (new loadMatches call) - Invalidation on same location (new loadMatches call) - Any other scenario triggering a new load The generation counter is a standard pattern in reactive systems (React, RxJS) for detecting stale computations. Benefits: - No circular references (vs storing full context) - Minimal memory (4 bytes) - Simple numeric comparison - Clear semantics (higher = newer) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
1 parent 98f1319 commit 3d51361

File tree

2 files changed

+62
-6
lines changed

2 files changed

+62
-6
lines changed

packages/router-core/src/load-matches.ts

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,32 @@ type InnerLoadContext = {
3535
sync?: boolean
3636
/** mutable state, scoped to a `loadMatches` call */
3737
matchPromises: Array<Promise<AnyRouteMatch>>
38+
/**
39+
* Generation number for this load operation (only set for non-preload loads).
40+
* Used to detect when this load has been superseded by a newer one.
41+
* Compared against router._loadGeneration to abort stale async operations.
42+
*/
43+
loadGeneration?: number
44+
}
45+
46+
/**
47+
* Checks if this load operation has been superseded by a newer one.
48+
* This prevents stale async operations from updating router state.
49+
*
50+
* Returns true if the operation should be aborted in these cases:
51+
* 1. Navigation to a different location
52+
* 2. Route invalidation on the same location (new loader dispatch)
53+
* 3. Any other scenario that triggers a new loadMatches() call with preload=false
54+
*
55+
* For preload operations (inner.loadGeneration undefined), never abort.
56+
*/
57+
const shouldAbortLoad = (inner: InnerLoadContext): boolean => {
58+
// Preloads don't have a generation number and should never be aborted by this check
59+
if (inner.loadGeneration === undefined) {
60+
return false
61+
}
62+
// Check if a newer load operation has started (higher generation number)
63+
return inner.loadGeneration !== inner.router._loadGeneration
3864
}
3965

4066
const triggerOnReady = (inner: InnerLoadContext): void | Promise<void> => {
@@ -584,15 +610,25 @@ const executeHead = (
584610
}
585611

586612
const executeAllHeadFns = async (inner: InnerLoadContext) => {
613+
// Check if this load operation has been superseded before starting
614+
if (shouldAbortLoad(inner)) return
615+
587616
// Serially execute head functions for all matches
588617
// Each execution is wrapped in try-catch to ensure all heads run even if one fails
589618
for (const match of inner.matches) {
619+
// Check before each match in case we get aborted during iteration
620+
if (shouldAbortLoad(inner)) return
621+
590622
const { id: matchId, routeId } = match
591623
const route = inner.router.looseRoutesById[routeId]!
592624
try {
593625
const headResult = executeHead(inner, matchId, route)
594626
if (headResult) {
595627
const head = await headResult
628+
629+
// Check again after async operation completes
630+
if (shouldAbortLoad(inner)) return
631+
596632
inner.updateMatch(matchId, (prev) => ({
597633
...prev,
598634
...head,
@@ -901,6 +937,12 @@ export async function loadMatches(arg: {
901937
matchPromises: [],
902938
})
903939

940+
// For non-preload operations, assign a generation number to detect stale operations later
941+
// This handles both navigation (different location) and invalidation (same location)
942+
if (!inner.preload) {
943+
inner.loadGeneration = ++inner.router._loadGeneration
944+
}
945+
904946
// make sure the pending component is immediately rendered when hydrating a match that is not SSRed
905947
// the pending component was already rendered on the server and we want to keep it shown on the client until minPendingMs is reached
906948
if (
@@ -959,13 +1001,12 @@ export async function loadMatches(arg: {
9591001
if (asyncLoaderPromises.length > 0) {
9601002
// Schedule re-execution after all async loaders complete (non-blocking)
9611003
// Use allSettled to handle both successful and failed loaders
962-
//
963-
// TODO! temporarily disabled to make sure solid-start/basic example can reproduce the bug
964-
const thisNavigationLocation = inner.location
9651004
Promise.allSettled(asyncLoaderPromises).then(() => {
966-
// Only execute if this navigation is still current (not superseded by new navigation)
967-
const latestLocation = inner.router.state.location
968-
if (latestLocation === thisNavigationLocation) {
1005+
// Only execute if this load operation hasn't been superseded
1006+
// This handles both:
1007+
// 1. Navigation to a different location
1008+
// 2. Route invalidation on the same location (new loader dispatch)
1009+
if (!shouldAbortLoad(inner)) {
9691010
executeAllHeadFns(inner)
9701011
}
9711012
})

packages/router-core/src/router.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -894,6 +894,21 @@ export class RouterCore<
894894
viewTransitionPromise?: ControlledPromise<true>
895895
isScrollRestoring = false
896896
isScrollRestorationSetup = false
897+
/**
898+
* Internal: Generation counter for tracking load operations (excludes preloads).
899+
* Incremented each time loadMatches() is called with preload=false.
900+
*
901+
* Purpose: Detects stale async operations (like detached head re-runs) when a new
902+
* load starts. Handles both navigation to different locations AND invalidation on
903+
* the same location.
904+
*
905+
* Example: If async loaders complete and schedule a head re-run, but a new navigation
906+
* or invalidation has started (incrementing this counter), the old head re-run will
907+
* detect staleness and abort before updating state.
908+
*
909+
* Why a counter: Simple, no circular references, standard pattern in reactive systems.
910+
*/
911+
_loadGeneration: number = 0
897912

898913
// Must build in constructor
899914
__store!: Store<RouterState<TRouteTree>>

0 commit comments

Comments
 (0)