Giter Site home page Giter Site logo

Comments (37)

cormacrelf avatar cormacrelf commented on May 17, 2024 23

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.

from reselect.

ellbee avatar ellbee commented on May 17, 2024 8

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.

from reselect.

ellbee avatar ellbee commented on May 17, 2024 3

@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.

from reselect.

vovacodes avatar vovacodes commented on May 17, 2024 2

@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?

from reselect.

bunkat avatar bunkat commented on May 17, 2024 1

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.

from reselect.

ellbee avatar ellbee commented on May 17, 2024 1

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

from reselect.

pheuter avatar pheuter commented on May 17, 2024 1

@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.

from reselect.

gajus avatar gajus commented on May 17, 2024
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.

from reselect.

gajus avatar gajus commented on May 17, 2024

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.

from reselect.

vovacodes avatar vovacodes commented on May 17, 2024

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

from reselect.

gajus avatar gajus commented on May 17, 2024

@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);

from reselect.

gajus avatar gajus commented on May 17, 2024

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

from reselect.

gajus avatar gajus commented on May 17, 2024

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

from reselect.

ellbee avatar ellbee commented on May 17, 2024

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.

from reselect.

ellbee avatar ellbee commented on May 17, 2024

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.

from reselect.

bunkat avatar bunkat commented on May 17, 2024

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.

from reselect.

ellbee avatar ellbee commented on May 17, 2024

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

from reselect.

vovacodes avatar vovacodes commented on May 17, 2024

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?

from reselect.

ellbee avatar ellbee commented on May 17, 2024

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().

from reselect.

gajus avatar gajus commented on May 17, 2024

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?

from reselect.

ellbee avatar ellbee commented on May 17, 2024

@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.

from reselect.

gajus avatar gajus commented on May 17, 2024

@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.

from reselect.

vovacodes avatar vovacodes commented on May 17, 2024

@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));

from reselect.

gajus avatar gajus commented on May 17, 2024

@wizardzloy Modify when?

from reselect.

vovacodes avatar vovacodes commented on May 17, 2024

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

from reselect.

gajus avatar gajus commented on May 17, 2024

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

from reselect.

ellbee avatar ellbee commented on May 17, 2024

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));

from reselect.

pheuter avatar pheuter commented on May 17, 2024

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?

from reselect.

thewillhuang avatar thewillhuang commented on May 17, 2024

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

from reselect.

thewillhuang avatar thewillhuang commented on May 17, 2024

@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?

from reselect.

bunkat avatar bunkat commented on May 17, 2024

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).

from reselect.

thewillhuang avatar thewillhuang commented on May 17, 2024

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.

from reselect.

cormacrelf avatar cormacrelf commented on May 17, 2024

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

from reselect.

thewillhuang avatar thewillhuang commented on May 17, 2024

@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?

from reselect.

rhagigi avatar rhagigi commented on May 17, 2024

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

from reselect.

cormacrelf avatar cormacrelf commented on May 17, 2024

@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.

from reselect.

Domiii avatar Domiii commented on May 17, 2024

@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! :)

from reselect.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.