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

How to use reselect with Immutable.js? #26

Closed
gajus opened this issue Aug 13, 2015 · 37 comments
Closed

How to use reselect with Immutable.js? #26

gajus opened this issue Aug 13, 2015 · 37 comments

Comments

@gajus
Copy link

gajus commented Aug 13, 2015

No description provided.

@gajus
Copy link
Author

gajus commented Aug 13, 2015

import {
    createSelector
} from 'reselect';
import Immutable from 'immutable';

let state = {},
    taskSelector,
    activeTaskSelector;

state.tasks = [
    {
        id: 1,
        name: 'foo',
        done: true
    },
    {
        id: 2,
        name: 'bar',
        done: true
    },
    {
        id: 3,
        name: 'baz',
        done: false
    },
];

state = Immutable.fromJS(state);

taskSelector = state => state.get('tasks');

activeTaskSelector = createSelector(
    [
        taskSelector
    ],
    tasks => {
        return tasks.filter(task => task.get('done'));
    }
);

console.log( activeTaskSelector(state) === activeTaskSelector(state) );

A common pitfall is to use createSelectorCreator(Immutable.is) instead of simple createSelector. See the rest of the discussion.

@gajus gajus closed this as completed Aug 13, 2015
@gajus
Copy link
Author

gajus commented Aug 13, 2015

I figure this is the proper way. If I am wrong, someone please correct it. Otherwise, I have created this issue as a reference point.

@vovacodes
Copy link

@gajus Could you please elaborate why should we use Immutable.is() as an equalsFn?

@gajus
Copy link
Author

gajus commented Aug 17, 2015

@wizardzloy It is explained in the Immutable.js docs, https://facebook.github.io/immutable-js/docs/#/is.

var map1 = Immutable.Map({a:1, b:1, c:1});
var map2 = Immutable.Map({a:1, b:1, c:1});
assert(map1 !== map2);
assert(Object.is(map1, map2) === false);
assert(Immutable.is(map1, map2) === true);

@vovacodes
Copy link

@gajus yes I saw that part of documentation, but I wonder why the simple === is not enough in case of selectors. I mean we suppose to use selector with the same state object I believe. So that if reference changed that means that the underlying data also changed. But Immutable.is() performs deep comparison which might introduce performance penalties. So my question is whether we really need to use it?

@gajus
Copy link
Author

gajus commented Aug 17, 2015

Thats a good point. I will need to do a few tests.

@ellbee
Copy link
Collaborator

ellbee commented Aug 17, 2015

Immutable.js always returns a new collection when certain operations (filter, reduce etc.) are applied to a collection. If you are using one of those operations to update Immutable.js collections in your state, === will indicate that collection has changed every time even if nothing actually changes.

@gajus
Copy link
Author

gajus commented Aug 17, 2015

@ellbee can you give an example in the context of reselect where that is relevant?

@ellbee
Copy link
Collaborator

ellbee commented Aug 17, 2015

If the EqualsFn thinks the arguments are different every single time the selector is called, the memoization isn't going to work. I guess it comes down to whether the cost of the deepEquals is greater than the cost of the computation in your selector.

@ellbee
Copy link
Collaborator

ellbee commented Aug 17, 2015

By the way, I think @wizardzloy is basically right and that === will often be the right thing to use. I just wanted to point out that depending on how you update a collection, an updated reference doesn't necessarily mean that the underlying data has changed.

@bunkat
Copy link

bunkat commented Aug 17, 2015

I think that === will always be the right thing to use even in the case when a selector is using filter or sort.

activeTaskSelector =createSelector(
    [
        taskSelector
    ],
    tasks => {
        return tasks.filter(task => task.get('done'));
    }
);

In this case, the selector will check taskSelector to see if it has changed. Since it is just reading from the store, === will determine if it has changed correctly. If it has changed, the selector will recompute the filter and memoize it. The next time that the selector is called, taskSelector will not have changed and so the memoized version will be returned, the filter will not be recomputed.

fooSelector = createSelector(
    [
        activeTaskSelector
    ],
    tasks => {
        return ...;
    }
);

Since the memoized version was returned, any composite filters down the line will also continue to work correctly. In this case, fooSelector will only be recomputed if activeTaskSelector was recomputed, which will only happen if taskSelector has changed, which is exactly what you want.

@ellbee
Copy link
Collaborator

ellbee commented Aug 18, 2015

@bunkat What if your reducer is updating the store using map, filter, reduce etc.?

@bunkat
Copy link

bunkat commented Aug 18, 2015

If your reducer updates the store (regardless of how it is done) the value will be different (=== will fail) and the selector will be recalculated which I think is what you would expect.

If your reducer updates the store with a strictly equivalent but new Immutable instance such that the selector would recalculate wastefully, I think that is a failure of the reducer (it should have just returned state in that case, not an updated state). I think it makes a lot more sense to do the equality check in the reducer (if that is a concern) rather than force all selectors to use Immutable.is which defeats a lot of the benefit of using immutables in the first place.

I actually can't imagine a scenario where a reducer would systematically be returning a strictly equivalent update as new state. That would mean that changes are happening outside of actions which shouldn't be the case, otherwise you would always know when running the map, filter, reduce would be necessary.

@vovacodes
Copy link

I totally agree with @bunkat. As for me selectors should be as performant and easy to reason about as possible. So I believe that we should let the Immutable.js do its job of figuring out whether data actually changed and avoid adding this complexity into selectors.
Should we somehow describe this in the README or create a gist?

@gajus gajus reopened this Aug 18, 2015
@ellbee
Copy link
Collaborator

ellbee commented Aug 18, 2015

Yes, you can do a deep equality check in the reducer and not update the state if it hasn't changed, or you can check the state in the action to figure out whether you need to update, or if you prefer the mechanism is there so you can do it in the selector. All I wanted to point out in my original comment is that So that if reference changed that means that the underlying data also changed is not necessarily true when using Immutable's map, filter etc. and this can be surprising to some people because other operations (e.g. merge) do only return a new reference when the underlying data changes.

Also, each selector can be given a different equality function, so there is no need to force every selector to use Immutable.is().

@gajus
Copy link
Author

gajus commented Aug 18, 2015

I am still missing a real life example of where Immutable.is would be useful in the context of reselect. Can you please contribute an example in code, @ellbee?

@ellbee
Copy link
Collaborator

ellbee commented Aug 18, 2015

@gajus Maybe, maybe not :-) I have to say that I've not found any need for it in any real life projects yet. I'll have a play around later and see if I can come up with any really compelling use cases.

gajus added a commit to gajus/redux-immutable-examples that referenced this issue Aug 18, 2015
@ellbee
Copy link
Collaborator

ellbee commented Aug 18, 2015

@gajus Here is the sort of situation where Immutable.is() as the equalsFn might possibly be helpful:

// state for reducer a:
Immutable.map({
  'list1': Immutable.List(),
  'list2': Immutable.List(),
  'keyword': ''
});

// selectors
const list1$ = state => state.a.get('list1');
const list2$ = state => state.a.get('list2');
const keyword$ = state => state.a.get('keyword');

const filteredList2$ = createSelector(
  [list2$, keyword$],
  (list2, keyword) => {
    return list2.filter(x => x.indexOf(keyword) !== -1);
  }
);

const createImmutableSelector = createSelectorCreator(Immutable.is);

const somethingExpensive$ = createImmutableSelector(
  [list1$, filteredList2$],
  (list1, filteredList2) => {
    return //expensive computation
  }
);

somethingExpensive$ with === as the equalsFn would re-run the expensive computation every time someone changed the keyword, even if that change did not cause any changes to the underlying data in filteredList$.

I'm not saying that this is the best approach, just that === will break memoization here.

@bunkat

I actually can't imagine a scenario where a reducer would systematically be returning a strictly equivalent update as new state. That would mean that changes are happening outside of actions which shouldn't be the case, otherwise you would always know when running the map, filter, reduce would be necessary.

Something I have done which is kind of like this was a background task, running on a timer, which filtered stale items from a list.

@gajus
Copy link
Author

gajus commented Aug 18, 2015

@ellbee I am still not seeing it.

import Immutable from 'immutable';
import {
    createSelector,
    createSelectorCreator
} from 'reselect';

let state,
    list1Selector,
    list2Selector,
    keywordSelector,
    filteredList2Selector,
    somethingExpensiveSelector,
    createImmutableSelector;

// state for reducer a:
state = Immutable.Map({
    list1: Immutable.List(),
    list2: Immutable.List(),
    keyword: ''
});

list1Selector = state => state.get('list1');
list2Selector = state => state.get('list2');
keywordSelector = state => state.get('keyword');

filteredList2Selector = createSelector(
    [
        list2Selector,
        keywordSelector
    ],
    (list2, keyword) => {
        console.log('Generating filteredList2Selector...');
        return list2
            .filter(x => {
                return x.indexOf(keyword) !== -1;
            });
    }
);

createImmutableSelector = createSelectorCreator(Immutable.is);

somethingExpensiveSelector = createImmutableSelector(
    [
        list1Selector,
        filteredList2Selector
    ],
    (list1, filteredList2) => {
        console.log('Generating somethingExpensiveSelector...');

        return Math.random();
    }
);

console.log('filteredList2Selector(state)', filteredList2Selector(state));
console.log('filteredList2Selector(state)', filteredList2Selector(state));

console.log('filteredList2Selector(state)', somethingExpensiveSelector(state));
console.log('filteredList2Selector(state)', somethingExpensiveSelector(state));

If you run this in node, you will get:

Generating filteredList2Selector...
filteredList2Selector(state) List []
filteredList2Selector(state) List []
Generating somethingExpensiveSelector...
filteredList2Selector(state) 0.29932390339672565
filteredList2Selector(state) 0.29932390339672565

You can run this code using babel-node CLI tool.

@vovacodes
Copy link

@gajus you need to modify keyword property of state in order to see the effect @ellbee described. In this case somethingExpensiveSelector will always be recalculated even if List returned by the filteredList2Selector contains the same items inside. That is because filteredList2Selector always returns new instance of List if recalculated. So just try to add one more line to your example:

console.log('filteredList2Selector(state)', filteredList2Selector(state));
console.log('filteredList2Selector(state)', filteredList2Selector(state));

console.log('filteredList2Selector(state)', somethingExpensiveSelector(state));

state.set('keyword', 'not empty string');

console.log('filteredList2Selector(state)', somethingExpensiveSelector(state));

@gajus
Copy link
Author

gajus commented Aug 18, 2015

@wizardzloy Modify when?

@vovacodes
Copy link

@gajus between 2 somethingExpensiveSelector(state) calls, see the code snippet above

@gajus
Copy link
Author

gajus commented Aug 18, 2015

But this would never happen... at least in redux domain.

@ellbee
Copy link
Collaborator

ellbee commented Aug 18, 2015

This is what I meant:

import Immutable from 'immutable';
import {
    createSelector,
    createSelectorCreator
} from 'reselect';

let state,
    list1Selector,
    list2Selector,
    keywordSelector,
    filteredList2Selector,
    createImmutableSelector;

// state for reducer a:
state = Immutable.Map({
    list1: Immutable.List(),
    list2: Immutable.List(['hello goodbye', 'hello hello']),
    keyword: ''
});

list1Selector = state => state.get('list1');
list2Selector = state => state.get('list2');
keywordSelector = state => state.get('keyword');

filteredList2Selector = createSelector(
    [list2Selector, keywordSelector],
    (list2, keyword) => {
        return list2
            .filter(x => {
                return x.indexOf(keyword) !== -1;
            });
    }
);

const somethingExpensive1Selector = createSelector(
    [list1Selector, filteredList2Selector],
    (list1, filteredList2) => {
        console.log('EXPENSIVE COMPUTATION TRIGGERED! Generating for ===...');
        return Math.random();
    }
);

createImmutableSelector = createSelectorCreator(Immutable.is);

const somethingExpensive2Selector = createImmutableSelector(
    [list1Selector, filteredList2Selector],
    (list1, filteredList2) => {
        console.log('EXPENSIVE COMPUTATION TRIGGERED! Generating for Immutable.is()...');
        return Math.random();
    }
);

state = state.set('keyword', '');
console.log('expensive1(state)', somethingExpensive1Selector(state));
state = state.set('keyword', 'he');
console.log('expensive1(state)', somethingExpensive1Selector(state));
state = state.set('keyword', 'hell');
console.log('expensive1(state)', somethingExpensive1Selector(state));
state = state.set('keyword', 'helldsfs');
console.log('expensive1(state)', somethingExpensive1Selector(state));
console.log('')
console.log('====================================')
console.log('')
state = state.set('keyword', '');
console.log('expensive2(state)', somethingExpensive2Selector(state));
state = state.set('keyword', 'he');
console.log('expensive2(state)', somethingExpensive2Selector(state));
state = state.set('keyword', 'hell');
console.log('expensive2(state)', somethingExpensive2Selector(state));
state = state.set('keyword', 'helldsfs');
console.log('expensive2(state)', somethingExpensive2Selector(state));

@ellbee ellbee mentioned this issue Aug 29, 2015
6 tasks
@ellbee ellbee closed this as completed Sep 9, 2015
@pheuter
Copy link

pheuter commented Oct 21, 2015

Just wondering what the discussion concluded on? If someone is using Immutable.js with Redux, like with redux-immutablejs for example, does it make sense to also use reselect?

@ellbee
Copy link
Collaborator

ellbee commented Oct 22, 2015

Yes, it does make sense. There is a section in the docs about it here.

@thewillhuang
Copy link

i fail to see why even bother using immutable js with reselect when reselect has already done prop checks for you anyway at the top level, therefore shortcutting any unnecessary rerenders, the docs you link don't actually give a compelling reason to use immutable js with reselect, if anything it just adds complexity for no apparent gain in performance

@pheuter
Copy link

pheuter commented Jan 15, 2016

@thewillhuang you bring up good points in terms of potentially neglibile performance gains, but a significant benefit I see from using Immutable.js is a consistent way of dealing with values throughout the codebase. Since we already use it in terms of the Redux store structure being an Immutable Map, having the selectors play nicely without converting JSON <-> Immutable types is good.

You also get nice, small things like accessing deep properties on an object (getIn) without worrying about possibly undefined values and runtime errors.

The general idea of why we went with Immutable.js in the first place is an easy API around producing memory-efficient immutable values without worrying about the edge cases of using the ES spread operator and creating copies of deeply-nested objects where just one property has changed.

@thewillhuang
Copy link

@pheuter good points, i was really just thinking if you can simply build a project with reselect or immutable and still get the performance benefits, which is really the main reason to use immutable or reselect in the first place.
Though i see the benefits of using both, Im just not sure its worth the added complexity to add both immutable.js AND reselect together to an already complex application, especially if react-redux's default mergeProps functions already implements shouldComponentUpdate by default.
If you are using immutable and react-redux's correctly, you will only be passed a prop that has changed.. which means regardless you will have to do a recalculation and really, in that case, what is the benefit of reselect?

@bunkat
Copy link

bunkat commented Jan 18, 2016

There really is no added complexity in using both immutable.js and reselect and they do work really well together. We use an immutable store with redux to make our reducers and change detection simpler. We use reselect in order to create a facade on top of our store so that the application never needs to know how the store is laid out. We also use reselect to cache computed data.

For example, our store has a list of entities that need to be displayed. Instead of the views directly accessing the entities in the store, they access them via reselect. The nice thing is that unknown to the views, the reselect query is actually combining sort and group preferences from the user, current entity list base on the url, and applying filter and searches. So the view just calls the 'entitySelector' and it gets back exactly the right data, which is automatically cached at the highest level possible (i.e. if the sort changes, the entity list is invalidated and recalculated by reselect).

@thewillhuang
Copy link

okay, i see your point. even with react-redux, every prop change, regardless of how small the change is will always force a filter or sort to happen, where as with reselect, there is potential to skip an expensive calculation if the input of a reselected calculation never changes. good points.

@cormacrelf
Copy link

cormacrelf commented Jan 19, 2016

I don't think the docs make it clear enough how easy it is to avoid problems with excessive recomputation, for most cases.

First, avoid using filter() & friends in your reducers. Store as little data as possible that will represent your app state; reselect helps you do that. If you absolutely must use them, then sure. But most of the time, don't. You probably either don't need to, or won't be affected much by a couple of false recomputations here and there. These parts of your reducers should be triggered by specific actions, like data coming in off the server or sporadic user-activated or long-interval maintenance operations. (For the example in the docs, you probably wouldn't worry; you don't need to clean up old todos very often.)

Second, whenever there's a value in your selectors that's coming from a filter() (or anything that gives a new object every time), extract that portion of the selector into a dependency, and use a simple createSelector to memoize it individually. You don't need Immutable.is at all, you can stick with simple equality checks.

For example, this:

const filterAndOther = createSelector(
    (state) => state.immutableCollection.filter((x) => x > 5), // ALWAYS gives new object ref for ===
    (state) => state.otherValue,
    (filtered, other) => {
        return { other, filtered };
    }
);

becomes this:

// filterOnly is memoized on immutableCollection, so doesn't change when
// immutableCollection doesn't change. 
const filterOnly = createSelector(
    (state) => state.immutableCollection // ONLY SOMETIMES gives new object ref for ===
    (coll) => coll.filter((x) => x > 5),
);
const filterAndOther = createSelector(
    filterOnly, 
    (state) => state.otherValue,
    (filtered, other) => {
        return { other, filtered };
    }
);

and the problem is solved.

If you're still worried about performance for those rare times you need to use filter(), map(), etc in your selectors... memoize that small part of your selectors.

@cormacrelf
Copy link

For an example using a cool multi-item cache of previous selector inputs and only filtering in the selector, check out https://jsbin.com/kohonu/231/edit?js,console,output

@thewillhuang
Copy link

@cormacrelf heres another question, would you do your filter and or sort inside the reducer, or would you rather do it in your selectors with reselect? Why?

@rhagigi
Copy link

rhagigi commented Jun 8, 2016

Depends what you're doing. I'm about to do this in selectors to use this same pattern with normalized immutable.js lists -> filters

@cormacrelf
Copy link

cormacrelf commented May 22, 2017

@Domiii

So the question remains: Why can't we just initialize reselect using a comparison function of our own choosing?

You can initialize reselect with a different comparison function using createSelectorCreator. See the docs, the code, and most of the rest of this thread. I don't know what you're talking about on that one.

The thing is that because the entire state is immutable, any change to any data will cause the entire page to re-render, no matter if it's data has been changed or not.

The whole tree doesn't re-render if only a middle-man piece of data changes. If you modify one part of the tree, yes, the new === equality propagates up to the root, but that's the point, it means Redux doesn't shoot itself. But it doesn't change unaffected parts. fromJS({ a: A, b: B}).delete('a').get('b') === B. And reselect doesn't change that, such that if you're rendering a list based on a (List<ID>, Map<ID,Model>) => List<Model> selector, then unchanged models are still unchanged in the output list from reselect. Contain the rendering of actual models to more components with their own shouldComponentUpdate using reference equality, and most of a 10,000 strong list will not be re-rendered, and rendering the list itself will be cheap.

I have to use Angular now, with @ngrx/store and RxJS' .distinctUntilChanged() operator does that job for me.

Edit: original comment from Domii was deleted.

@Domiii
Copy link
Contributor

Domiii commented May 23, 2017

@cormacrelf Thank you for your clarification. And yeah, I deleted my comment, after I realized that I got a few things mixed up.

Also thank you for createSelectorCreator!

I'll keep experimenting! :)

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

9 participants