Skip to content

Commit

Permalink
fix(redux): Actually fix memoization on parameterized selectors (#4305)
Browse files Browse the repository at this point in the history
### 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.
  • Loading branch information
jophish authored Oct 12, 2023
1 parent 2c4c767 commit f6a4fb7
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 0 deletions.
6 changes: 6 additions & 0 deletions src/tokens/selectors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
})

Expand Down
18 changes: 18 additions & 0 deletions src/tokens/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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
},
}
)

Expand Down

0 comments on commit f6a4fb7

Please sign in to comment.