[4.1] refactor(tokens): remove excessive types and consts #3196
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
shoom3301
changed the title
…ocol/cowswap into refactor/tokens-lib-wiring-1
…ocol/cowswap into refactor/tokens-lib-wiring-1
| @@ -200,7 +200,7 @@ async function _updateOrders({ | |||
|
|
|||
| if (presigned.length > 0) { | |||
| // Only mark as presigned the orders we were not aware of their new state | |||
| const presignedOrderIds = presigned as OrderID[] | |||
| const presignedOrderIds = presigned as string[] | |||
There was a problem hiding this comment.
Is the casting still needed?
There was a problem hiding this comment.
but I guess is because it can be one of these two types: OrderFulfillmentData | string
Why do we assume is a string? or maybe the question is, why did we do such a strange type?
There was a problem hiding this comment.
Sorry, I don't know, it also looks weird to me
| @@ -119,7 +119,7 @@ class GasFeeApi { | |||
| return response.json() | |||
| } | |||
|
|
|||
| async getGasPrices(chainId: ChainId = DEFAULT_NETWORK_FOR_LISTS): Promise<GasFeeEndpointResponse> { | |||
There was a problem hiding this comment.
nit: maybe is not bad to have a default network (imagine a widget in Gnosis Chain only app)?
There was a problem hiding this comment.
We still have it.
DEFAULT_NETWORK_FOR_LISTS === ChainId.MAINNET
| @@ -200,7 +200,7 @@ async function _updateOrders({ | |||
|
|
|||
| if (presigned.length > 0) { | |||
| // Only mark as presigned the orders we were not aware of their new state | |||
| const presignedOrderIds = presigned as OrderID[] | |||
| const presignedOrderIds = presigned as string[] | |||
There was a problem hiding this comment.
but I guess is because it can be one of these two types: OrderFulfillmentData | string
Why do we assume is a string? or maybe the question is, why did we do such a strange type?
| @@ -199,7 +199,7 @@ function getEthFlowOverridesOnSelect( | |||
| ): Pick<SwapState, 'independentField' | 'typedValue'> { | |||
| if ( | |||
| inputCurrencyId?.toUpperCase() === | |||
| NATIVE_CURRENCY_BUY_TOKEN[state.chainId || ChainId.MAINNET]?.symbol?.toUpperCase() | |||
| NATIVE_CURRENCY_BUY_TOKEN[(state.chainId as ChainId) || ChainId.MAINNET]?.symbol?.toUpperCase() | |||
There was a problem hiding this comment.
shouldn't state.chainId be of type ChainId in the first place? or this is a number to allow users to be in any other network?
and if so, is the casting safe?
There was a problem hiding this comment.
It definitely must. I'm afraid of changing SwapState, it will cause of lots of new changes
…ocol/cowswap into refactor/tokens-lib-wiring-1 # Conflicts: # apps/cowswap-frontend/src/api/gnosisProtocol/index.ts
…ocol/cowswap into refactor/tokens-lib-wiring-1
* refactor(tokens): wire up components to new hooks (#3198) * refactor(tokens): remove Uniswap currency entities usage (#3199) * refactor(tokens): use new tokens UI and logic by default (#3200) * fix(tokens): fix e2e tests for tokens updates (#3193) * refactor(tokens): remove legacy code (#3194) * fix(tokens): fix tokens list loading state (#3201)
Summary
In this PR I removed some excessive types and consts from the legacy token lists module.
DEFAULT_NETWORK_FOR_LISTSby replacing it withSupportedChainId.MAINNETOrderID(it's actually just string), andUnsupportedToken(not relevant anymore)WithChainIdTo Test
Please, test everything in #3201