fix(router): patch router-core so evicting an in-flight preload doesn't TypeError#1436
Conversation
…'t TypeError Fixes #1387. Datadog RUM has been recording (10x/day at peak, 8 sessions) a handled 'TypeError: Cannot read properties of undefined (reading "_nonReactive")' from router.preloadRoute. Root cause: several paths in @tanstack/router-core's load-matches.js re-look the match up with getMatch() after an await and dereference ._nonReactive without a guard. A preload's match only lives in cachedMatches, so navigating (or router.invalidate()/cache GC) while a hover-intent preload is in flight evicts it, getMatch() returns undefined, and preloadRoute console.error()s the TypeError — which RUM picks up on every hover-then-navigate race. Upstream report is TanStack/router#7759 with open fix PRs #7003/#7006; no released version contains a fix (we are on the latest 1.170.17/1.171.14). This ports PR #7003 as a pnpm patch: a missing match during load now throws an internal MatchLoadCancelledError that resolves the evicted match's controlled promises, aborts it, and is swallowed by preloadRoute (preload becomes a quiet no-op), and the background-reload cleanup no longer assumes the match still exists. The new regression tests reproduce the exact production error against the unpatched package (cache GC and invalidate() during an in-flight preload) and pass with the patch. Drop the patch once upstream ships the fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
/gemini review |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
Good instinct putting a version guard on this — patching router-core internals is exactly the kind of thing that should fail loudly on a dependency bump rather than silently stop applying. One thing worth fixing before merge: in the "else" branch of the patch ( Smaller one: the — KrAIs |
…anch Review feedback on #1436 (kriszyp): when a second load of the same match joins an in-flight loaderPromise and the cache evicts that match during the await, cleanupMatch was still undefined, so the eviction throw skipped resolving the evicted match's controlled promises. Assign cleanupMatch as soon as prevMatch is fetched so that branch cleans up too. Also make isMatchLoadCancelledError fall back to err.name === 'MatchCancel' so the cancellation path survives a dual-package hazard (two router-core instances), and add regression coverage for eviction during concurrent preloads of the same route. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@kriszyp Both addressed in 5a19ed4:
🤖 Addressed by Claude Code |
DavidCockerill
left a comment
There was a problem hiding this comment.
LGTM — faithful port of upstream TanStack/router #7003 as a pnpm patchedDependencies patch. It's a real cancellation (resolves the evicted match's controlled promises + aborts the controller), not a TypeError-swallow, so parked concurrent-preload awaiters wake cleanly — I traced the cleanupMatch threading and the last-valid-match tracking holds. Ships with three regression tests covering all eviction triggers (GC, invalidate, concurrent preloads) asserting on console.error, and pnpm hard-errors (not silent-drops) on the next version bump.
Two low-severity notes, neither blocking:
- Cancellation is thrown before
firstUnhandledRejectioninloadMatches, so if one sibling match is cancelled while another throws a genuine loader error, the cancellation wins and the real error is dropped. In practice the whole preloaded tree is being torn down so the sibling error is moot, and this matches upstream #7003. - Carrying the patch ahead of an upstream merge means a manual reconciliation if upstream lands the alternative (#7006) with a different shape — the CLAUDE.md note mitigates this. No action now.
kriszyp
left a comment
There was a problem hiding this comment.
LGTM — narrowly-scoped, version-guarded router-core patch that fixes the in-flight-preload eviction TypeError, and it's backed by a test. No hot-path cost (this only touches the router's cold eviction path).
Reviewed with a Gemini-assisted pass via review-queue.
— Claude (Sonnet 5)
Fixes #1387.
What
Datadog RUM has been recording a handled
TypeError: Cannot read properties of undefined (reading '_nonReactive')fromrouter.preloadRoute— escalated to 10 occurrences across 8 sessions in a day, across many routes.Root cause (upstream, TanStack/router#7759): several code paths in
@tanstack/router-core'sload-matches.jsre-look up the match withgetMatch(matchId)after anawaitand dereference._nonReactivewithout a guard. A preload's match only lives incachedMatches, so if the user navigates (orrouter.invalidate()/ cache GC runs) while a hover-intent preload is in flight, the match is evicted,getMatch()returnsundefined, andpreloadRouteconsole.errors the TypeError — which RUM picks up on every hover-then-navigate race.We're already on the latest published versions (
react-router@1.170.17/router-core@1.171.14); no released version contains a fix. Upstream has two open fix PRs (#7003, #7006).How
patches/@tanstack__router-core@1.171.14.patch(pnpmpatchedDependencies): ports upstream PR #7003. A missing match during load now throws an internalMatchLoadCancelledErrorthat resolves the evicted match's controlled promises and aborts it;preloadRouteswallows it (the preload becomes a quiet no-op). The background stale-reload cleanup also no longer assumes the match still exists. Patched in bothdist/esmanddist/cjs.src/router/__tests__/preloadEvictionRepro.test.ts: regression tests that reproduce the exact production error against the unpatched package (cache GC andinvalidate()during an in-flight preload) and pass with the patch.CLAUDE.md: documents the patch and what to do on the next router bump (check whether upstream shipped the fix; otherwise re-create the patch — pnpm will hard-error on the version mismatch, so it can't be silently lost).Verification
router-corewith the exact production TypeError, pass with the patch.Drop the patch once upstream ships the fix.
🤖 Generated with Claude Code