Repository navigation
Conversation
trackSelector ran beginTracking → selector → endTracking with nothing guarding the throw path. A selector that throws — an observer's render suspending on a thrown promise, or erroring to a boundary — skipped endTracking and left its tracker as tracking.current with trackCount above zero. Every later untracked get() then recorded its node in that orphan forever, retaining whole observable trees. Wrap the selector in try/finally so tracking always ends, and add regression tests at the trackSelector and observer + Suspense levels. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
trackSelectorrunsbeginTracking()→ selector →endTracking()with nothing between them guarding against a throw. When the selector throws,endTracking()never runs, and the tracker it created stays behind astracking.currentwithtrackCountabove zero.That is not a theoretical path.
observerrenders the whole component as a selector, so any observed component that suspends (a thrown promise from a third-party hook such as TanStack Query'suseSuspenseQuery) or throws to an error boundary leaves one of these orphans. From that moment on, everyget()anywhere in the app that runs outside a selector (effects, event handlers, plain code) records its node in the orphan'snodesMap, which nothing ever reads or disposes. Each node retains its root, so whole observable trees are pinned for the lifetime of the page.This PR wraps the selector call in
try/finallyso tracking always ends, and adds two regression tests.How we found it
Profiling a React/Capacitor app where a coach switches between athletes, each with a large workout observable (~10k leaf values). Every switch retained the previous athlete's workout, about 13.5 MB each; the heap went 111 → 165 MB after four switches and never came back. With this change the heap is flat. The
observerwrapping auseSuspenseQuerycall was the trigger, andtracking.currentwas already a stale tracker right after app load.The fix
Behaviour is unchanged when the selector returns. When it throws, tracking is balanced and the error (or Suspense promise) still propagates exactly as before, so React handles it the same way. This mirrors what
batch()already does aroundbeginBatch/endBatch, and whatrunInRenderdoes forrenderDepth(added in #638 for the same reason: "thrown selectors or nested scopes do not poison later updates").Tests
tests/tracking.test.ts: a selector that throws restores the outer tracking context and laterget()calls land there, not in a leaked tracker.tests/react.test.tsx: anobservercomponent that suspends on first render (a promise thrown from inside the observed render, as a data library's suspense hook would) leavestracking.currentundefined, both while suspended and after it resolves.Both tests fail on
mainand pass with this change. Assertions compare booleans and sizes on purpose: printing a leaked tracker in a failure message walks its proxies forever.eslintandprettierpass locally. The fulljestsuite passes except for one flake of the wall-clockLazy activation perf2test under the parallel run (37 ms against its 25 ms local budget); it measures a plainget()that never enterstrackSelector, and it passes in isolation on bothmainand this branch.tsc --noEmitreports only the pre-existing errors insrc/sync-plugins/supabase.tsandtanstack-query.ts, none in the touched files.🤖 Generated with Claude Code