Is getMappedItems an anti-pattern in mapStateToProps? #1948

Closed
pgilad opened this Issue Sep 13, 2016 · 2 comments

Comments

2 participants
@pgilad

pgilad commented Sep 13, 2016

In all real-world examples (including code we use in our own work) I see the following pattern emerging in mapStateToProps:

// action
export const getOrganizations = state => state.entities.organizations;

export const getMappedOrganizations = (state, ids) => {
    const { byId } = getOrganizations(state);
    return ids.map(id => byId[id]);
};

// view
const mapStateToProps = state => {
    const { byAll } = getOrganizations(state);
    const organizations = getMappedOrganizations(state, byAll.ids);

    return {
        isFetching: byAll.isFetching,
        lastUpdated: byAll.lastUpdated,
        organizations,
    };
};

This provides the view all the data it needs, but unfortunately organizations will always be a new array when store updates, resulting in redundant updates of the view even though nothing has changed.

This is especially noticed when you have a popup or modal that updates store as well, and the list in the background, resulting in slowness because the view is always updating.

Is this really the advised pattern? Should you modify shouldComponentUpdate to fit it for non-shallow comparison in this case? Perhaps you should pass down the functions to create the mapped items and do so in the render function instead?

@nighca

This comment has been minimized.

Show comment
Hide comment
@nighca

nighca Sep 13, 2016

In this case, you can use reselect to create your selectors, so that their result can be memoized.
For example:

import { createSelector } from 'reselect'

const organizationsSelector = state => state.entities.organizations;

const allOrganazationIdsSelector = createSelector(
  organizationsSelector,
  organizations => organizations.byAll.ids
)

// with the same `organizations` & `allOrganazationIds`,
// the selector always returns the same `allOrganazations`
const allOrganazationsSelector = createSelector(
  organizationsSelector,
  allOrganazationIdsSelector,
  ({ byId }, ids) => ids.map(id => byId[id])
)

const mapStateToProps = state => {
    const { byAll } = organizationsSelector(state);
    const organizations = allOrganazationsSelector(state);

    return {
        isFetching: byAll.isFetching,
        lastUpdated: byAll.lastUpdated,
        organizations,
    };
};

nighca commented Sep 13, 2016

In this case, you can use reselect to create your selectors, so that their result can be memoized.
For example:

import { createSelector } from 'reselect'

const organizationsSelector = state => state.entities.organizations;

const allOrganazationIdsSelector = createSelector(
  organizationsSelector,
  organizations => organizations.byAll.ids
)

// with the same `organizations` & `allOrganazationIds`,
// the selector always returns the same `allOrganazations`
const allOrganazationsSelector = createSelector(
  organizationsSelector,
  allOrganazationIdsSelector,
  ({ byId }, ids) => ids.map(id => byId[id])
)

const mapStateToProps = state => {
    const { byAll } = organizationsSelector(state);
    const organizations = allOrganazationsSelector(state);

    return {
        isFetching: byAll.isFetching,
        lastUpdated: byAll.lastUpdated,
        organizations,
    };
};
@pgilad

This comment has been minimized.

Show comment
Hide comment
@pgilad

pgilad Sep 13, 2016

Great answer! thanks for the help!

pgilad commented Sep 13, 2016

Great answer! thanks for the help!

@pgilad pgilad closed this Sep 13, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment