Move bridgeStoreToAsyncIterable from react/query to subscribable/ - #1811
Conversation
🦋 Changeset detectedLatest commit: d67e6c8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 48 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
BundleMonFiles updated (10)
Unchanged files (140)
Total files change +1.48KB +0.27% Final result: ✅ View report in BundleMon website ➡️ |
|
Documentation Preview: https://kit-docs-etp1ue4mg-anza-tech.vercel.app |
trevor-cortex
left a comment
trevor-cortex
left a comment
There was a problem hiding this comment.
Nice refactor — LGTM overall (leaving as COMMENT since I don't have approval rights on this repo). Moving bridgeStoreToAsyncIterable to @solana/subscribable is the right call: it's a store adapter with no React/TanStack coupling, and the behavioural change (bridge only observes; caller owns connect()/reset()) aligns it with the rest of the package where a store does nothing until you connect it. The new "seeds from current snapshot" semantic falls out cleanly and is a nice quality-of-life improvement — order of connect vs iteration no longer matters.
Correctness spot-checks all pass:
- Seeding:
subscribethen a synchronousonChange()is safe — latest-wins collapses any duplicate emission if the store also fires synchronously on subscribe. - Already-aborted signal: the
while (true)loop's first check issignal.aborted, so an entering-aborted state returns cleanly before touchingfailure/latest. Abort still wins over an incidental store error triggered by the abort itself. - Already-errored store:
onChange()seedsfailureand the loop throws on first pull — covered by the newthrows an error already present when iteration beginstest. - Hook call sites: both
useSubscriptionQueryanduseTrackedDataQueryconnect before handing the store to the bridge, so the pending first pull will see either the seeded value or park correctly.
Error code addition is well-placed (sequential in the reserved 8195000–8195999 subscribable range, added to the union alphabetically, matching message). Retaining SOLANA_ERROR__REACT__SUBSCRIPTION_CLOSED_WITHOUT_ERROR for the SWR bridge is correct — you can't remove codes, and re-pointing SWR would be a breaking behavioural change. Worth a follow-up thought (not this PR): the two codes now describe the same condition ("stream closed without a payload"), so consumers checking one won't catch the other. If SWR ever migrates in a major bump it'd be nice to converge on the new subscribable code.
Docs (reactive-stores.mdx, subscribable/README.md) are excellent — the contrast against createAsyncIterableFromDataPublisher (queued, publisher-level) and the RPC subscription's own iterable (raw transport, no store lifecycle) is exactly the framing readers need. Docblock covers @param/@throws/@see/@example per the skill.
Test updates look right, and the switch to throwing stubs for connect/withSignal/reset in createFakeStore is textbook per the AGENTS.md placeholder-mock guidance — an accidental lifecycle call now fails loudly.
One consistency question for subsequent reviewers to weigh in on (see inline on useTrackedDataQuery.ts).
Changeset marks @solana/subscribable, @solana/errors, @solana/kit, @solana/react all minor. That looks right: new public export in subscribable+kit+errors; react's useSubscriptionQuery/useTrackedDataQuery change is behavioural (no more reset() on teardown) but the consumer-observable state is the same (connection torn down via signal abort, store GC'd), so minor is defensible pre-1.0.
d480375 to
d67e6c8
Compare
|
Because there has been no activity on this PR for 14 days since it was merged, it has been automatically locked. Please open a new issue if it requires a follow up. |
3 participants
Problem
When we added the Tanstack Query versions of
useSubscriptionanduseTrackedData, we added a bridge to anAsyncIterablefrom theReactiveStreamStore. This is how TanStack's (experimental)streamedQueryis implemented.This bridge can be useful for consumers outside of Tanstack Query, which want to consume an
AsyncIterable. It's likely that more/future libraries will eventually standardise on this data structure too.We intentionally didn't just use the
AsyncIterablefromrpcSubscriptions.method().subscribe()for this, for compatibility withuseSubscriptionand also because the store lifecycle is better suited for UI, always providing the latest data and providing status etc.So any other consumer looking for an
AsyncIterablein reactive UI would benefit from this bridge, which was internal inreact/query.Summary of Changes
This PR moves
bridgeStoreToAsyncIterabletopackages/subscribableas a public export.It's made less opinionated and now just observes the store, the caller still controls connect/reset. This aligns it with the rest of the
subscribableAPI. Thereact/queryconsumers are updated accordingly.