feat: allow adding networks without invalidating local-storage #5134
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
alfetopito
reviewed
Dec 5, 2024
Comment on lines
+6
to
+7
| export function useUnsupportedTokens(): UnsupportedTokensState { | ||
| return useAtomValue(currentUnsupportedTokensAtom) || {} |
There was a problem hiding this comment.
Given the recent issue returning a new array on every render, shouldn't this also have a stable empty value declared outside of the hook?
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Make CoW Swap more resilient for run-time errors when we introduce new networks.
Motivation
Every addition of a new chain makes us increase the version all the local storages that have some state per network.
This is because a lot of state is model as
Record<SupportedChainId, MyState>>, becauseSupportedChainIdtype is changes with the addition of a network, we need to version the local storage key.This makes all users to loose their state or settings.
What is worse is that, updating the version is something super easy to forget to do, and if you forget to do it for one of the states it will easily lead to a hard crash of the app that is:
The error comes from assuming the type of the deserialised state is the one defined in the app, which contains a state for a newly added chain, however because the object was serialised in the past, the state for that chain is undefined.
Typescript will be unable to detect at compile time if you forgot to handle the undefined case. This will lead to run-time errors.
Solution
Basically assume that the state can be undefined for all states that follow the pattern
Record<SupportedChainId, MyState>.Because this pattern is very common, and we have a lot of persisted states by network, I defined a new helper type which makes the definition of the state simpler.
type PersistentStateByChain<T> = Record<SupportedChainId, T | undefined>This way, we can turn this state:
Record<SupportedChainId, CustomHooksState>>into:
PersistentStateByChain<CustomHooksState>>And it will already assume that some chain might be un-initialised
Test
This PR doesn't change any logic. So everything should work as before.
Its a bit sensitive PR because it touches the state logic of many critical parts of the app, such as balances, token lists, etc
So probably good to test all flows.
If we want to stress test this PR, we could go to the local storage and delete the KEY for one chain we have.
For example:
allTokenListsInfoAtom:v4with{}hooksStoreAtom:v3we can leave only mainnet{"1":{}}