Drop auto-fire from PendingRpcRequest.reactiveStore()#1628
Open
mcintyre94 wants to merge 1 commit into
Open
Conversation
🦋 Changeset detectedLatest commit: 593face 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 |
This was referenced May 13, 2026
Member
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
BundleMonFiles updated (13)
Unchanged files (134)
Total files change +3.32KB +0.63% Final result: ✅ View report in BundleMon website ➡️ |
Contributor
|
Documentation Preview: https://kit-docs-7bwcreamt-anza-tech.vercel.app |
Member
Author
trevor-cortex
left a comment
trevor-cortex
left a comment
There was a problem hiding this comment.
(Submitting as comment — I don't have approval rights on this repo. Treat this as a 👍 from me.)
Clean, focused breaking change. Dropping auto-fire from reactiveStore() brings it in line with createReactiveActionStore(fn) and removes the asymmetry around the first dispatch — the motivation in the changeset (uniform signal attachment across all dispatches) is well-argued, and the upstack abort-signal work will clearly benefit.
The diff is tight: implementation, docblock, README, and tests all move together, and the migration snippet in the changeset is exactly what callers need. Major bump on @solana/rpc-spec is the right call.
Things to watch / verify
- Subscriber-notification test relaxation:
notifies subscribers when state changesswitched fromtoHaveBeenCalledTimes(1)totoBeGreaterThanOrEqual(2). That's correct — subscribers now attach before dispatch, so they observeidle → runningandrunning → success(two notifications) instead of onlyrunning → success(one). The>= 2is appropriately defensive againstReactiveActionStoreadding intermediate transitions later, though a strict=== 2would be a tighter contract today. Either is fine; flagging only so a future reader doesn't wonder why it's loose. - Downstream callers in the monorepo: worth a quick check that no other package (e.g.
@solana/rpc, any react/svelte helpers, examples, docs-site snippets) calls.reactiveStore()and relies on the auto-fire behavior. The diff only touchesrpc-spec, so any in-repo consumers would now silently sit inidle. Reviewers familiar with the wider tree should sanity-check this. onRetry={store.dispatch}in the README example remains correct (initial dispatch + retry both go through the same call), which is exactly the simplification this PR is unlocking.
No blocking concerns from me.
lorisleiva
approved these changes
May 13, 2026
Member
lorisleiva
left a comment
lorisleiva
left a comment
There was a problem hiding this comment.
Nice, a more predictable API. 👌
mcintyre94
force-pushed
the
react/use-action
branch
from
000d8bb to
44975cf
Compare
mcintyre94
force-pushed
the
reactive-store/no-auto-dispatch
branch
from
0c7e36f to
23bc249
Compare
mcintyre94
force-pushed
the
react/use-action
branch
from
44975cf to
67b0db4
Compare
mcintyre94
force-pushed
the
reactive-store/no-auto-dispatch
branch
from
23bc249 to
5fac7a4
Compare
This was referenced May 19, 2026
`reactiveStore()` no longer dispatches the request automatically when called. It now returns a `ReactiveActionStore` in the `idle` state, leaving the caller responsible for the initial `dispatch()`. The auto-fire was created to make `reactiveStore()` parallel to `send()` — the consumer got a "live" store back the same way they get a live promise. In practice the auto-fire created an asymmetry at the start of the store's lifecycle: the initial request had no caller-visible dispatch site, so attaching an `AbortSignal` to that one specific attempt required a separate construction-time option distinct from the mechanism used for all subsequent attempts. Without the auto-fire, every dispatch is the caller's, and signal attachment lives uniformly at the dispatch site. This also brings `reactiveStore()` in line with `createReactiveActionStore(fn)` (which doesn't auto-fire either), so the two creation paths now have identical "construct then dispatch" semantics. React hooks consuming `reactiveStore` (e.g. `useRequest`) handle the dispatch internally — consumers of those hooks see no change. Major version bump for `@solana/rpc-spec`: callers using `reactiveStore()` directly will need to add an explicit `store.dispatch()` to keep the existing fire-on-creation behavior.
mcintyre94
force-pushed
the
react/use-action
branch
from
67b0db4 to
a9c5449
Compare
mcintyre94
force-pushed
the
reactive-store/no-auto-dispatch
branch
from
5fac7a4 to
593face
Compare
This was referenced May 20, 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.
Problem
I've realised that auto-dispatch on
RpcPendingRequest.reactiveStore()was a mistake, it complicates the API when we add abort signals and is more opinionated/less flexible than theReactiveActionStoreitself.Summary of Changes
This PR removes auto-dispatch from
RpcPendingRequest.reactiveStore(). It now returns a store in theidlestate, which can be dispatched as normal, at the caller's control.Among other things will simplify adding abort signals to the store in the upstack PR, becasuse all dispatches now work the same, and
.reactiveStore()is no longer responsible for a dispatch.Note: the "major" label just indicates the major changeset for the breaking change, just a note for myself managing the stack.