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
There was a problem hiding this comment.
Changes on this files are only to wrap the returned functions with useCallback
There was a problem hiding this comment.
Changes on this files are mostly to wrap the returned functions with useCallback. One more change is making sure that the connector.change listener is properly cleaned up on unmount
chrstph-dvx
changed the title
| @@ -85,12 +86,12 @@ export function useClaimWithdrawal(): UseClaimWithdrawalResult { | |||
| throw 'Signer is undefined' | |||
| } | |||
| if (tx.assetType === AssetType.ETH) { | |||
| res = await arbTokenBridge.eth.triggerOutbox({ | |||
| res = await ethTriggerOutbox({ | |||
There was a problem hiding this comment.
curious, why don't we simply call eth.triggerOutbox?
yarn.lock
Outdated
| "@ledgerhq/connect-kit-loader@^1.0.1": | ||
| version "1.1.8" | ||
| resolved "https://registry.yarnpkg.com/@ledgerhq/connect-kit-loader/-/connect-kit-loader-1.1.8.tgz#6cc32191660dd9d6e8f89047af09b0f201e30190" | ||
| integrity sha512-mDJsOucVW8m3Lk2fdQst+P74SgiKebvq1iBk4sXLbADQOwhL9bWGaArvO+tW7jPJZwEfSPWBdHcHoYi11XAwZw== | ||
|
|
| l1NetworkChainId: parentChain.id, | ||
| l2NetworkChainId: childChain.id |
There was a problem hiding this comment.
now the Injector has a few unused vars from hooks
| if ( | ||
| state.app.l1NetworkChainId !== newChainId && | ||
| state.app.l2NetworkChainId !== newChainId | ||
| state.app.sourceChainId !== newChainId && | ||
| state.app.destinationChainId !== newChainId | ||
| ) { | ||
| // only reset the selected token if we are not switching between the pair of l1-l2 networks. | ||
| // we dont want to reset the token if we are switching from Goerli to Arbitrum Goerli for example | ||
| // because we are maybe in the process of auto switching the network and triggering deposit or withdraw | ||
| state.app.selectedToken = null | ||
| } |
There was a problem hiding this comment.
i think this condition causes the bug we had with selected token (which is hackily fixed now by resetting the selected token to null whenever we change source/destination chain id in TransferPanelMain)
here with this code snippet, when user's source is Ethereum, and destination is Arb One, and then user changes destination chain to Arb Nova, the source chain id === new chain id, so the selected token remains the same but the token data should have been different
You should read this comment before reviewing it as it will be easier to review