Add useAction for tracked async actions#1612
Conversation
🦋 Changeset detectedLatest commit: a9c5449 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 |
|
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 (10)
Unchanged files (137)
Total files change +3.4KB +0.65% Final result: ✅ View report in BundleMon website ➡️ |
|
Documentation Preview: https://kit-docs-aej7gzfvw-anza-tech.vercel.app |
mcintyre94
force-pushed
the
react/use-action
branch
from
8d881fc to
1f901a7
Compare
mcintyre94
force-pushed
the
graphite-base/1612
branch
from
42064ae to
75196f0
Compare
mcintyre94
force-pushed
the
react/use-action
branch
from
1f901a7 to
f759d97
Compare
mcintyre94
force-pushed
the
graphite-base/1612
branch
from
75196f0 to
e7cf86e
Compare
mcintyre94
force-pushed
the
react/provider
branch
2 times, most recently
from
afabea4 to
a9d79e3
Compare
mcintyre94
force-pushed
the
react/use-action
branch
from
f759d97 to
916d08c
Compare
mcintyre94
force-pushed
the
react/use-action
branch
from
916d08c to
22b4547
Compare
trevor-cortex
left a comment
trevor-cortex
left a comment
There was a problem hiding this comment.
Summary
Adds useAction to @solana/react: a generic React hook that bridges any async function into a tracked action by wrapping createReactiveActionStore from @solana/subscribable. The hook is the React-side adapter for the existing framework-agnostic state machine — store is created once, the user's fn is held in a layout-effect-synced ref so the latest closure is invoked on every send(...) (no deps array), state is consumed via useSyncExternalStore, and the store is reset() on unmount to abort any in-flight call. The shape (status + data + error + derived is* booleans + stable send / reset) matches the supersede-on-second-call / stale-while-revalidate semantics already encoded in the underlying store.
Good, well-scoped PR. Nice docblocks and a thorough test suite covering all four state transitions, supersede semantics, SWR data preservation, fresh-closure capture, and reference stability of send/reset. Changeset is present and well-written; the tsconfig.json ESNext.Promise → ES2024.Promise narrowing is appropriate (Promise.withResolvers landed in ES2024 and the tests rely on it). README placement under the foundational-hooks section before ## Hooks (wallet hooks) reads consistently with the existing layout.
Things to watch / discussion points
Nothing blocking — the points below are suggestions worth a glance:
-
Store creation via
useMemo([])— React technically reserves the right to discarduseMemocaches; the canonical "create once, never re-create" pattern for store-like objects isconst [store] = useState(() => createReactiveActionStore(...))(oruseRefwith lazy init). In practice this doesn't bite today, butuseStatemakes the lifetime guarantee explicit and is whatzustand,jotai, etc. use for the same situation. Inline comment with more detail. -
useLayoutEffectand SSR —useLayoutEffectemits the well-known no-op-on-server warning under React SSR. This package builds a Node entrypoint anduseSyncExternalStoreitself is SSR-safe, so consumers may render this in an SSR shell. The common workaround isconst useIsoLayoutEffect = typeof window !== 'undefined' ? useLayoutEffect : useEffect. Worth confirming whether@solana/reactconsiders SSR a target. -
Ref-staleness during the layout-effect window — the latest-
fnref is assigned inuseLayoutEffect, which means asend(...)invoked from a child component's layout effect (which runs before the parent's) could still see the previousfn. Vanishingly unlikely in practice, and TanStack/wagmi have the same edge — flagging only for awareness.
For subsequent reviewers
- Verify the
useLayoutEffectchoice against whatever SSR posture@solana/reacttargets — if SSR is supported, swap for thetypeof windowguarded variant. - Sanity-check the assumption baked into the silenced
react-hooks/refs: it relies oncreateReactiveActionStoreonly invoking its callback insidedispatch/dispatchAsync, never during construction orgetState. That matches the current implementation inpackages/subscribable/src/reactive-action-store.ts, so the rule suppression is sound — but it's an invariant worth keeping in mind if that store ever gains eager behavior. - The
keeps stable send / reset referencestest passes trivially today because both come straight off the store, which is created with[]deps. If point (1) above changes the construction strategy, that test still holds.
mcintyre94
force-pushed
the
react/use-action
branch
from
22b4547 to
7c3927e
Compare
1 similar comment
trevor-cortex
left a comment
trevor-cortex
left a comment
There was a problem hiding this comment.
Re-review
Would approve if I had repo write access — GitHub blocks APPROVE reviews from accounts without explicit access, so leaving this as a comment.
All three points from the prior review are addressed:
- Store lifetime —
useMemo([])→useState(() => createReactiveActionStore(...)). Lifetime is now an explicit React guarantee rather than a best-effort cache. - SSR safety —
useIsomorphicLayoutEffectshim added with a clear inline comment explaining why falling back touseEffecton the server is functionally equivalent (no event handlers can fire during SSR, so the ref-sync timing doesn't matter there). - Typetest — the trivial
result.error satisfies unknownwas dropped; the remaining assertions (ActionResult<...>shape,send(7) satisfies Promise<string>,data satisfies string | undefined) are all meaningful.
The eslint-disable react-hooks/refs suppression on the useState initializer is correct: fnRef.current is only read inside the dispatchAsync body in createReactiveActionStore, never at construction or during getState, so the rule's concern doesn't apply. The accompanying comment captures this invariant well.
LGTM from my side. 👍
mcintyre94
force-pushed
the
react/provider
branch
from
d073150 to
f473f42
Compare
mcintyre94
force-pushed
the
react/use-action
branch
from
7c3927e to
6f0326e
Compare
lorisleiva
left a comment
lorisleiva
left a comment
There was a problem hiding this comment.
Looks good! Just one important comment IMO to avoid introducing two terms for the same concept.
| * `error`. Awaiters that read the resolved value (e.g. to navigate on success) should filter | ||
| * supersede rejections with `isAbortError` from `@solana/promises`. | ||
| */ | ||
| send: (...args: TArgs) => Promise<TResult>; |
There was a problem hiding this comment.
Should this not be dispatch to match the ReactiveActionStore API?
export type ReactiveActionStore<TArgs extends readonly unknown[], TResult> = {
readonly dispatch: (...args: TArgs) => void;
readonly dispatchAsync: (...args: TArgs) => Promise<TResult>;
readonly getState: () => ReactiveActionState<TResult>;
readonly reset: () => void;
readonly subscribe: (listener: () => void) => () => void;
};I feel like introducing a new dispatch term here could be more confusing than helping.
There was a problem hiding this comment.
Changed to dispatch, I think this is mostly better. The one thing I'd flag is that it's actually mirroring dispatchAsync (ie you can await the result if you need to), but I think that's unlikely to be confusing in practice. I don't think there's value in distinguishing dispatch and dispatchAsync in the React layer, and I think using dispatchAsync would be confusing since it's mostly going to be fire-and-forget.
There was a problem hiding this comment.
Agreed! Appreciate the heads up though. 🙏
mcintyre94
force-pushed
the
graphite-base/1612
branch
from
f473f42 to
0d2bb30
Compare
mcintyre94
force-pushed
the
react/use-action
branch
from
6f0326e to
000d8bb
Compare
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
mcintyre94
force-pushed
the
react/use-action
branch
from
000d8bb to
44975cf
Compare
mcintyre94
force-pushed
the
react/provider
branch
from
0d2bb30 to
e6facdd
Compare
mcintyre94
force-pushed
the
react/provider
branch
from
e6facdd to
c99b375
Compare
mcintyre94
force-pushed
the
react/use-action
branch
from
44975cf to
67b0db4
Compare
Bridges an arbitrary async function into a reactive `ActionResult<TArgs, TResult>` with `send` / `status` / `data` / `error` / `reset`. Each `send(...)` runs the function with a fresh `AbortSignal` and tracks its lifecycle through React state; a second `send` while a first is in flight aborts the first. `fn` is held in a ref that always points at the latest render's closure — there is no `deps` array to maintain. Each `send(...)` invokes the most recently rendered `fn`, so values captured inside (form state, route params, etc.) are always fresh. This matches the convention used by `useMutation` in TanStack Query and `useWriteContract` in wagmi. In-flight calls are unaffected — they continue with the closure they captured at dispatch time. Built on `createReactiveActionStore` from `@solana/subscribable`. Awaiters of a superseded call see a rejection with an `AbortError` filterable via `isAbortError` from `@solana/promises`.
mcintyre94
force-pushed
the
react/use-action
branch
from
67b0db4 to
a9c5449
Compare
mcintyre94
force-pushed
the
react/provider
branch
from
c99b375 to
7eedd39
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
3 participants
Summary of Changes
This PR adds the
useActionhook in React. This bridges any async action into theReactiveActionStorestate machine:The function passed to
useActiondoesn't need to be memoised etc, we handle that lifecycle internally with a ref.The status comes from
ReactiveActionStoreand can be idle, running, success, error. The react hook adds boolean aliasesisRunningetc. Later requests abort previous ones, and previous data remains available for SWR.This is available for apps for any async functionality they need, and plugin hooks such as
useSendTransactionwill also be built on top of it.