Drop memoize-state, replace with a shallow equal selector

Fix some selectors not getting updated unless I set the pure: true option, which has performance implications.
herrmannjob · Feb 1, 2020 · b0c25cb · b0c25cb
1 parent 7611320
commit b0c25cb
Show file tree

Hide file tree

Showing 8 changed files with 125 additions and 182 deletions.
diff --git a/packages/components/package.json b/packages/components/package.json
@@ -24,7 +24,6 @@
     "hoist-non-react-statics": "3.3.0",
     "immer": "4.0.1",
     "lodash": "4.17.15",
-    "memoize-state": "2.0.13",
     "moment": "2.24.0",
     "polished": "3.4.1",
     "qs": "6.9.1",

diff --git a/packages/components/src/components/App.tsx b/packages/components/src/components/App.tsx
@@ -1,4 +1,3 @@
-import { setAutoFreeze } from 'immer'
 import React, { Fragment } from 'react'
 
 import '../libs/analytics'
@@ -13,10 +12,6 @@ import { UnreadCountProvider } from './context/UnreadCountContext'
 
 enableNetworkInterceptors()
 
-// workaround to memoize-state/proxyequal proxy bug
-// @see https://github.com/theKashey/proxyequal/issues/25
-setAutoFreeze(false)
-
 // TODO: Enable StrictMode after react-native fixes it
 // @see https://github.com/facebook/react-native/issues/22186
 const StrictModePlaceholder = Fragment

diff --git a/packages/components/src/components/cards/BaseCard.shared.ts b/packages/components/src/components/cards/BaseCard.shared.ts
@@ -52,7 +52,7 @@ import { PixelRatio } from 'react-native'
 
 import { Platform } from '../../libs/platform'
 import * as actions from '../../redux/actions'
-import { memoizeMultipleArguments } from '../../redux/selectors/helpers'
+import { betterMemoize } from '../../redux/selectors/helpers'
 import { ExtractActionFromActionCreator } from '../../redux/types/base'
 import {
   avatarSize,
@@ -1150,7 +1150,7 @@ function _getCardPropsForItem(
   }
 }
 
-const _memoizedGetCardPropsForItemFnByColumnId = memoizeMultipleArguments(
+const _memoizedGetCardPropsForItemFnByColumnId = betterMemoize(
   (_columnId: string) => (
     type: ColumnSubscription['type'],
     item: EnhancedItem,
@@ -1167,7 +1167,7 @@ const _memoizedGetCardPropsForItemFnByColumnId = memoizeMultipleArguments(
   10,
 )
 
-const _memoizedGetCardPropsForItem = memoizeMultipleArguments(
+const _memoizedGetCardPropsForItem = betterMemoize(
   (
     type: ColumnSubscription['type'],
     columnId: string,

diff --git a/packages/components/src/redux/selectors/columns.ts b/packages/components/src/redux/selectors/columns.ts
@@ -9,9 +9,10 @@ import { EMPTY_ARRAY, EMPTY_OBJ } from '../../utils/constants'
 import { RootState } from '../types'
 import { createItemsBySubscriptionIdsSelector } from './data'
 import { currentGitHubUsernameSelector } from './github'
-import { createImmerSelector } from './helpers'
+import { betterMemoize, createShallowEqualSelector } from './helpers'
 import {
   createSubscriptionsDataSelector,
+  subscriptionsByIdSelector,
   subscriptionSelector,
 } from './subscriptions'
 
@@ -30,27 +31,32 @@ export const columnIdsSelector = (state: RootState) =>
 export const columnCountSelector = (state: RootState) =>
   columnIdsSelector(state).length
 
-export const columnsArrSelector = createImmerSelector((state: RootState) => {
-  const byId = s(state).byId
-  const columnIds = columnIdsSelector(state)
-
-  if (!(byId && columnIds)) return EMPTY_ARRAY
-
-  return columnIds.map(columnId => byId[columnId]).filter(Boolean) as Column[]
-})
+export const columnsArrSelector = createShallowEqualSelector(
+  (state: RootState) => s(state).byId,
+  (state: RootState) => columnIdsSelector(state),
+  (byId, columnIds) => {
+    if (!(byId && columnIds)) return EMPTY_ARRAY
+    return columnIds.map(columnId => byId[columnId]).filter(Boolean) as Column[]
+  },
+)
 
 export const hasCreatedColumnSelector = (state: RootState) =>
   s(state).byId !== null
 
 export const createColumnSubscriptionsSelector = () =>
-  createImmerSelector((state: RootState, columnId: string) => {
-    const column = columnSelector(state, columnId)
-    const subscriptionIds = (column && column.subscriptionIds) || EMPTY_ARRAY
-
-    return subscriptionIds
-      .map(subscriptionId => subscriptionSelector(state, subscriptionId))
-      .filter(Boolean) as ColumnSubscription[]
-  })
+  createShallowEqualSelector(
+    (state: RootState, columnId: string) => {
+      const column = columnSelector(state, columnId)
+      const subscriptionIds = (column && column.subscriptionIds) || EMPTY_ARRAY
+      return subscriptionIds
+    },
+    (state: RootState) => subscriptionsByIdSelector(state),
+    (subscriptionIds, subscriptionsById) => {
+      return subscriptionIds
+        .map(subscriptionId => subscriptionsById[subscriptionId])
+        .filter(Boolean) as ColumnSubscription[]
+    },
+  )
 
 export const createColumnSubscriptionSelector = () => {
   const columnSubscriptionsSelector = createColumnSubscriptionsSelector()
@@ -67,25 +73,23 @@ export const createColumnSubscriptionSelector = () => {
 
 export const createColumnHeaderDetailsSelector = () => {
   const columnSubscriptionsSelector = createColumnSubscriptionsSelector()
-  const getColumnHeaderDetailsMemoized = createImmerSelector(
-    getColumnHeaderDetails,
+  const getColumnHeaderDetailsMemoized = betterMemoize(getColumnHeaderDetails)
+
+  return createShallowEqualSelector(
+    (state: RootState, columnId: string) => columnSelector(state, columnId),
+    (state: RootState, columnId: string) =>
+      columnSubscriptionsSelector(state, columnId),
+    (state: RootState, _columnId: string) =>
+      currentGitHubUsernameSelector(state),
+    (column, subscriptions, loggedUsername) => {
+      return getColumnHeaderDetailsMemoized(
+        column,
+        subscriptions,
+        { loggedUsername },
+        PixelRatio.getPixelSizeForLayoutSize,
+      )
+    },
   )
-
-  return createImmerSelector((state: RootState, columnId: string) => {
-    const subscriptions = columnSubscriptionsSelector(state, columnId)
-
-    const column = columnSelector(state, columnId)
-    if (!column) return undefined
-
-    const loggedUsername = currentGitHubUsernameSelector(state)
-
-    return getColumnHeaderDetailsMemoized(
-      column,
-      subscriptions,
-      { loggedUsername },
-      PixelRatio.getPixelSizeForLayoutSize,
-    )
-  })
 }
 
 export const createColumnDataSelector = () => {

diff --git a/packages/components/src/redux/selectors/data.ts b/packages/components/src/redux/selectors/data.ts
@@ -2,7 +2,7 @@ import { EnhancedItem } from '@devhub/core'
 
 import { EMPTY_ARRAY, EMPTY_OBJ } from '../../utils/constants'
 import { RootState } from '../types'
-import { createImmerSelector } from './helpers'
+import { createShallowEqualSelector } from './helpers'
 
 const s = (state: RootState) => state.data || EMPTY_OBJ
 
@@ -16,33 +16,36 @@ export const dataReadIds = (state: RootState) => s(state).readIds
 export const dataSavedIds = (state: RootState) => s(state).savedIds
 
 export const createItemsBySubscriptionIdsSelector = () => {
-  return createImmerSelector((state: RootState, subscriptionIds: string[]) => {
-    const idsBySubscriptionId = dataNodeIdsOrIdsBySubscriptionId(state)
-    const entryById = dataByNodeIdOrId(state)
-
-    if (!idsBySubscriptionId) return EMPTY_ARRAY
-    if (!(subscriptionIds && subscriptionIds.length)) return EMPTY_ARRAY
-
-    const itemIds: string[] = []
-    const items: EnhancedItem[] = []
-    subscriptionIds.forEach(subscriptionId => {
-      const ids = idsBySubscriptionId[subscriptionId]
-      if (!(ids && ids.length)) return
-
-      ids.forEach(id => {
-        if (!id) return
-        if (itemIds.includes(id)) return
-
-        const entry = entryById[id]
-        if (!(entry && entry.item)) return
-
-        itemIds.push(id)
-        items.push(entry.item)
+  return createShallowEqualSelector(
+    (_state: RootState, subscriptionIds: string[]) => subscriptionIds,
+    (state: RootState, _subscriptionIds: string[]) =>
+      dataNodeIdsOrIdsBySubscriptionId(state),
+    (state: RootState, _subscriptionIds: string[]) => dataByNodeIdOrId(state),
+    (subscriptionIds, idsBySubscriptionId, entryById) => {
+      if (!idsBySubscriptionId) return EMPTY_ARRAY
+      if (!(subscriptionIds && subscriptionIds.length)) return EMPTY_ARRAY
+
+      const itemIds: string[] = []
+      const items: EnhancedItem[] = []
+      subscriptionIds.forEach(subscriptionId => {
+        const ids = idsBySubscriptionId[subscriptionId]
+        if (!(ids && ids.length)) return
+
+        ids.forEach(id => {
+          if (!id) return
+          if (itemIds.includes(id)) return
+
+          const entry = entryById[id]
+          if (!(entry && entry.item)) return
+
+          itemIds.push(id)
+          items.push(entry.item)
+        })
       })
-    })
 
-    if (!(items && items.length)) return EMPTY_ARRAY
+      if (!(items && items.length)) return EMPTY_ARRAY
 
-    return items
-  })
+      return items
+    },
+  )
 }
diff --git a/packages/components/src/redux/selectors/helpers/index.ts b/packages/components/src/redux/selectors/helpers/index.ts
@@ -1,17 +1,14 @@
 import _ from 'lodash'
-import memoizeState from 'memoize-state'
+import { shallowEqual } from 'react-redux'
 import { createSelectorCreator } from 'reselect'
 
-export function shallowEqualityCheckOrDeepIfArray(a: unknown, b: unknown) {
-  return a === b || (Array.isArray(a) && _.isEqual(a, b))
-}
-
-const refIsEqual = (a: any, b: any) => a === b
-
+// Better Memoize supports a custom equality check function and a custom cache size.
+// Is also re-runs the equality checks in the final result
+// to avoid changing the result reference when possible.
 // tslint:disable-next-line ban-types
-export function memoizeMultipleArguments<F extends Function>(
+export function betterMemoize<F extends Function>(
   func: F,
-  equalityCheck = refIsEqual,
+  equalityCheck = shallowEqual,
   cacheSize = 1,
 ): F {
   let cacheArr: Array<{ args: unknown[]; result: unknown }> = []
@@ -64,17 +61,4 @@ export function memoizeMultipleArguments<F extends Function>(
   }) as any) as F
 }
 
-export const createDeepEqualSelector = createSelectorCreator(
-  memoizeMultipleArguments,
-  shallowEqualityCheckOrDeepIfArray,
-)
-
-export function createImmerSelector<T>(fn: T) {
-  return memoizeState(fn, {
-    equalCheck: true,
-    nestedEquality: true,
-    safe: true,
-    shallowCheck: true,
-    strictArity: false,
-  })
-}
+export const createShallowEqualSelector = createSelectorCreator(betterMemoize)
diff --git a/packages/components/src/redux/selectors/subscriptions.ts b/packages/components/src/redux/selectors/subscriptions.ts
@@ -10,21 +10,23 @@ import { EMPTY_ARRAY, EMPTY_OBJ } from '../../utils/constants'
 import { RootState } from '../types'
 import { currentUserPlanSelector } from './auth'
 import { columnsArrSelector } from './columns'
-import { createImmerSelector } from './helpers'
+import { betterMemoize, createShallowEqualSelector } from './helpers'
 
 const s = (state: RootState) => state.subscriptions || EMPTY_OBJ
 
 export const subscriptionIdsSelector = (state: RootState) =>
   s(state).allIds || EMPTY_ARRAY
 
-export const subscriptionSelector = (state: RootState, id: string) =>
-  (s(state).byId && s(state).byId[id]) || undefined
+export const subscriptionsByIdSelector = (state: RootState) =>
+  s(state).byId || EMPTY_OBJ
 
-export const allSubscriptionsArrSelector = createImmerSelector(
-  (state: RootState) => {
-    const subscriptionIds = subscriptionIdsSelector(state)
-    const byId = s(state).byId
+export const subscriptionSelector = (state: RootState, id: string) =>
+  subscriptionsByIdSelector(state)[id] || undefined
 
+export const allSubscriptionsArrSelector = createShallowEqualSelector(
+  (state: RootState) => s(state).byId,
+  (state: RootState) => subscriptionIdsSelector(state),
+  (byId, subscriptionIds) => {
     if (!(byId && subscriptionIds && subscriptionIds.length)) return EMPTY_ARRAY
 
     return subscriptionIds
@@ -33,12 +35,11 @@ export const allSubscriptionsArrSelector = createImmerSelector(
   },
 )
 
-export const userSubscriptionsArrSelector = createImmerSelector(
-  (state: RootState): ColumnSubscription[] => {
-    const plan = currentUserPlanSelector(state)
-    const columns = columnsArrSelector(state)
-    const byId = s(state).byId
-
+export const userSubscriptionsArrSelector = createShallowEqualSelector(
+  (state: RootState) => currentUserPlanSelector(state),
+  (state: RootState) => columnsArrSelector(state),
+  (state: RootState) => subscriptionsByIdSelector(state),
+  (plan, columns, byId): ColumnSubscription[] => {
     if (!(plan && columns && columns.length && byId)) return EMPTY_ARRAY
 
     const limit =
@@ -58,36 +59,44 @@ export const userSubscriptionsArrSelector = createImmerSelector(
 )
 
 export const createSubscriptionsSelector = () =>
-  createImmerSelector((state: RootState, subscriptionIds: string[]) => {
-    return subscriptionIds
-      .map(id => subscriptionSelector(state, id))
-      .filter(Boolean) as ColumnSubscription[]
-  })
+  createShallowEqualSelector(
+    (_state: RootState, subscriptionIds: string[]) =>
+      subscriptionIds || EMPTY_ARRAY,
+    (state: RootState, _subscriptionIds: string[]) =>
+      subscriptionsByIdSelector(state),
+    (subscriptionIds, byId) => {
+      return subscriptionIds
+        .map(id => byId[id])
+        .filter(Boolean) as ColumnSubscription[]
+    },
+  )
 
 export const createSubscriptionsDataSelector = () => {
   const subscriptionsSelector = createSubscriptionsSelector()
-  const memoizedGetItemsFromSubscriptions = createImmerSelector(
+  const memoizedGetItemsFromSubscriptions = betterMemoize(
     getItemsFromSubscriptions,
   )
 
-  return createImmerSelector((state: RootState, subscriptionIds: string[]) => {
-    const subscriptions = subscriptionsSelector(state, subscriptionIds)
-    const dataByNodeIdOrId = state.data.byId
-
-    const getItemByNodeIdOrId = (nodeIdOrId: string) =>
-      dataByNodeIdOrId &&
-      dataByNodeIdOrId[nodeIdOrId] &&
-      dataByNodeIdOrId[nodeIdOrId]!.item
-
-    const result = memoizedGetItemsFromSubscriptions(
-      subscriptions,
-      getItemByNodeIdOrId,
-    )
-
-    if (!(result && result.length)) return EMPTY_ARRAY
-
-    return result
-  })
+  return createShallowEqualSelector(
+    (state: RootState, subscriptionIds: string[]) =>
+      subscriptionsSelector(state, subscriptionIds),
+    (state: RootState, _subscriptionIds: string[]) => state.data.byId,
+    (subscriptions, dataByNodeIdOrId) => {
+      const getItemByNodeIdOrId = (nodeIdOrId: string) =>
+        dataByNodeIdOrId &&
+        dataByNodeIdOrId[nodeIdOrId] &&
+        dataByNodeIdOrId[nodeIdOrId]!.item
+
+      const result = memoizedGetItemsFromSubscriptions(
+        subscriptions,
+        getItemByNodeIdOrId,
+      )
+
+      if (!(result && result.length)) return EMPTY_ARRAY
+
+      return result
+    },
+  )
 }
 
 export const subscriptionLastFetchedAtSelector = createSelector(