fix: Refetch owned safes after deployment #4673
Conversation
Branch preview✅ Deploy successful! Storybook: |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -59,7 +69,7 @@ const usePendingSafeNotifications = (): void => { | |||
| return () => { | |||
| unsubFns.forEach((unsub) => unsub()) | |||
| } | |||
| }, [dispatch, safeAddress, chain]) | |||
| }, [dispatch, safeAddress, chain, refetch]) | |||
| } | |||
|
|
|||
| export default usePendingSafeNotifications | |||
There was a problem hiding this comment.
-
Unnecessarily destructuring
addresswith a default value of an empty string inuseWallet(). EnsureuseWallet()provides a consistent object structure. -
Dependencies for
useEffectrevised correctly, but consider removing the inlineifcondition for clarity:const skipTokenCondition = address === ''. -
Consider moving the logic from
useEffectoutside to maintain its primary role of side-effect management and prevent excess logic within the hook.
| const { address = '' } = useWallet() || {} | ||
| const { refetch } = useGetAllOwnedSafesQuery(address === '' ? skipToken : { walletAddress: address }) |
There was a problem hiding this comment.
Its probably cleaner to create a new listener middleware for this. Wdyt @katspaugh @compojoom
There was a problem hiding this comment.
Although it does require access to the connected wallet which I am not sure is possible inside a middleware. We would have to move the connected wallet state into Redux as well.
📦 Next.js Bundle Analysis for safe-wallet-webThis analysis was generated by the Next.js Bundle Analysis action. 🤖 🎉 Global Bundle Size Decreased
DetailsThe global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster. Any third party scripts you have added directly to your app using the If you want further insight into what is behind the changes, give @next/bundle-analyzer a try! |
Coverage report
Test suite run success1707 tests passing in 231 suites. Report generated by 🧪jest coverage report action from 7441dcc |
What it solves
Fixes issue found in the release
How this PR fixes it
refetchon the RTK query for all owned safes when receiving aSafeCreationEvent.INDEXEDHow to test it
Checklist