Skip to content

createRefetchableFragment: superseded refetch subscriptions leak and can clobber newer data #79

Description

@nyanrus

Where

src/primitives/createRefetchableFragment.ts

  • refetchObservable (a createMemo) — L181-205, raw subscribe at L192
  • the completion createComputed — L206-224, raw subscribe at L211
// L191-192
const replaySubject = new ReplaySubject<GraphQLResponse>();
fetchObservable.subscribe({ /* … */ });      // <- no onCleanup

// L211
obs.subscribe({
  async complete() {
    const data = await waitForFragmentData(environment(), refetchableRequest.fragment, getQueryRef(refetchQuery));
    // …
    setFragmentRef(unwrap(refetchedFragmentRef));   // L221
  },
});                                            // <- no onCleanup

What happens

Both .subscribe(...) calls sit inside a reactive computation with no onCleanup. In SolidJS a raw subscribe inside a memo/computed is not torn down when the computation re-runs — only onCleanup callbacks and owned reactive nodes are. The memo tracks state().refetchQuery and preloadedQueryRef() (L182-183), both of which change on every refetch(), so each superseded refetch leaves its previous subscription live.

Why it matters

  1. Subscriptions accumulate across refetches. On the __internal.fetchQuery branch (L189), each stale subscription also keeps a fetchQueryDeduped entry alive by observer ref-count, so the superseded in-flight request is never cancelled.
  2. Stale completion clobbers newer data. relay's ReplaySubject replays the terminal complete to every subscriber, so a later-completing stale refetch re-runs waitForFragmentDatasetFragmentRef with the old refetchQuery (L216/L221). An out-of-order refetch overwrites the newer result.

Proposal

Capture each subscription and release it on cleanup:

const sub = fetchObservable.subscribe({ /* … */ });
onCleanup(() => sub.unsubscribe());
// …and the same for the createComputed's obs.subscribe(...)

onCleanup in a memo/computed runs before each re-run, bounding live subscriptions to one.

I have this ready as a PR — the fix plus the out-of-order-refetch regression test — and I'll open it if the direction looks right.

Costs / risks

Minimal. onCleanup inside a memo/computed is standard SolidJS; a single refetch behaves identically. Only the superseded-subscription case changes.

Alternative

Leave as-is — defensible only if refetch is never called more than once per mount and out-of-order completion never happens. Search / filter UIs refetch repeatedly, so the clobber is reachable in practice.

References

Reproduced with a regression test: refetch({id:"2"}) then refetch({id:"3"}), resolve the newer first and the older last, assert the newer (id 3) wins. Fails on current main (reads the older id 2), passes with the fix.

About this issue

Drafted by Shiro (Claude Opus 4.8), an AI assistant working with @nyanrus. If I've misread how these computations re-run or when the completion fires, please tell me — I'd rather be corrected than confidently wrong.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions