-
Notifications
You must be signed in to change notification settings - Fork 1.6k
chore: refactor send events to be more consistent #5351
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 24da3af The changes in this PR will be included in the next version bump. This PR includes no changesetsWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
10 Skipped Deployments
|
Visual Regression Test Results ✅ Passed✨ No visual changes detected Chromatic Build: https://www.chromatic.com/build?appId=6493191bf4b10fed8ca7353f&number=415 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors send-related event tracking to improve consistency by centralizing all event firing in the SendController.sendToken() method rather than having events scattered across UI components and various helper methods.
Key Changes:
- Moved
SEND_INITIATED,SEND_SUCCESS,SEND_REJECTED, andSEND_ERRORevent tracking from UI and helper methods into the mainsendToken()method - Replaced
returnstatements withbreakstatements in the chain namespace switch to allow the success event to fire after send completion - Removed duplicate event tracking calls from
sendEvmToken(),sendNativeToken(),sendERC20Token(), andsendSolanaToken()
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/scaffold-ui/src/views/w3m-wallet-send-preview-view/index.ts | Removed error event tracking from UI layer (SEND_REJECTED/SEND_ERROR), moving responsibility to controller |
| packages/controllers/src/controllers/SendController.ts | Centralized all send-related event tracking in sendToken() method; added Event type import; replaced return with break statements in switch cases; removed duplicate event calls from helper methods |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📦 Bundle Size Check✅ All bundles are within size limits 📊 View detailed bundle sizes> @reown/[email protected] size /home/runner/work/appkit/appkit > size-limit |
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Stale Hash Corrupts Transaction Events
The sendEvmToken method only updates state.hash when hash is truthy, but resetSend doesn't clear state.hash. If a transaction returns no hash, the SEND_SUCCESS event will incorrectly report the hash from a previous transaction instead of an empty string, causing event tracking to associate the wrong transaction hash with the current send operation.
packages/controllers/src/controllers/SendController.ts#L208-L228
appkit/packages/controllers/src/controllers/SendController.ts
Lines 208 to 228 in 24da3af
| if (SendController.state.token?.address) { | |
| const { hash } = await SendController.sendERC20Token({ | |
| receiverAddress: SendController.state.receiverAddress, | |
| tokenAddress: SendController.state.token.address, | |
| sendTokenAmount: SendController.state.sendTokenAmount, | |
| decimals: SendController.state.token.quantity.decimals | |
| }) | |
| if (hash) { | |
| state.hash = hash | |
| } | |
| } else { | |
| const { hash } = await SendController.sendNativeToken({ | |
| receiverAddress: SendController.state.receiverAddress, | |
| sendTokenAmount: SendController.state.sendTokenAmount, | |
| decimals: SendController.state.token.quantity.decimals | |
| }) | |
| if (hash) { | |
| state.hash = hash | |
| } |
Description
Type of change
Associated Issues
For Linear issues: Closes APKT-4271
Checklist
Note
Centralizes SEND_* event emission in SendController.sendToken (initiated/success/rejected/error), removes event logic from submethods and UI, and updates tests accordingly.
packages/controllers):sendToken()emittingSEND_INITIATED,SEND_SUCCESS, andSEND_REJECTED/SEND_ERRORwith standardized properties (isSmartAccount,token,amount,network,hash).sendEvmToken(),sendNativeToken(),sendERC20Token(), andsendSolanaToken(); simplifies control flow and keepshashassignment and balance updates.Eventtype usage and consolidates error handling (including user-rejection detection viaErrorUtil).fetchTokenBalance()withonErrorcallback andSnackController.showError('Token Balance Unavailable').packages/scaffold-ui):w3m-wallet-send-preview-view: removesEventsControllerusage; relies onSendController.sendToken()for events; shows user-facing errors viaSnackControllerand success viaSnackController/navigation.SEND_INITIATED→SEND_SUCCESS/SEND_REJECTED/SEND_ERROR) and UI error display behavior.@reown/appkit-controllersand@reown/appkit-scaffold-uinoting improved send flow events handling.Written by Cursor Bugbot for commit 24da3af. This will update automatically on new commits. Configure here.