Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is getMappedItems an anti-pattern in mapStateToProps? #1948

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

Is getMappedItems an anti-pattern in mapStateToProps? #1948

pgilad opened this issue Sep 13, 2016 · 2 comments

Comments

@pgilad
Copy link

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
Copy link

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
Copy link
Author

pgilad commented Sep 13, 2016

Great answer! thanks for the help!

@pgilad pgilad closed this as completed Sep 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants