Add sync reactiveStore() to pending RPC subscription#1553
Merged
Conversation
🦋 Changeset detectedLatest commit: 26ce0b0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 47 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 |
Member
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
BundleMonFiles updated (4)
Unchanged files (143)
Total files change +287B +0.05% Final result: ✅ View report in BundleMon website ➡️ |
mcintyre94
force-pushed
the
subscription-reactive-store
branch
3 times, most recently
from
44cf49d to
bd46726
Compare
Contributor
|
Documentation Preview: https://kit-docs-fr9o7tmdf-anza-tech.vercel.app |
Member
Author
trevor-cortex
left a comment
trevor-cortex
left a comment
There was a problem hiding this comment.
Summary
Adds a synchronous reactiveStore() method to PendingRpcSubscriptionsRequest alongside the existing (now @deprecated) async reactive(). The new method delegates to createReactiveStoreFromDataPublisherFactory from the prior PR, so consumers get a store they can use immediately — status: 'loading' covers publisher creation and waiting for the first notification, and retry() re-invokes the transport. This is a clear UX win for useSyncExternalStore and similar reactive primitives, where having to await a promise before you have a store is awkward.
The changeset correctly marks this as a minor bump on @solana/rpc-subscriptions-spec and @solana/kit. Since the repo is pre-1.0 and reactive() is only soft-deprecated, this is appropriately non-breaking.
Key things to watch for
reactive()is only JSDoc-deprecated, still fully functional. Downstream mocks inkitandtransaction-confirmationadd a no-opreactiveStore: jest.fn()purely to satisfy the interface — confirms nothing internal has migrated yet, which is fine for this PR.- Naming: as the author calls out,
reactiveStore()is more descriptive thanreactive(). Worth locking in the name before 1.0 — if the intent is eventually to rename back toreactive()as a breaking change, committing to one or the other now would avoid carrying both long-term. - Test relocation: the old
rpc-subscriptions-reactive-test.tswas deleted and its suite moved verbatim intorpc-subscriptions-test.tsalongside the newreactiveStore()suite. Pure reorganization, no behavioural change.
Notes for subsequent reviewers
- The
reactiveStore()implementation inrpc-subscriptions.tsis a thin pass-through: it forwards the caller'sabortSignalboth to the factory (as the outer signal) and into the transport on each connect. Mirrors howreactive()/subscribe()already handle abort propagation. - The new test suite covers loading, notifications, error surfacing, and retry. Two gaps worth considering as follow-ups (not blocking): (1) a test that aborting the caller's signal actually stops the store from receiving further notifications, and (2) a test that
getUnifiedState()returns a stable object reference between unrelated calls — that stability is explicitly documented as important foruseSyncExternalStore. flushMicrotasks()in the new test ticks twice — enough for the.then()insideconnect()plus the factory resolution. A one-line comment explaining the two ticks would help future readers.
Overall shape LGTM.
mcintyre94
force-pushed
the
subscription-reactive-store
branch
from
bd46726 to
57fd7dd
Compare
This was referenced Apr 22, 2026
mcintyre94
force-pushed
the
reactive-store-updates
branch
from
4baf015 to
1d06dcc
Compare
mcintyre94
force-pushed
the
subscription-reactive-store
branch
from
57fd7dd to
330cffd
Compare
mcintyre94
force-pushed
the
reactive-store-updates
branch
from
1d06dcc to
cb4c655
Compare
mcintyre94
force-pushed
the
subscription-reactive-store
branch
from
330cffd to
a9b4199
Compare
Member
Author
Merge activity
|
mcintyre94
changed the base branch from
reactive-store-updates
to
graphite-base/1553
mcintyre94
force-pushed
the
subscription-reactive-store
branch
from
a9b4199 to
26ce0b0
Compare
Contributor
|
🔎💬 Inkeep AI search and chat service is syncing content for source 'Solana Kit Docs' |
This was referenced May 6, 2026
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
3 participants
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 of Changes
The previous PR updated
ReactiveStore, and added a newcreateReactiveStoreFromDataPublisherFactoryfunction that takes acreateDataPublisher(): Promise<DataPublisher>as input, instead of justDataPublisher. This enables retry functionality, where the publisher is re-created and re-subscribed to, without losing other store state.But it also enables
rpcSubscriptions.someNotification(...).reactive()to be made synchronous: previously it needed to create theDataPublisherasynchronously, now it can pass that promise tocreateReactiveStoreFromDataPublisherFactory. That function already handles the lifecycle of creating theDataPublisherfrom the factory.This is a significantly better API for reactive consumers, as they have a synchronous store available immediately that will begin publishing data. They no longer need to handle the async store creation. The store's
loadingstate covers both theDataPublisherbeing created and waiting for the first publish, and handles errors creating theDataPublisher. In react specifically, there's no need to awkwardly wrap the promise in auseEffectand handle extra state.React example:
Making
reactive()synchronous would be a breaking change, so this PR adds.reactiveStore()as a sync method on pending RPC subscriptions.reactive()is deprecated.TBH I think
reactiveStore()might be the better name, but we could switch back toreactive()as a breaking change later if we want to.