React-redux-firebase: docs(perf): document the correct way reference to state.firestore when creating selectors

Created on 10 Jan 2019  路  5Comments  路  Source: prescottprue/react-redux-firebase

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
For some reason i dont understand the reference to state.firestore is changed with every action.
When using default memoization used by reselect's createSelector, the data is never memoized (the default memoization uses reference equality). This issue causes too many unnecessary updates of any react component that depends on the data from state.firestore.

import { createSelector } from 'reselect';

const selectFirestore = state.firestore;
const selectData = createSelector(
     selectFirestore,
     firestore => firestore.data,
)

// The reference of the data is changing with every action of redux-firestore
const data = selectDate(state);

// This causes any react component using 'data' to re-render even if it is unnecessary
<Component data={data} />

What is the expected behavior?
The reference to the main object shouldn't be changing with every actions so the default memoization using object reference would work.

My hack is to memoize state.firestore using lodash's isEqual. This solution is not optimal due to the complexity of the comparison.

import { createSelectorCreator, defaultMemoize } from 'reselect';
import { isEqual } from 'lodash';

export const selectFirestore = createSelectorCreator(defaultMemoize, isEqual)(
    state => state.firestore,
    firestore => firestore,
);
docs-update

Most helpful comment

I don't think memoizing state.firestore object is necessary, but instead "antipattern". createSelector is used for memoize function with expensive calculation and usually should have an relevant object as it's argument and firestore object is too shallow to be memoized as any change in firestore state will update the state reference.

To sum up about this issue, state.firestore needs to be changed every actions that update it's state. That's how redux works.

For correct example

const netTotal  = createSelector(
  state => _.get(state, 'firestore.data.products'),
  products => _.sumBy(products, 'price')
)

With this reselect will memoize products object instead, and even if there's any update to other query or redux firestore internal state, the memoized products object will stay the same until there's any actual update to the query or documents.

All 5 comments

Thanks for reporting! Performance has been a big focus for the v1.0.0 redux-firestore buildout that is currently going on (installable from the alpha tag). What version are you currently running?

I'm using [email protected] and [email protected], i've just realized i've put the issues into the wrong project, but to be honest i had quite similar problems with the state.firebase object and fixed it the same way :)

I don't think memoizing state.firestore object is necessary, but instead "antipattern". createSelector is used for memoize function with expensive calculation and usually should have an relevant object as it's argument and firestore object is too shallow to be memoized as any change in firestore state will update the state reference.

To sum up about this issue, state.firestore needs to be changed every actions that update it's state. That's how redux works.

For correct example

const netTotal  = createSelector(
  state => _.get(state, 'firestore.data.products'),
  products => _.sumBy(products, 'price')
)

With this reselect will memoize products object instead, and even if there's any update to other query or redux firestore internal state, the memoized products object will stay the same until there's any actual update to the query or documents.

@illuminist I agree - we should add more to the docs about this

Closing since a reselect section with a similar example has been added to both latest and next.

Thanks to everyone for posting

Was this page helpful?
0 / 5 - 0 ratings