Add a way to shortcut evaluation about reselect HOT 26 CLOSED

I'm obviously very late to the party here, but I'm curious what you think about the following solution to the original problem:

So, the idea is that selectors are just functions, right? So why not make a "higher order" selector - a selector that returns a selector:

const finalListSelector$ = createSelector(
    shouldFilter$,
    (shouldFilter) => shouldFilter ? filteredList$ : hugeList$
)

// wrapper function for convenience
const finalList$ = state => finalListSelector$(state)(state)

Do you see any problems with this solution? It should memoize correctly, shouldn't it?

from reselect.

This is great, thanks for the detailed and clear write up. I agree this would be useful and should probably be in the library. What do you think @faassen?

from reselect.

It would break compatibility, but a very easy solution is to replace the valueEquals function with a shouldSelectorUpdate function, and call it directly instead of using argsEquals. The current functionality could be implemented by just defaulting shouldSelectorUpdate to a shallow list equality check (like argsEquals is now).

from reselect.

Thanks for that writeup! I wondered what would happen if you combine both solutions, but that won't help; expensive and quite possibly needless computation could still be triggered if someone changes shouldFilter.

I think we should do our research: what do NuclearJS and re-frame do about this? If they happen to do nothing about it, we have to think about why not, too.

I am curious to see both your suggested alternatives in code; I'm having trouble understanding the symbol alternative, and I'm curious where you'd pass in shouldSelectorUpdate for the second suggestion you make.

from reselect.

@faassen Haven't looked into anyone else's solution, but I'll try and look around tonight. Right now I prefer the shouldUpdate option as it's really just a more flexible version of the current code.

These are rough, but should give you an idea of what I'm thinking.

The symbol method:

export const ShouldNotUpdate = Symbol();

function memoize(func, valueEquals) {
    let lastArgs = null;
    let lastResult = null;
    return (...args) => {
        if (lastArgs !== null && argsEquals(args, lastArgs, valueEquals)) {
            return lastResult;
        }
        let result = func(...args);
        if (result === ShouldNotUpdate) {
            return lastResult;
        }
        lastArgs = args;
        lastResult = result;
        return lastResult;
    }
}

// end-user code

import { createSelector, ShouldNotUpdate } from 'reselect';

const filteredList$ = createSelector(
  [shouldFilter$, hugeList$],
  (shouldFilter, hugeList) => {
    if (!shouldFilter) { return ShouldNotUpdate; }
    /* do expensive computation here */
    return filteredList;
  }
);

The shouldUpdate method:

function memoize(func, shouldUpdate, initialValue = null) {
    let lastArgs = null;
    let lastResult = initialValue;
    return (...args) => {
        if (!shouldUpdate(args, lastArgs)) {
            return lastResult;
        }
        lastArgs = args;
        lastResult = func(...args);
        return lastResult;
    }
}

function defaultShouldUpdate(args, lastArgs) {
    if (args === null) {
        return true;
    }
    return args.some((arg, index) => arg !== lastArgs[index]);
}


// end-user code

import { createSelector } from 'reselect';

const filteredList$ = createSelector(
  [shouldFilter$, hugeList$],
  (shouldFilter, hugeList) => {
    /* do expensive computation here */
    return filteredList;
  },
  ([shouldFilter, hugeList], lastArgs) => {
    return shouldFilter && hugeList !== lastArgs[1];
  }
);

from reselect.

Another way to deal with it is to give a selector access to state as an additional argument.

const filteredList$ = createSelector(
  [hugeList$],
  (hugeList, {shouldFilter}) => {
    if (!shouldFilter) return;
    /* do expensive computation here */
    return filteredList;
  }
);

I do something similar in react-derive but maybe it makes more sense with props than with state.

from reselect.

Yet another idea: turn the selector into a thunk if we want to conditionally execute it. The following is not a smart way to implement it (a new thunk gets created on every update) but it should give an idea of what it could look like.

import { createSelector, makeLazy } from 'reselect';

const shouldFilter$ = state => state.shouldFilter;
const hugeList$ = state => state.hugeList;
const keyword$ = state => state.keyword;

const filteredList$ = createSelector(
  [hugeList$, keyword$],
  (hugeList, keyword) => {
    /* do expensive computation here */
    return filteredList;
  }
);

const list$ = createSelector(
  [shouldFilter$, hugeList$, makeLazy(FilteredList$)],
  (shouldFilter, hugeList, lazyFilteredList) => {
    return shouldFilter ? lazyFilteredList() : hugeList;
  }
);

makeLazy:

export function makeLazy(selector) {
    return state => () => selector(state);
}

from reselect.

I was traveling last week, now sitting down again to try to think this through again. Please bare with me as I'm rusty. :)

@heyimalex Thanks for working them out! That lets us evaluate them better. I'm a bit wary of the arguments approach, as it needs so much dealing with argument lists and is in part dependent on the order.

I like how the approach by @gilbox makes things simple, but it appears to break abstraction -- we only pass in state because we want to bypass the selector caching mechanism. I think.

@ellbee's approach is very interesting. We express in the selector that we want to make the calculation of the sub-selector under the control of the function itself.

But before we proceed I still think we should do the research: what do other frameworks do about this?

from reselect.

NuclearJS adds the result to a cache every time the arguments change so toggling shouldFilter back and forth does not cause the expensive recalculation. As far as I am aware re-frame does not do anything to address the problem.

I've just been looking more closely at the approach by @gilbox. In the example is { shouldFilter } one of the arguments taken into account for memoization? It looks to me that if { shouldFilter } is part of the memoization then it will perform the expensive calculation every time shouldFilter changes from false to true, but if the { shouldFilter } is not part of the memoization then changes to shouldFilter will have no effect unless hugeList also changes.

from reselect.

Another option that hasn't been suggested yet is adding a cache for the memoization like NuclearJS.

from reselect.

@ellbee, { shouldFilter } is simply destructuring of the component's state object (ie the pre-derived data). It's not a memoized value and there is no evaluation at all.

from reselect.

Thanks @gilbox. So the overall example looks something like this:

const shouldFilter$ = state => state.shouldFilter;
const hugeList$ = state => state.hugeList;

const filteredList$ = createSelector(
  [hugeList$],
  (hugeList, {shouldFilter}) => {
    if (!shouldFilter) return hugeList;
    /* do expensive computation here */
    return filteredList;
  }
);

const list$ = createSelector(
  [shouldFilter$, filteredList$],
  (shouldFilter, filteredList) =>  filteredList
);

from reselect.

Giving the user access to the state as an additional parameter is tempting but i vote against it as it will result in quite unpredictable selectors. (e.g. the result depending on a parameter which was not memoized. therefore updates could be lost as the memoized parameters did not change)

@gilbox I would suggest another approach which should give you the desired behavior. If the "memoize" function used in reselect is explicitly exported as an additional symbol (@faasen would this be possible).
The filter function could be memoized (and hence would only be recomputed if the input list actually changes).

Hence the resulting code for the filteredListSelector would be:

let filterList = memoize(inputList => { expensive filter operation... });

let filteredListSelector = createSelector([hugeList, shouldFilter], (hugeList, shouldFilter) => {
     return (shouldFilter) ? filterList(hugeList) : hugeList;
});

Imho this would make the conditional code as well as the memoization of the filter operation explicit. In addition the function solely depends on the list and whether it sould be filtered. @heyimalex By memoizing the filterList operation itself frequent toggeling of shouldFilter will not result in a lagging ui.

from reselect.

@faassen @speedskater I like this idea.

from reselect.

Honestly I don't use redux or reselect, I just follow reselect as inspiration for react-derive so maybe I misunderstand how selectors are used... Do you only ever use components with selectors 1 time (ie "container" components)? If not then you are using the same filterList memoized function for multiple instances of the same component. It will work in many cases but it's not a general solution.

from reselect.

@gilbox The idea behind selectors is to provide accessor functions on the global state. Which in case of redux is composed by using multiple reducers. So the filteredListSelector used in this example extracts and filters a specific list in your store (the global state). This selector is independent of the context in which it can be used, it can be another selector or it can be one or many components.

If you want to reuse the selector for different lists in your store you can create a factory function which allows you to create different selectors bound to different lists and therefore having different memoized versions of filterList (but actually only one implementation of filterList).

Does this explanation make it clearer or you?

from reselect.

@speedskater I like it because:

• It doesn't break anything
• This is an optimization anyways, and most of the other solutions don't make life any easier for the end user. Ultimately reselect is <50 lines so if anyone ever runs in to perf problems they can read the lib and solve it themselves.

However, to export memoize we'd have to undo some changes I made in #9 😄

from reselect.

@heyimalex I see ;). So should we rename memoize to internalMemoize and export the following function for explicit usage?

export function memoize(func, valueEquals = valueEquals) {
   let memoizedFunction = internalMemoize(func, valueEquals);
   return (...args) => memoizedFunction(args);
}

from reselect.

@faassen @ellbee If we solve this issue with the export of memoize i think we should add advanced examples to the documentation. So users have examples how to solve advanced use cases.

from reselect.

@faassen, @heyimalex, @speedskater

So, shall we do this with an exportable memoize then?

Also, I agree with @speedskater that the docs should have an advanced use cases section so I'll open an issue for that.

from reselect.

👍

from reselect.

👍
This week I have no time. But I think i could create a PR till the end of next week.

from reselect.

Hey @speedskater, I already have a PR open for this that I created yesterday as no-one had done it yet. If you like I'll change it to make you the commit author as it was your idea and I used the code from your comments!

from reselect.

@ellbee No its okay for me. Great that your are working on it.

from reselect.

The default memoize function is an export in version 1.0.0

from reselect.

@ropez I like that solution as well. I wrote a quick selector creator to leverage it:

https://gist.github.com/CGamesPlay/e3cb9d62e95be13a364100c707f46dbf

from reselect.