Feat: Persistent selected wallet account context provider#1154
Conversation
🦋 Changeset detectedLatest commit: 7745d9f The changes in this PR will be included in the next version bump. This PR includes changesets to release 43 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 |
|
Hi @mcintyre94 I have done the selected wallet context provider. And I am trying to unit test for that but I find the existing tests are being written using react-test-renderer which is deprecated. Should I write the test for this using react-test-renderer or react-testing-library? I think its better to start writing in react-testing-library as its gonna be migrated to this at some point. |
|
Thanks @ningthoujamSwamikumar, I'll try to review this soon. In terms of testing I'm happy to use BTW a useful validation check for this would be using it in the react app :) |
BundleMonFiles updated (3)
Unchanged files (133)
Total files change +2.28KB +0.59% Final result: ✅ View report in BundleMon website ➡️ |
|
@mcintyre94 Hi. Can you please review this PR. I have ensured all test pass except for node unit test because of pattern matching issue in test-config/jest-unit.config.node.ts . the ignore path pattern (above permalink) doesn't match with tsx file. Similar issue in test-config/jest-unit.config.browser.ts . We need to update the ignore path patterns to match '.tsx' files as well. It is matching only '.ts' files now. And I think since we now have two types of environment to run test on namely browser and node, we need to name the test files appropriately to match ignore path pattern in these test configs. i.e. A test file name should match the pattern '-test.browser.tsx?$', or '-test.node.tsx?$' respectively for browser, and node tests. This way browser test will ignore node files, and node test will ignore browser test files. Hence we won't get test failures due to wrong environment which I am current having while running test for node. Because the filename ending with browser.tsx does not match the ignore path pattern of node test. But the file name should have extensin tsx as it has jsx syntax. Let me know of any feedbacks and comments on my thoughts and the changes made in this PR. |
mcintyre94
left a comment
There was a problem hiding this comment.
Thankyou, this is looking really good! Great tests too :)
On the Node test issue, I agree with you that we should change that pattern for Node. '-test.browser.tsx?$' works for me locally in the jest-unit.config.node.ts file and makes sense. Feel free to add that to this PR or I can. Don't worry about the browser equivalent, we're unlikely to use JSX in a test that can't run in the browser.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
I have updated Thanks for all the feedback. |
mcintyre94
left a comment
There was a problem hiding this comment.
This is looking great now! I've tested locally and it works well in the example app. Thankyou for all your work on this!
I'm going to make a few small changes to the docs and then will get this merged :)
…d SelectedWalletAccountContextProvider
7d27c55 to
c72df49
Compare
e67a257 to
e3cf0eb
Compare
e3cf0eb to
a6b5425
Compare
Your organization requires reapproval when changes are made, so Graphite has dismissed approvals. See the output of git range-diff at https://github.com/anza-xyz/kit/actions/runs/20824418634
|
🔎💬 Inkeep AI search and chat service is syncing content for source 'Solana Kit Docs' |
|
Because there has been no activity on this PR for 14 days since it was merged, it has been automatically locked. Please open a new issue if it requires a follow up. |
Problem
The biggest piece of complexity in the example react-app is about persisting the selected wallet. And while wallet-standard allows an app to be connected to multiple wallets/accounts per wallet, in practice most apps will want to have some context about the currently active account, ie the UiWalletAccount to pass to hooks like useSignAndSendTransaction.
Summary of Changes
Fixes #1148