refactor: Remove useArbTokenBridge from store #1479
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| @@ -853,6 +854,7 @@ export function TransferPanelMain({ | |||
| networks.destinationChain, | |||
| isTestnetMode, | |||
| setNetworks, | |||
| actions.app, | |||
There was a problem hiding this comment.
would be better if we just kept this out so we don't introduce unnecessary rerenders
There was a problem hiding this comment.
actions.app is the same reference every time, it doesn't cause rerender
There was a problem hiding this comment.
The diff on this file is actually fairly simple:
- Add shared zustand store for bridgeTokens: 4cf85e2
- Replace l1/l2 params with useNetworks: c02f0e5
- Move functions so we can later wrap them with
useCallbackwithout errors d2e4846...65583ee - Add
useCallbackb87dc6f...207c6f9
There was a problem hiding this comment.
It can be browsed from commit 7dc7d8c
| @@ -100,14 +101,11 @@ export function TransferPanel() { | |||
| useState(false) | |||
|
|
|||
| const { | |||
| app: { | |||
| connectionState, | |||
There was a problem hiding this comment.
i see that connectionState is not used in the whole app, let's nuke it from the code base
There was a problem hiding this comment.
useAppState is now an unused import
There was a problem hiding this comment.
this doesn't seem related to useArbTokenBridge?
There was a problem hiding this comment.
This wasn't memoed and it was causing issues
| transactions: emptyState, | ||
| loading: isLoadingTxsWithoutStatus, | ||
| error, | ||
| failedChainPairs: emptyState, |
There was a problem hiding this comment.
i am not entirely sure if emptyState should be reused here
wouldn't both be using the same reference?
i get that we only want to return an empty array for both here and will not change them but this feels very weird?
There was a problem hiding this comment.
It would, but it should not cause issues as you're not mutating them.
There was a problem hiding this comment.
i don't think this file has anything to do with useArbTokenBridge's refactoring either
There was a problem hiding this comment.
Same here, we needed to add memo
You should read this comment before reviewing it as it will be easier to review