fix(selectors): Fix memoization with parameterized selectors (#4296)

### Description See [this Slack thread](https://valora-app.slack.com/archives/C025V1D6F3J/p1696932462287309) for more context. ### Test plan Unit and manual tested. ### Related issues ### Backwards compatibility Yes. --------- Co-authored-by: Tom McGuire <[email protected]>
valora-inc · Oct 11, 2023 · 913803c · 913803c
1 parent 683e38e
commit 913803c
Show file tree

Hide file tree

Showing 3 changed files with 114 additions and 105 deletions.
diff --git a/src/tokens/hooks.ts b/src/tokens/hooks.ts
@@ -10,11 +10,11 @@ import {
   tokenCompareByUsdBalanceThenByName,
   tokensByAddressSelector,
   tokensByCurrencySelector,
-  tokensByIdSelectorWrapper,
-  tokensListSelectorWrapper,
+  tokensByIdSelector,
+  tokensListSelector,
   tokensListWithAddressSelector,
-  tokensWithUsdValueSelectorWrapper,
-  totalTokenBalanceSelectorWrapper,
+  tokensWithUsdValueSelector,
+  totalTokenBalanceSelector,
 } from 'src/tokens/selectors'
 import { TokenBalance } from 'src/tokens/slice'
 import {
@@ -38,23 +38,23 @@ export function useTokenInfoByAddress(tokenAddress?: string | null) {
 }
 
 export function useTokensWithUsdValue(networkIds: NetworkId[]) {
-  return useSelector(tokensWithUsdValueSelectorWrapper(networkIds))
+  return useSelector((state) => tokensWithUsdValueSelector(state, networkIds))
 }
 
 export function useTotalTokenBalance() {
   const supportedNetworkIds = getSupportedNetworkIdsForTokenBalances()
-  return useSelector(totalTokenBalanceSelectorWrapper(supportedNetworkIds))
+  return useSelector((state) => totalTokenBalanceSelector(state, supportedNetworkIds))
 }
 
 export function useTokensWithTokenBalance() {
   const supportedNetworkIds = getSupportedNetworkIdsForTokenBalances()
-  const tokens = useSelector(tokensListSelectorWrapper(supportedNetworkIds))
+  const tokens = useSelector((state) => tokensListSelector(state, supportedNetworkIds))
   return tokens.filter((tokenInfo) => tokenInfo.balance.gt(TOKEN_MIN_AMOUNT))
 }
 
 export function useTokensForAssetsScreen() {
   const supportedNetworkIds = getSupportedNetworkIdsForTokenBalances()
-  const tokens = useSelector(tokensListSelectorWrapper(supportedNetworkIds))
+  const tokens = useSelector((state) => tokensListSelector(state, supportedNetworkIds))
 
   return tokens
     .filter((tokenInfo) => tokenInfo.balance.gt(TOKEN_MIN_AMOUNT) || tokenInfo.showZeroBalance)
@@ -83,17 +83,17 @@ export function useTokensForAssetsScreen() {
 }
 
 export function useTokensInfoUnavailable(networkIds: NetworkId[]) {
-  const totalBalance = useSelector(totalTokenBalanceSelectorWrapper(networkIds))
+  const totalBalance = useSelector((state) => totalTokenBalanceSelector(state, networkIds))
   return totalBalance === null
 }
 
 export function useTokensList() {
   const networkIds = Object.values(networkConfig.networkToNetworkId)
-  return useSelector(tokensListSelectorWrapper(networkIds))
+  return useSelector((state) => tokensListSelector(state, networkIds))
 }
 
 export function useTokenPricesAreStale(networkIds: NetworkId[]) {
-  const tokens = useSelector(tokensListSelectorWrapper(networkIds))
+  const tokens = useSelector((state) => tokensListSelector(state, networkIds))
   // If no tokens then prices cannot be stale
   if (tokens.length === 0) return false
   // Put tokens with priceUsd into an array
@@ -114,7 +114,7 @@ export function useSendableTokens() {
   const networkIdsForSend = getDynamicConfigParams(
     DynamicConfigs[StatsigDynamicConfigs.MULTI_CHAIN_FEATURES]
   ).showSend
-  const tokens = useSelector(tokensListSelectorWrapper(networkIdsForSend))
+  const tokens = useSelector((state) => tokensListSelector(state, networkIdsForSend))
   return tokens.filter((tokenInfo) => tokenInfo.balance.gt(TOKEN_MIN_AMOUNT))
 }
 
@@ -123,7 +123,7 @@ export function useSwappableTokens() {
   const networkIdsForSwap = getDynamicConfigParams(
     DynamicConfigs[StatsigDynamicConfigs.MULTI_CHAIN_FEATURES]
   ).showSwap
-  const tokens = useSelector(tokensListSelectorWrapper(networkIdsForSwap))
+  const tokens = useSelector((state) => tokensListSelector(state, networkIdsForSwap))
   return tokens
     .filter(
       (tokenInfo) =>
@@ -138,15 +138,15 @@ export function useCashInTokens() {
   const networkIdsForCico = getDynamicConfigParams(
     DynamicConfigs[StatsigDynamicConfigs.MULTI_CHAIN_FEATURES]
   ).showCico
-  const tokens = useSelector(tokensListSelectorWrapper(networkIdsForCico))
+  const tokens = useSelector((state) => tokensListSelector(state, networkIdsForCico))
   return tokens.filter((tokenInfo) => tokenInfo.isCashInEligible && isCicoToken(tokenInfo.symbol))
 }
 
 export function useCashOutTokens() {
   const networkIdsForCico = getDynamicConfigParams(
     DynamicConfigs[StatsigDynamicConfigs.MULTI_CHAIN_FEATURES]
   ).showCico
-  const tokens = useSelector(tokensListSelectorWrapper(networkIdsForCico))
+  const tokens = useSelector((state) => tokensListSelector(state, networkIdsForCico))
   return tokens.filter(
     (tokenInfo) =>
       tokenInfo.balance.gt(TOKEN_MIN_AMOUNT) &&
@@ -157,7 +157,7 @@ export function useCashOutTokens() {
 
 export function useTokenInfo(tokenId?: string): TokenBalance | undefined {
   const networkIds = Object.values(networkConfig.networkToNetworkId)
-  const tokens = useSelector(tokensByIdSelectorWrapper(networkIds))
+  const tokens = useSelector((state) => tokensByIdSelector(state, networkIds))
   return tokenId ? tokens[tokenId] : undefined
 }
 

diff --git a/src/tokens/selectors.test.ts b/src/tokens/selectors.test.ts
@@ -6,10 +6,10 @@ import {
   tokensByAddressSelector,
   tokensByUsdBalanceSelector,
   tokensListWithAddressSelector,
-  tokensWithUsdValueSelectorWrapper,
-  totalTokenBalanceSelectorWrapper,
-  tokensByIdSelectorWrapper,
-  tokensListSelectorWrapper,
+  tokensWithUsdValueSelector,
+  totalTokenBalanceSelector,
+  tokensByIdSelector,
+  tokensListSelector,
 } from 'src/tokens/selectors'
 import { NetworkId } from 'src/transactions/types'
 import { ONE_DAY_IN_MILLIS } from 'src/utils/time'
@@ -127,10 +127,10 @@ const state: any = {
   },
 }
 
-describe(tokensByIdSelectorWrapper, () => {
+describe(tokensByIdSelector, () => {
   describe('when fetching tokens by id', () => {
     it('returns the right tokens', () => {
-      const tokensById = tokensByIdSelectorWrapper([NetworkId['celo-alfajores']])(state)
+      const tokensById = tokensByIdSelector(state, [NetworkId['celo-alfajores']])
       expect(Object.keys(tokensById).length).toEqual(6)
       expect(tokensById['celo-alfajores:0xusd']?.symbol).toEqual('cUSD')
       expect(tokensById['celo-alfajores:0xeur']?.symbol).toEqual('cEUR')
@@ -156,13 +156,13 @@ describe(tokensByAddressSelector, () => {
   })
 })
 
-describe(tokensListSelectorWrapper, () => {
+describe(tokensListSelector, () => {
   describe('when fetching tokens with id as a list', () => {
     it('returns the right tokens', () => {
-      const tokens = tokensListSelectorWrapper([
+      const tokens = tokensListSelector(state, [
         NetworkId['celo-alfajores'],
         NetworkId['ethereum-sepolia'],
-      ])(state)
+      ])
       expect(tokens.length).toEqual(7)
       expect(tokens.find((t) => t.tokenId === 'celo-alfajores:0xusd')?.symbol).toEqual('cUSD')
       expect(tokens.find((t) => t.tokenId === 'celo-alfajores:0xeur')?.symbol).toEqual('cEUR')
@@ -257,9 +257,9 @@ describe('tokensByUsdBalanceSelector', () => {
   })
 })
 
-describe('tokensWithUsdValueSelectorWrapper', () => {
+describe('tokensWithUsdValueSelector', () => {
   it('returns only the tokens that have a USD balance', () => {
-    const tokens = tokensWithUsdValueSelectorWrapper([NetworkId['celo-alfajores']])(state)
+    const tokens = tokensWithUsdValueSelector(state, [NetworkId['celo-alfajores']])
     expect(tokens).toMatchInlineSnapshot(`
       [
         {
@@ -300,25 +300,24 @@ describe(defaultTokenToSendSelector, () => {
   })
 })
 
-describe(totalTokenBalanceSelectorWrapper, () => {
+describe(totalTokenBalanceSelector, () => {
   describe('when fetching the total token balance', () => {
     it('returns the right amount', () => {
-      expect(totalTokenBalanceSelectorWrapper([NetworkId['celo-alfajores']])(state)).toEqual(
+      expect(totalTokenBalanceSelector(state, [NetworkId['celo-alfajores']])).toEqual(
         new BigNumber(107.5)
       )
     })
 
     it('returns null if there was an error fetching and theres no cached info', () => {
-      expect(
-        totalTokenBalanceSelectorWrapper([NetworkId['celo-alfajores']])({
-          ...state,
-          tokens: {
-            tokenBalances: {},
-            error: true,
-            loading: false,
-          },
-        } as any)
-      ).toBeNull()
+      const errorState = {
+        ...state,
+        tokens: {
+          tokenBalances: {},
+          error: true,
+          loading: false,
+        },
+      } as any
+      expect(totalTokenBalanceSelector(errorState, [NetworkId['celo-alfajores']])).toBeNull()
     })
   })
 

diff --git a/src/tokens/selectors.ts b/src/tokens/selectors.ts
@@ -30,37 +30,34 @@ export type CurrencyTokens = {
 export const tokenFetchLoadingSelector = (state: RootState) => state.tokens.loading
 export const tokenFetchErrorSelector = (state: RootState) => state.tokens.error
 
-/**
- * Selector-like functions suffixed with "wrapper" are higher-order functions which return a selector
- * that only looks at tokens from the specified networkIds. These functions should not be called
- * directly from components, but instead from within hooks
- */
-export const tokensByIdSelectorWrapper = (networkIds: NetworkId[]) =>
-  createSelector(
+export const tokensByIdSelector = createSelector(
+  [
     (state: RootState) => state.tokens.tokenBalances,
-    (storedBalances) => {
-      const tokenBalances: TokenBalances = {}
-      for (const storedState of Object.values(storedBalances)) {
-        if (
-          !storedState ||
-          storedState.balance === null ||
-          !networkIds.includes(storedState.networkId)
-        ) {
-          continue
-        }
-        const priceUsd = new BigNumber(storedState.priceUsd ?? NaN)
-        const tokenPriceUsdIsStale =
-          (storedState.priceFetchedAt ?? 0) < Date.now() - TIME_UNTIL_TOKEN_INFO_BECOMES_STALE
-        tokenBalances[storedState.tokenId] = {
-          ...storedState,
-          balance: new BigNumber(storedState.balance),
-          priceUsd: priceUsd.isNaN() || tokenPriceUsdIsStale ? null : priceUsd,
-          lastKnownPriceUsd: !priceUsd.isNaN() ? priceUsd : null,
-        }
+    (_state: RootState, networkIds: NetworkId[]) => networkIds,
+  ],
+  (storedBalances, networkIds) => {
+    const tokenBalances: TokenBalances = {}
+    for (const storedState of Object.values(storedBalances)) {
+      if (
+        !storedState ||
+        storedState.balance === null ||
+        !networkIds.includes(storedState.networkId)
+      ) {
+        continue
+      }
+      const priceUsd = new BigNumber(storedState.priceUsd ?? NaN)
+      const tokenPriceUsdIsStale =
+        (storedState.priceFetchedAt ?? 0) < Date.now() - TIME_UNTIL_TOKEN_INFO_BECOMES_STALE
+      tokenBalances[storedState.tokenId] = {
+        ...storedState,
+        balance: new BigNumber(storedState.balance),
+        priceUsd: priceUsd.isNaN() || tokenPriceUsdIsStale ? null : priceUsd,
+        lastKnownPriceUsd: !priceUsd.isNaN() ? priceUsd : null,
       }
-      return tokenBalances
     }
-  )
+    return tokenBalances
+  }
+)
 
 /**
  * Get an object mapping token addresses to token metadata, the user's balance, and its price
@@ -70,7 +67,7 @@ export const tokensByIdSelectorWrapper = (networkIds: NetworkId[]) =>
  * @deprecated use tokensByIdSelector instead
  */
 export const tokensByAddressSelector = createSelector(
-  tokensByIdSelectorWrapper([networkConfig.defaultNetworkId]),
+  (state: RootState) => tokensByIdSelector(state, [networkConfig.defaultNetworkId]),
   (tokens) => {
     const output: TokenBalancesWithAddress = {}
     for (const token of Object.values(tokens)) {
@@ -86,10 +83,12 @@ export const tokensByAddressSelector = createSelector(
   }
 )
 
-export const tokensListSelectorWrapper = (networkIds: NetworkId[]) =>
-  createSelector(tokensByIdSelectorWrapper(networkIds), (tokens) => {
+export const tokensListSelector = createSelector(
+  (state: RootState, networkIds: NetworkId[]) => tokensByIdSelector(state, networkIds),
+  (tokens) => {
     return Object.values(tokens).map((token) => token!)
-  })
+  }
+)
 
 /**
  * @deprecated use tokensListSelector instead
@@ -269,51 +268,62 @@ export const lastKnownTokenBalancesSelector = createSelector(
   }
 )
 
-export const tokensWithUsdValueSelectorWrapper = (networkIds: NetworkId[]) =>
-  createSelector(tokensListSelectorWrapper(networkIds), (tokens) => {
+export const tokensWithUsdValueSelector = createSelector(
+  (state: RootState, networkIds: NetworkId[]) => tokensListSelector(state, networkIds),
+  (tokens) => {
     return tokens.filter((tokenInfo) =>
       tokenInfo.balance.multipliedBy(tokenInfo.priceUsd ?? 0).gt(STABLE_TRANSACTION_MIN_AMOUNT)
     ) as TokenBalanceWithPriceUsd[]
-  })
-
-export const totalTokenBalanceSelectorWrapper = (networkIds: NetworkId[]) =>
-  createSelector(
-    [
-      tokensListSelectorWrapper(networkIds),
-      tokensWithUsdValueSelectorWrapper(networkIds),
-      usdToLocalCurrencyRateSelector,
-      tokenFetchErrorSelector,
-      tokenFetchLoadingSelector,
-    ],
-    (tokensList, tokensWithUsdValue, usdToLocalRate, tokenFetchError, tokenFetchLoading) => {
-      if (tokenFetchError || tokenFetchLoading) {
-        return null
-      }
+  }
+)
 
-      if (!usdToLocalRate || tokensList.length === 0) {
-        return null
-      }
-      let totalBalance = new BigNumber(0)
+export const totalTokenBalanceSelector = createSelector(
+  [
+    (state: RootState, networkIds: NetworkId[]) => tokensListSelector(state, networkIds),
+    (state: RootState, networkIds: NetworkId[]) => tokensWithUsdValueSelector(state, networkIds),
+    usdToLocalCurrencyRateSelector,
+    tokenFetchErrorSelector,
+    tokenFetchLoadingSelector,
+    (_state: RootState, networkIds: NetworkId[]) => networkIds,
+  ],
+  (
+    tokensList,
+    tokensWithUsdValue,
+    usdToLocalRate,
+    tokenFetchError,
+    tokenFetchLoading,
+    networkIds
+  ) => {
+    if (tokenFetchError || tokenFetchLoading) {
+      return null
+    }
 
-      for (const token of tokensWithUsdValue.filter((token) =>
-        networkIds.includes(token.networkId)
-      )) {
-        const tokenAmount = new BigNumber(token.balance)
-          .multipliedBy(token.priceUsd)
-          .multipliedBy(usdToLocalRate)
-        totalBalance = totalBalance.plus(tokenAmount)
-      }
+    if (!usdToLocalRate || tokensList.length === 0) {
+      return null
+    }
+    let totalBalance = new BigNumber(0)
 
-      return totalBalance
+    for (const token of tokensWithUsdValue.filter((token) =>
+      networkIds.includes(token.networkId)
+    )) {
+      const tokenAmount = new BigNumber(token.balance)
+        .multipliedBy(token.priceUsd)
+        .multipliedBy(usdToLocalRate)
+      totalBalance = totalBalance.plus(tokenAmount)
     }
-  )
 
-export const tokensInfoUnavailableSelectorWrapper = (networkIds: NetworkId[]) =>
-  createSelector(totalTokenBalanceSelectorWrapper(networkIds), (totalBalance) => {
+    return totalBalance
+  }
+)
+
+export const tokensInfoUnavailableSelector = createSelector(
+  (state: RootState, networkIds: NetworkId[]) => totalTokenBalanceSelector(state, networkIds),
+  (totalBalance) => {
     // The total balance is null if there was an error fetching the tokens
     // info and there are no cached values
     return totalBalance === null
-  })
+  }
+)
 
 export const visualizeNFTsEnabledInHomeAssetsPageSelector = (state: RootState) =>
   state.app.visualizeNFTsEnabledInHomeAssetsPage