From 07750bfb4ca6e9975204c5ae9b9ccdf39cf35bcf Mon Sep 17 00:00:00 2001 From: infinite-persistence Date: Mon, 8 Nov 2021 19:01:52 +0800 Subject: [PATCH] Fix memo: selectMyChannelClaims, selectActiveChannelClaim ## Issue These should never recalculate after `channel_list` has been fetched, but they do because of poor selector dependency. ## Change With the `byId` changes from the previous commit, we are now able to memoize these selectors correctly. --- ui/redux/selectors/app.js | 10 ++++------ ui/redux/selectors/claims.js | 22 +++++++++++++++++----- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/ui/redux/selectors/app.js b/ui/redux/selectors/app.js index 923556e72..37f6786ba 100644 --- a/ui/redux/selectors/app.js +++ b/ui/redux/selectors/app.js @@ -1,5 +1,5 @@ import { createSelector } from 'reselect'; -import { selectClaimsById, selectMyChannelClaims, makeSelectStakedLevelForChannelUri } from 'redux/selectors/claims'; +import { selectClaimWithId, selectMyChannelClaims, makeSelectStakedLevelForChannelUri } from 'redux/selectors/claims'; export const selectState = (state) => state.app || {}; @@ -70,15 +70,13 @@ export const selectSplashAnimationEnabled = (state) => selectState(state).splash export const selectActiveChannelId = (state) => selectState(state).activeChannel; export const selectActiveChannelClaim = createSelector( - selectActiveChannelId, - selectClaimsById, + (state) => selectClaimWithId(state, selectActiveChannelId(state)), // i.e. 'byId[activeChannelId]' specifically, instead of just 'byId'. selectMyChannelClaims, - (activeChannelClaimId, claimsById, myChannelClaims) => { - if (!activeChannelClaimId || !claimsById || !myChannelClaims || !myChannelClaims.length) { + (activeChannelClaim, myChannelClaims) => { + if (!activeChannelClaim || !myChannelClaims || !myChannelClaims.length) { return undefined; } - const activeChannelClaim = claimsById[activeChannelClaimId]; if (activeChannelClaim) { return activeChannelClaim; } diff --git a/ui/redux/selectors/claims.js b/ui/redux/selectors/claims.js index 3c2a0e427..1ee07dd04 100644 --- a/ui/redux/selectors/claims.js +++ b/ui/redux/selectors/claims.js @@ -419,14 +419,26 @@ export const selectFetchingMyCollections = (state: State) => selectState(state). export const selectMyChannelClaimIds = (state: State) => selectState(state).myChannelClaims; -export const selectMyChannelClaims = createSelector(selectState, selectClaimsById, (state, byId) => { - const ids = state.myChannelClaims; - if (!ids) { - return ids; +export const selectMyChannelClaims = createSelector(selectMyChannelClaimIds, (myChannelClaimIds) => { + if (!myChannelClaimIds) { + return myChannelClaimIds; } + if (!window || !window.store) { + return undefined; + } + + // Note: Grabbing the store and running the selector this way is anti-pattern, + // but it is _needed_ and works only because we know for sure that 'byId[]' + // will be populated with the same claims as when 'myChannelClaimIds' is populated. + // If we put 'state' or 'byId' as the input selector, it essentially + // recalculates every time. Putting 'state' as input to createSelector() is + // always wrong from a memoization standpoint. + const state = window.store.getState(); + const byId = selectClaimsById(state); + const claims = []; - ids.forEach((id) => { + myChannelClaimIds.forEach((id) => { if (byId[id]) { // I'm not sure why this check is necessary, but it ought to be a quick fix for https://github.com/lbryio/lbry-desktop/issues/544 claims.push(byId[id]);