From f6a4fb7a6368cfb910dc29db1806729dc13d204a Mon Sep 17 00:00:00 2001 From: Joe Bergeron Date: Thu, 12 Oct 2023 13:22:01 -0400 Subject: [PATCH] fix(redux): Actually fix memoization on parameterized selectors (#4305) ### Description Taking another crack at this -- see [here](https://valora-app.slack.com/archives/C02898GN22V/p1697100246829169?thread_ts=1697040837.299889&cid=C02898GN22V). See [this documentation](https://github.com/reduxjs/reselect/blob/master/README.md#defaultmemoizefunc-equalitycheckoroptions--defaultequalitycheck) for a bit more info. Confusingly, `equalityCheck` is called once for each argument, so we need to use a type guard to determine when to use deep equality (for `networkIds`) versus the default reference equality (for state). ### Test plan Unit and manual tested. ### Related issues N/A ### Backwards compatibility Yes. --- src/tokens/selectors.test.ts | 6 ++++++ src/tokens/selectors.ts | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/tokens/selectors.test.ts b/src/tokens/selectors.test.ts index f790645146b..1d46c67d778 100644 --- a/src/tokens/selectors.test.ts +++ b/src/tokens/selectors.test.ts @@ -139,6 +139,12 @@ describe(tokensByIdSelector, () => { expect(tokensById['celo-alfajores:0x5']?.name).toEqual('0x5 token') expect(tokensById['celo-alfajores:0x6']?.name).toEqual('0x6 token') }) + it('avoids unnecessary recomputation', () => { + const tokensById = tokensByIdSelector(state, [NetworkId['celo-alfajores']]) + const tokensById2 = tokensByIdSelector(state, [NetworkId['celo-alfajores']]) + expect(tokensById).toEqual(tokensById2) + expect(tokensByIdSelector.recomputations()).toEqual(1) + }) }) }) diff --git a/src/tokens/selectors.ts b/src/tokens/selectors.ts index 3ec29fb25ab..9feed3c4622 100644 --- a/src/tokens/selectors.ts +++ b/src/tokens/selectors.ts @@ -19,6 +19,7 @@ import { Currency } from 'src/utils/currencies' import { isVersionBelowMinimum } from 'src/utils/versionCheck' import networkConfig from 'src/web3/networkConfig' import { sortByUsdBalance, sortFirstStableThenCeloThenOthersByUsdBalance } from './utils' +import _ from 'lodash' type TokenBalanceWithPriceUsd = TokenBalance & { priceUsd: BigNumber @@ -27,6 +28,12 @@ export type CurrencyTokens = { [currency in Currency]: TokenBalanceWithAddress | undefined } +function isNetworkIdList(networkIds: any): networkIds is NetworkId[] { + return ( + networkIds.constructor === Array && + networkIds.every((networkId) => Object.values(NetworkId).includes(networkId)) + ) +} export const tokenFetchLoadingSelector = (state: RootState) => state.tokens.loading export const tokenFetchErrorSelector = (state: RootState) => state.tokens.error @@ -56,6 +63,17 @@ export const tokensByIdSelector = createSelector( } } return tokenBalances + }, + { + memoizeOptions: { + equalityCheck: (previousValue, currentValue) => { + if (isNetworkIdList(previousValue) && isNetworkIdList(currentValue)) { + return _.isEqual(previousValue, currentValue) + } + return previousValue === currentValue + }, + maxSize: 10, // This is somewhat arbitrary, but appears to reliably prevent recalculation + }, } )