Repository navigation
fix: give each escrow/subscription row an independent pending indicator - #65
Merged
abayomicornelius merged 10 commits intoAug 17, 2026
Merged
abayomicornelius merged 10 commits into
abayomicornelius merged 10 commits into
Conversation
A single shared useMutation's .variables is a scalar overwritten by the most recent .mutate() call, not a record of everything currently pending — so deriving a row's loading indicator from it silently clobbers another row's indicator when two actions overlap (StellarSend#52). usePendingIds tracks a Set of in-flight ids instead, updated independently of any single mutation instance.
releaseMutation.variables is a single scalar overwritten by the most recent .mutate() call — releasing escrow B while escrow A's release is still in flight silently un-showed A's spinner (and re-enabled its button) even though A's request hadn't settled (StellarSend#52). Escrow.tsx now tracks releasing ids via usePendingIds, and EscrowList/EscrowItem derive isReleasing from Set membership instead of scalar equality.
Same clobbering bug as the release fix, but for refundMutation — a depositor refunding escrow B while escrow A's refund is still in flight silently un-showed A's spinner (StellarSend#52). EscrowList now derives isRefunding from a Set of in-flight refunding ids too, matching the release fix.
…ator Identical shape to the escrow release/refund bug: cancelMutation was shared across the whole SubscriptionList, so cancelling subscription B while subscription A's cancellation was still in flight silently un-showed A's spinner (StellarSend#52). Subscriptions.tsx now tracks cancelling ids via usePendingIds, matching the escrow fix.
Two ids tracked concurrently stay independently pending; resolving one never clears a different, still-pending id — the exact clobbering this hook exists to prevent (StellarSend#52). Also covers rejection (cleared without an unhandled rejection), same-id re-tracking, and track's referential stability across renders.
Two different rows in releasingIds show as releasing at the same time without either one's state overwriting the other's — the UI symptom of StellarSend#52. Also covers undefined releasingIds rendering nothing as pending.
Mirrors the EscrowList coverage: two different rows in cancellingIds show as cancelling at the same time without either one's state overwriting the other's (StellarSend#52).
Combines the real useReleaseEscrow mutation with usePendingIds (mocking only the API/wallet boundary), following the issue's own suggested testing strategy: releasing escrow B while escrow A's release is still in flight must not affect A's pending state. Explicitly asserts releaseMutation.variables becomes 'escrowB' at that point — the old, wrong source of truth — while pendingIds correctly still has both.
Mirrors the release test for useRefundEscrow: refunding escrow B while escrow A's refund is still in flight must not affect A's pending state, proven against the real mutation rather than a mock of usePendingIds itself.
Same shape as the escrow release/refund tests but for useCancelSubscription: cancelling subscription B while A's cancellation is still in flight must not affect A's pending state. Completes coverage for all three mutations named in StellarSend#52.
Contributor
|
All 4 checks passed. good one |
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
2 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
Escrow.tsxandSubscriptions.tsxeach derived every row's loading spinner from a single shared mutation's.variables— the argument of the most recent.mutate()call, not a record of everything currently pending. Releasing escrow B while escrow A's release was still in flight silently overwrotereleaseMutation.variablesto'escrowB', so escrow A's spinner disappeared and its button re-enabled even though its request hadn't settled — inviting a second, racing call against the same escrow. Identical shape forrefundMutationandSubscriptions.tsx'scancelMutation.usePendingIdshook tracks aSetof in-flight ids instead, updated viatrack(id, promise)independently of any single mutation instance.Escrow.tsx/Subscriptions.tsxnow callmutation.mutateAsync(id)throughtrackrather than deriving state from.variables.EscrowList/SubscriptionListderive each row'sisReleasing/isRefunding/isCancellingfromSet.has(id)instead of scalar equality.Closes #52
Test plan
npm run type-check— cleannpm run lint— cleannpm run build— succeedsnpx vitest run— 91/91 pass (74 pre-existing + 17 new), including:usePendingIds.test.ts— tracking, isolation between ids, cleanup on both resolve/reject, no unhandled rejectionEscrowList.test.tsx/SubscriptionList.test.tsx— two different rows show as pending simultaneously without clobbering each otheruseEscrows.test.tsx/useSubscriptions.test.tsx— end-to-end reproduction of Escrow release/refund (and Subscription cancel) share one mutation instance across the whole list — concurrent actions on different rows clobber each other's loading state #52's exact repro steps against the realuseReleaseEscrow/useRefundEscrow/useCancelSubscriptionmutations (API/wallet mocked at the boundary only): starting a second mutation while the first is still in flight keeps both independently pending, and resolving one doesn't clear the other