-
Notifications
You must be signed in to change notification settings - Fork 1.6k
fix: Added missing unsubscribe returns for sub functions in AppKit Core #5376
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
|
|
@glitch-txs is attempting to deploy a commit to the Reown Team on Vercel. A member of the Team first needs to authorize it. |
|
Thank you for your contribution! We ask that you please read and sign our CTA Document before we can accept your contribution. You can sign the CTA simply by posting a Pull Request Comment with the following text: I have read the CTA Document and I hereby sign the CTA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
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 fixes a bug where two subscription methods (subscribeShouldUpdateToAddress and subscribeCaipNetworkChange) were missing return statements for their unsubscribe functions, and refactors subscribeAccount to properly aggregate and return all unsubscribe functions from its multiple controller subscriptions.
- Added
returnstatements tosubscribeShouldUpdateToAddressandsubscribeCaipNetworkChangemethods - Refactored
subscribeAccountto collect and return all unsubscribe functions via a cleanup function - Added comprehensive tests to verify all subscribe methods return unsubscribe functions
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/appkit/src/client/appkit-base-client.ts | Added missing return statements for two subscribe methods and refactored subscribeAccount to properly aggregate unsubscribe functions |
| packages/appkit/tests/client/appkit-core.test.ts | Added comprehensive tests to verify all subscribe methods return unsubscribe functions and that cleanup works correctly |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
can we merge this one too? I'm needing it to handle some stuff programmatically inside a useEffect cc @tomiir @magiziz |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Description
Please include a brief summary of the change.
Type of change
Associated Issues
For Linear issues: Closes APKT-xxx
For GH issues: closes #5377
Showcase (Optional)
If there is a UI change include the screenshots with before and after state.
If new feature is being introduced, include the link to demo recording.
Checklist