[jhuleatt]

Summary

Fixes two cache lifecycle defects in SuspenseSubject and preloadObservable:

1. Source subscriptions were never released on unmount:
   • SuspenseSubject piped innerObservable through shareReplay(1) (refCount: false), which keeps the underlying Firebase listener attached indefinitely even after all subscribers unmount.
   • this._warmupSubscription was never unsubscribed inside _subscribe(), keeping refCount >= 1 permanently.
   • When _suspenseEnabled was true, _timeoutHandler was a no-op (setTimeout(() => {}, this._timeoutWindow)), leaking abandoned <Suspense> renders forever.
2. Errored cache entries were never evicted (#742, #485):
   • Once an observable emitted an error (e.g., permission-denied before sign-in or on sign-out), the errored SuspenseSubject remained in preloadedObservables under its observableId, causing subsequent mounts or <ErrorBoundary> retries to immediately replay the stale error.

Changes

• Replace shareReplay(1) with share({ connector: () => new ReplaySubject<T>(1), resetOnError: true, resetOnComplete: false, resetOnRefCountZero: () => timer(RELEASE_DELAY_MS).pipe(tap(onEvict)) }) so the shared Firebase listener stays warm across React <StrictMode> double-mounts (RELEASE_DELAY_MS = 1000ms) and detaches + evicts from preloadedObservables once no subscribers remain.
• Unsubscribe this._warmupSubscription in _subscribe() after attaching the real subscriber so ownership hands off without detaching the underlying listener.
• Replace the _suspenseEnabled no-op timeout with releaseWarmupWhenSettled, which waits until firstEmission has settled (this._hasValue || this._error, preserving #525's guarantee for slow initial reads) before releasing unclaimed warm-up subscriptions.
• Schedule setTimeout(onEvict, RELEASE_DELAY_MS) on stream error so remounts and <ErrorBoundary> retries start a fresh subscription.

Closes #742
Closes #485

[tyler-reitz]

Requesting changes for one thing: the fix needs rxjs 7.1.0 or later, and dependencies allows ^6.6.3 || ^7.0.1.

The function form of resetOnRefCountZero arrived in 7.1.0. On 7.0.x the function is treated as a truthy true and resets immediately; on 6.x share() takes no arguments and ignores the config. With this exact config on 6.6.3 and 7.0.1, onEvict never fires, so those installs keep the leak with no error. CI only installs 7.8.1.

Your call which way: raise the floor to ^7.1.0, which breaks rxjs 6 users and so belongs on v5 (where Closes won't fire), or keep the range and say in the body that the fix applies to 7.1+ only.

Everything else checks out. Each new test goes red against main's src/ and when its fix is reverted. Non-blocking:

• Three paths have no test: the 1s grace window (a 0ms delay stays green), releaseWarmupWhenSettled (an immediate return stays green), and the identity guard in onEvict (a plain delete(id) stays green). The guard is right: a late subscriber holding an evicted subject re-arms its timer, which would otherwise delete the newer entry.
• Errored entries used to stay cached and now clear after 1s, in suspense mode too since the timeout is unconditional. That's what #742 asks for, but it's a behavior change under fix:.
• The TODO in the value getter ("burn the cache entry until the timeout fires") is stale now.

[tyler-reitz]

share never sees an error here: catchError(() => empty()) upstream turns it into a completion, so this option never applies (flipping it to false leaves the suite green). Errored entries are evicted only by the setTimeout(onEvict) in tap's error handler. Worth saying so here, so nobody deletes that timeout as redundant.

[tyler-reitz]

Since errors arrive as completions, this covers every errored stream too, and a completion cancels any pending refcount timer and stops it being rescheduled. So entries for streams that complete normally (the first() in useFirestoreDocOnce and useFirestoreDocDataOnce) never leave preloadedObservables: the listener is gone, but the entry stays, which on a server is still per-user growth (#790). Intended?

[tyler-reitz]

The three untested paths, the comments and the TODO are all addressed, and each new test goes red when its fix is reverted. One blocker left, and it comes from how the rxjs floor installs rather than from the code.

Blocking: this is breaking, so the title wants a !. It also fails silently on rxjs 6: rxjs is in dependencies, so an rxjs 6 app installs cleanly with a nested 7.x while rxfire binds to the app's 6.x, and unsubscribing never reaches the source. In a scratch app, five remounts left 5 listeners attached on this PR versus 1 on main. Moving rxjs to peerDependencies at ^7.1.0 would turn that into an install error instead.

Non-blocking:

• The if (this._error) branch in _subscribe has no test. Swapping it for subscriber.complete() keeps the suite green, but then a mounted hook with initialData whose stream errors resubscribes in a loop (59 subscriptions in 3s).
• *Once entries still stay cached on the usual path: the component subscribes before first() completes, which cancels the warm-up timer, and the teardown only evicts on error. Also evicting on _isComplete there should cover it. Fine as a follow-up under #790.

[tyler-reitz]

Correction to my review above: on this PR's own build, rxjs and rxfire are inlined into dist, so reactfire's built-in hooks run on the bundled rxjs 7 and don't leak in rxjs 6 apps (useUser and useFirestoreDocData tore down 5 of 5). What I measured applies today only to apps passing their own rxjs 6 observables to useObservable. It becomes every hook once #838 externalizes rxjs and rxfire: with both merged, an rxjs 6.6.7 app leaks one listener per remount (1,2,3,4,5 after five cycles). The ask stands, a ! plus rxjs as a ^7.1.0 peer dependency, in whichever of the two lands first.