Skip to content

Commit e9d370e

Browse files
committed
docs: remove in-place mutation of route objects
Route objects are never mutated. matchRoutes uses the sync result as a local variable for matching; Router's .then() handler only triggers a re-render via setLazyCache (no route.children assignment). After resolution, subsequent matchRoutes calls invoke the user's function which returns the cached array synchronously. This keeps route definitions immutable — the user's caching function is the single source of truth for resolution state. https://claude.ai/code/session_01HqWiPByktzUpdesqZbNcHp
1 parent 3d03326 commit e9d370e

File tree

1 file changed

+42
-54
lines changed

1 file changed

+42
-54
lines changed

design/lazy-route-definitions.md

Lines changed: 42 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ children?: RouteDefinition[] | LazyRouteChildren;
225225

226226
`matchRoutes` remains **synchronous**. When it encounters a route whose `children` is a function, it **calls the function** to determine how to proceed:
227227

228-
1. **Sync result** (function returns an array): The children are resolved immediately. `matchRoutes` mutates `route.children` to the resolved array and continues matching children — exactly as if they had been static all along.
228+
1. **Sync result** (function returns an array): The children are resolved. `matchRoutes` uses the returned array for child matching within that call — exactly as if they had been static children.
229229

230230
2. **Async result** (function returns a promise): The children can't be resolved synchronously. The route matches as a prefix (same as having children), but only the parent is included in the match result.
231231

@@ -236,29 +236,32 @@ function matchRoute(route, pathname, options) {
236236
const isLazyChildren = typeof route.children === "function";
237237

238238
// Call the lazy function to resolve children (sync or async)
239+
let resolvedChildren: InternalRouteDefinition[] | undefined;
239240
if (isLazyChildren) {
240241
const result = route.children();
241242
if (Array.isArray(result)) {
242-
// Sync resolution — install children and continue matching
243-
route.children = internalRoutes(result);
243+
// Sync resolution — use returned array for matching
244+
resolvedChildren = internalRoutes(result);
244245
}
245-
// If result is a Promise, leave route.children as the function
246-
// (it will be called again later and should return cached result)
246+
// If result is a Promise, resolvedChildren stays undefined
247247
}
248248

249-
const hasStaticChildren =
250-
Array.isArray(route.children) && route.children.length > 0;
251-
const hasUnresolvedLazy = typeof route.children === "function"; // still a function = async
252-
const hasChildren = hasStaticChildren || hasUnresolvedLazy;
249+
const staticChildren = Array.isArray(route.children)
250+
? route.children
251+
: undefined;
252+
const children = resolvedChildren ?? staticChildren;
253+
const hasChildren =
254+
(children !== undefined && children.length > 0) ||
255+
(isLazyChildren && resolvedChildren === undefined);
253256

254257
// Lazy children affect isExact: route matches as prefix
255258
const isExact = route.exact ?? !hasChildren;
256259

257260
// ... path matching (unchanged) ...
258261

259-
if (hasStaticChildren) {
260-
// Existing child matching logic (unchanged)
261-
} else if (hasUnresolvedLazy) {
262+
if (children !== undefined && children.length > 0) {
263+
// Match against children (resolved lazy or static — same logic)
264+
} else if (isLazyChildren && resolvedChildren === undefined) {
262265
// Async children — return parent match only
263266
return [result];
264267
}
@@ -267,6 +270,8 @@ function matchRoute(route, pathname, options) {
267270
}
268271
```
269272

273+
**No mutation of route objects:** `matchRoutes` does not modify `route.children`. The returned array is used as a local variable for matching within that call. The function stays on the route object and is called again on subsequent `matchRoutes` invocations. Since the user's function caches its result, repeated calls are cheap (just return the cached value).
274+
270275
**Why `matchRoutes` calls the function:** This handles the case where the function returns a cached result synchronously (e.g., after a previous async resolution). If the function returns sync, matching continues without any Suspense or re-render — the route tree is fully resolved in a single pass. This is critical for resilience: if Router's state was lost (see [Router suspension edge case](#router-suspension)), the function returns sync on retry and `matchRoutes` resolves it immediately.
271276

272277
**Side effect in `matchRoutes`:** Calling the lazy function is technically a side effect (it may trigger a dynamic import on the first call). This is acceptable because: (1) it's idempotent — the user's caching ensures repeated calls return the same result, (2) it mirrors how `React.lazy` triggers loading on first render, and (3) in React strict mode, `useMemo` may run twice, but both calls get the same cached result.
@@ -374,7 +379,7 @@ export function Router({ routes: inputRoutes, ... }: RouterProps): ReactNode {
374379

375380
const matchedRoutesWithData = useMemo(() => {
376381
// matchRoutes now calls lazy functions internally —
377-
// sync results are resolved in-place, async results cause partial match.
382+
// sync results are used directly, async results cause partial match.
378383
// (existing matching + loader logic otherwise unchanged)
379384
}, [routes, adapter, urlObject, runLoaders, locationKey, lazyCache]);
380385
// ^^^^^^^^^
@@ -401,10 +406,10 @@ export function Router({ routes: inputRoutes, ... }: RouterProps): ReactNode {
401406
// Call the function — user's cache returns the same Promise that
402407
// matchRoutes received, so no duplicate loading is triggered.
403408
const promise = lazyFn();
404-
const voidPromise = promise.then((resolved) => {
405-
route.children = internalRoutes(resolved);
406-
// New Map reference triggers matchedRoutesWithData recomputation.
407-
// This runs in .then() (outside render) — a normal async state update.
409+
const voidPromise = promise.then(() => {
410+
// Trigger re-render. On re-render, matchRoutes calls the function
411+
// again — user's cache now returns sync → full match.
412+
// No mutation of route objects — the function stays on the route.
408413
setLazyCache((prev) => new Map(prev));
409414
});
410415
// Register promise in cache. setState-during-render on own state:
@@ -420,7 +425,7 @@ export function Router({ routes: inputRoutes, ... }: RouterProps): ReactNode {
420425
}
421426
```
422427

423-
**Note:** Router calls the lazy function a second time (after `matchRoutes` already called it). This is safe because the user's caching ensures the same Promise object is returned — no duplicate import is triggered. The Router needs to call it to attach its own `.then()` handler for the state update.
428+
**Note:** Router calls the lazy function a second time (after `matchRoutes` already called it). This is safe because the user's caching ensures the same Promise object is returned — no duplicate import is triggered. The Router needs to call it to attach its own `.then()` handler that triggers the re-render via `setLazyCache`.
424429

425430
**How the promise reaches `PendingOutlet`:**
426431

@@ -451,8 +456,8 @@ The Router's subscription to `currententrychange` wraps updates in `startTransit
451456
6. On re-render: promise is in cache, `RouteRenderer` produces `<PendingOutlet promise={...}>`
452457
7. `<PendingOutlet>` calls `use(promise)`**suspends inside the transition**
453458
8. React keeps the old page visible (transition behavior)
454-
9. Promise resolves → `.then()` mutates `route.children`, calls `setLazyCache` (async state update)
455-
10. React retries the transition with resolved children
459+
9. Promise resolves → `.then()` calls `setLazyCache` (async state update)
460+
10. React retries the transition
456461
11. `matchRoutes` calls function → sync (user cached) → full match, loaders run
457462
12. Transition completes
458463

@@ -488,11 +493,10 @@ User clicks link to /admin/settings
488493
h. PendingOutlet calls use(promise) → SUSPENDS
489494
i. React keeps old page visible (startTransition behavior)
490495
5. Lazy children resolve asynchronously:
491-
a. .then() mutates adminRoute.children = [settingsRoute, usersRoute, ...]
492-
b. setLazyCache(new Map(...)) → new cache reference (async state update)
496+
a. .then() calls setLazyCache(new Map(...)) → new cache reference (async state update)
493497
6. React retries the transition render:
494498
a. useMemo (lazyCache changed): matchRoutes calls function → sync (user cached)
495-
mutates route.children → full match [adminRoute, settingsRoute]
499+
→ full match [adminRoute, settingsRoute]
496500
b. executeLoaders runs for all matched routes
497501
c. RouteRenderer renders AdminLayout + Settings
498502
7. Transition completes → new page shown with full content
@@ -519,10 +523,9 @@ Browser loads /admin/settings directly
519523
e. Suspense boundary shows <Loading />
520524
5. User sees AdminLayout with loading fallback in outlet area
521525
6. Lazy children resolve:
522-
a. .then() mutates adminRoute.children = [settingsRoute, usersRoute, ...]
523-
b. setLazyCache(new Map(...)) → new cache reference (async state update)
526+
a. .then() calls setLazyCache(new Map(...)) → new cache reference (async state update)
524527
7. useMemo re-runs (lazyCache changed): matchRoutes calls function → sync (user cached)
525-
mutates route.children → full match: [adminRoute match, settingsRoute match]
528+
→ full match: [adminRoute match, settingsRoute match]
526529
8. executeLoaders runs settings loader
527530
9. RouteRenderer renders AdminLayout + Settings content
528531
```
@@ -533,23 +536,13 @@ If `<Outlet />` is not wrapped in `<Suspense>`, the suspension propagates up to
533536

534537
### Resolution Caching
535538

536-
Caching happens at two levels:
539+
Route objects are **never mutated**. `route.children` stays as the user-provided function for the lifetime of the route definition. Caching is handled entirely by the user's function and Router's state:
537540

538541
1. **User-level caching** (required): The lazy function itself caches its result. On first call it returns a Promise; on subsequent calls it returns the resolved array synchronously. This is the user's responsibility (see [Caching contract](#caching-contract)). This is the **primary** caching mechanism and the one that ensures resilience when React state is lost.
539542

540-
2. **In-place mutation** (optimization): After resolution, `matchRoutes` (sync path) and Router's `.then()` handler (async path) both mutate `route.children` to the resolved array:
541-
542-
```typescript
543-
// Before resolution:
544-
route.children = () => { ... }; // function
545-
546-
// After resolution:
547-
route.children = [settingsRoute, usersRoute, ...]; // array
548-
```
549-
550-
Once mutated, `matchRoutes` sees the array and takes the normal static-children path — the function is never called again. The route tree is indistinguishable from a fully-static one after all lazy subtrees are resolved.
543+
2. **Router's `lazyCache` state**: Stores in-flight promises so `PendingOutlet` can suspend via `use()`. Its identity change (new `Map` reference) triggers `matchedRoutesWithData` recomputation after a promise resolves.
551544

552-
**Trade-off:** This mutates the route definition objects. Since `route()` returns the input object as-is (just a type assertion), the mutation affects the original object passed by the user. This is intentional — the mutation is an optimization that replaces a one-shot factory with its result.
545+
After resolution, subsequent `matchRoutes` calls invoke the function again — but the user's cache returns the resolved array synchronously, so matching completes in a single pass with no suspension. The function call is cheap (a cache lookup) and keeps the route objects immutable.
553546

554547
## Edge Cases
555548

@@ -598,7 +591,7 @@ Navigate to: /admin/nonexistent
598591
2. Navigation is intercepted (parent matched)
599592
3. Navigation commits → React renders in transition
600593
4. Router registers promise in lazyCache (setState-during-render) → re-render → `PendingOutlet` suspends
601-
5. Lazy children resolve → `setLazyCache``matchRoutes` re-runs (function returns sync)
594+
5. Lazy children resolve → `setLazyCache``matchRoutes` re-runs (function now returns sync)
602595
6. `/admin` matches, but no child matches `/nonexistent`
603596
7. If `requireChildren` is true (default): re-match returns `null`
604597
8. Nothing renders
@@ -626,7 +619,7 @@ In practice, this is not a problem: for a user to submit a form on `/admin/setti
626619

627620
If the user navigates away while lazy children are loading:
628621

629-
- The in-flight lazy resolution continues in the background. When it resolves, it installs children into the route tree and calls `setLazyCache` to create a new `Map` reference. This is harmless — the installed children are correct and will be available for future navigations.
622+
- The in-flight lazy resolution continues in the background. When it resolves, `.then()` calls `setLazyCache` to create a new `Map` reference. This is harmless — the user's cache stores the result, and future `matchRoutes` calls will resolve synchronously.
630623
- The new navigation triggers a fresh `startTransition`, which supersedes the old one. React discards the old transition's render tree.
631624
- If the new navigation path also requires lazy resolution, a new `PendingOutlet` handles it independently.
632625

@@ -647,7 +640,7 @@ For SSR-critical routes, users should define children statically. Lazy children
647640

648641
### Route definitions prop change
649642

650-
If the `routes` prop passed to `<Router>` changes (new array/new objects), previously resolved lazy children live on the old objects. The new route definitions may have fresh lazy functions that need resolution. This works correctly because `matchRoutes` checks `typeof route.children === 'function'` — fresh functions trigger resolution, while already-resolved arrays (or mutated children) are matched synchronously.
643+
If the `routes` prop passed to `<Router>` changes (new array/new objects), the new route definitions may have fresh lazy functions that need resolution. Router clears its `lazyCache` (derived state pattern), and `matchRoutes` calls the new functions on the next render. Previously resolved children on old route objects are unaffected — they still have their original functions, and the user's cache for those functions still works.
651644

652645
### Router suspension
653646

@@ -658,17 +651,12 @@ With sync return from the user's cache:
658651
1. Router renders (first time, no state) → `matchRoutes` calls function → Promise → partial match
659652
2. Router registers in lazyCache (setState-during-render) → PendingOutlet suspends
660653
3. State is lost (tree discarded by parent Suspense)
661-
4. Promise resolves in background `.then()` mutates `route.children` (mutation persists on the route object even though React state is gone)
654+
4. Promise resolves in background (`.then()` calls `setLazyCache` which is now stale — harmless no-op)
662655
5. Later, React re-renders the tree from scratch
663-
6. Router renders fresh → `matchRoutes` sees `route.children` is now an array (mutated) → full match
664-
7. No suspension needed
665-
666-
Even if the in-place mutation from `.then()` hasn't run yet (race condition), the user's cache provides a second line of defense:
667-
668-
1. `matchRoutes` calls function → sync (user cached result) → mutates `route.children` → full match
669-
2. No suspension, no lazyCache needed
656+
6. Router renders fresh → `matchRoutes` calls function → sync (user cached result) → full match
657+
7. No suspension needed, no lazyCache needed
670658

671-
This is why the [caching contract](#caching-contract) matters: it ensures that the system converges to a resolved state regardless of how many times React state is discarded.
659+
This is why the [caching contract](#caching-contract) matters: it ensures that the system converges to a resolved state regardless of how many times React state is discarded. The user's function is the single source of truth for resolution state — not React state, and not route object mutation.
672660

673661
## Interaction with Existing Features
674662

@@ -795,7 +783,7 @@ useEffect(() => {
795783
**File:** `packages/router/src/core/matchRoutes.ts`
796784

797785
- When `typeof route.children === 'function'`: call the function
798-
- If result is an array (sync): mutate `route.children`, continue matching children normally
786+
- If result is an array (sync): use it for child matching (no mutation of route objects)
799787
- If result is a Promise (async): treat as having children for `isExact`, return parent-only match (bypass `requireChildren`)
800788

801789
### Step 3: Add `lazyCache` to `RouterContext`
@@ -847,7 +835,7 @@ Test cases:
847835
11. **Suspense fallback shown on initial load**: Verify that the `<Suspense>` boundary around `<Outlet />` shows its fallback during lazy resolution on initial load.
848836
12. **No Suspense fallback during navigation**: Verify that during navigation, the old page stays visible (transition behavior) and no Suspense fallback is shown.
849837
13. **Sync resolution (cached function)**: Lazy function returns array synchronously on second call. Verify `matchRoutes` resolves it immediately with no suspension.
850-
14. **`matchRoutes` handles sync return**: Call `matchRoutes` with a route whose children function returns an array. Verify full match is returned and `route.children` is mutated to the array.
838+
14. **`matchRoutes` handles sync return**: Call `matchRoutes` with a route whose children function returns an array. Verify full match is returned and `route.children` is NOT mutated (still a function).
851839

852840
### Step 8: Export types
853841

@@ -861,7 +849,7 @@ Test cases:
861849
| ---------------------------------------------- | ----------------------------------------------------------------------------- |
862850
| `packages/router/src/types.ts` | Update `InternalRouteDefinition.children` type |
863851
| `packages/router/src/route.ts` | Add `LazyRouteChildren` type; update `children` on all route definition types |
864-
| `packages/router/src/core/matchRoutes.ts` | Call lazy function; handle sync (mutate + match) and async (partial match) |
852+
| `packages/router/src/core/matchRoutes.ts` | Call lazy function; handle sync (match) and async (partial match) |
865853
| `packages/router/src/context/RouterContext.ts` | Add `lazyCache` to `RouterContextValue` |
866854
| `packages/router/src/Router/PendingOutlet.tsx` | New file: thin component that suspends via `use(promise)` |
867855
| `packages/router/src/Router/RouteRenderer.tsx` | Produce `<PendingOutlet>` outlet for routes with lazy children |

0 commit comments

Comments
 (0)