memoize map/fmap about reselect

[ellbee]

Unfortunately, I think you are correct, Reselect does not manage this case at the moment. I'm actually currently in the middle of a project where I ran into a similar issue. Soon, when I am less busy, I intend to look into the cases I am where I am aware that Reselect isn't a good fit, and either figure out a way to support them directly with Reselect, release some custom selectors with different memoization strategies or make some recipes for the docs.

[heyimalex]

I just finished up a separate package, reselect-map, with the new selector creators we had talked about. Because it's so niche and the code ended up being more complex than expected, I wasn't sure it was a great candidate for being folded into reselect, but I'm not opposed or anything.

RE: Documentation, I feel like this issue should just be left to languish and those who really need it will eventually land here. Adding an advanced reselect section to the readme will mean a bunch of people implementing optimizations they don't need. But then again, maybe it's not cool for us to make that call...

[mindjuice]

@acthp For now can you just use _.memoize() on your makeExpensiveIndex() function?

[acthp]

That will leak, of course. Might not be a problem, except for long sessions. I've written a fmapMemoize1 that is like _.mapObject, but caches the prop values from the previous call.

[mindjuice]

You could try: https://github.com/medikoo/memoizee

It lets you limit cache size on an LRU basis, and supports a WeakMap version to avoid hanging onto results for which the source objects no longer exist.

[kbrownlees]

I just hit a similar issue in my code base, I have solved it in the meantime by making a 'dynamic selector' which updates as the state changes. Comments on whether this is a good approach (/ safe) are welcome!

https://gist.github.com/kbrownlees/8ed41ed6d86499b8c590

I have a relatively stable state so I don't need to change it that often, but it makes the logic a lot nicer since you act on each 'stream' individually while still retaining the advantages of caching.

[arcanis]

I've started to experiment with the following code, what do you think? I'm somewhat mixed, because it dynamically adds a new property to some objects, which may hurt the JS engine optimizations, but API-wise, it seems quite elegant.

The following is much better, it basically wraps the createSelector call to add a new syntax that allows you to set a custom comparator for each field (might still have a bug somewhere, I haven't wrote any test yet, but it worked fine on my application):

import { isPlainObject }                         from 'lodash';
import { createSelectorCreator, defaultMemoize } from 'reselect';

function defaultComparator(a, b) {
    return a === b;
}

function wrappedComparator(a, b) {
    return a.comparator(a.value, b.value);
}

function mapWrapper(dependency) {

    let extractor = dependency;
    let comparator = defaultComparator;

    if (isPlainObject(dependency) && dependency.extractor)
        extractor = dependency.extractor;

    if (isPlainObject(dependency) && dependency.comparator)
        comparator = dependency.comparator;

    if (typeof extractor !== `function`)
        throw new Error(`Expected the extractor to be a function`);

    if (typeof comparator !== `function`)
        throw new Error(`Expected the comparator to be a function`);

    return (... args) => ({ extractor, comparator, value: extractor(... args) });

}

export let createSelector = (dependencies, computer) => {

    return createSelectorCreator(defaultMemoize, wrappedComparator)(dependencies.map(mapWrapper), (... args) => {
        return computer(... args.map(({ value }) => value));
    });

};

Then you can use createSelector as such:

import { createSelector } from './reselectTools';

let getStreamModels = createSelector([

    { extractor: (state, props) => state.streamLists.get(props.eventLocator, Immutable.Set()).map(locator => state.resourceRegistry.get(locator)),
      comparator: (a, b) => a.size === b.size && a.every(aVal => b.find(bVal => aVal === bVal)) },

    (state, props) => props.sortBy,
    (state, props) => props.maxSize

], (resources, sortBy, maxSize) => {

    resources = resources.sortBy(sortBy);

    if (maxSize !== null)
        resources = resources.slice(0, maxSize);

    return resources;

});

[heyimalex]

My intuition is that a general solution that allows for nesting without really changing reselect doesn't exist; reselect hides memoization from the end user by using pure functions, but a function that does sub-element caching is inherently impure.

However, writing a few common selector-creators for a single level (say map object property by key, array element by index, array element by key-function) wouldn't be too difficult, if a little inelegant. That would cover many use cases and wouldn't take a whole lot of effort.

Maybe an interesting line of thought; react does this (that is, nested memoization) very well. One of the things it doesn't do well that we really need is control over evaluation. Maybe combining the two systems could be a good solution? It's probably better suited as a new library, but maybe someone smarter than me wants to explore the idea a bit more.

[ellbee]

My intuition is that a general solution that allows for nesting without really changing reselect doesn't exist; reselect hides memoization from the end user by using pure functions, but a function that does sub-element caching is inherently impure.

This is my intuition too.

However, writing a few common selector-creators for a single level (say map object property by key, array element by index, array element by key-function) wouldn't be too difficult, if a little inelegant. That would cover many use cases and wouldn't take a whole lot of effort.

Agreed.

[ellbee]

Unfortunately, I think you are correct, Reselect does not manage this case at the moment. I'm actually currently in the middle of a project where I ran into a similar issue. Soon, when I am less busy, I intend to look into the cases I am where I am aware that Reselect isn't a good fit, and either figure out a way to support them directly with Reselect, release some custom selectors with different memoization strategies or make some recipes for the docs.

Above is what I wrote (5 months ago!) in response to the OP. In the project I referenced the solution I ended up with was a custom selector with an LRU cache and a custom hashing scheme. At that point I had pretty much overridden all of Reselect with custom code.

In addition to the suggestions made by @heyimalex, maybe the docs could be improved with ideas of things to look at when the default Reselect stuff is not enough like using LRU caches, making sure state shape is optimal etc.

I will get around to this eventually, but in the mean time if anyone has any ideas for common selector-creators or wants to take a shot at the docs it would be welcome.

[arcanis]

The approach detailed above (separating the data selection from the data comparison so that the compared data may be different from the data actually returned by the selector) seems to work quite well for our use case, and looks generic enough to be fit multiple other usages. Do you see an issue with it?

[ellbee]

Sorry @arcanis, I somehow missed it. I will have a play with it.

[heyimalex]

@arcanis I'm not sure that approach addresses the issue? It's a little hard to follow from my phone, but it seems like that allows you to use a per-dependency equalityCheck function. If you applied it to the code in the original issue, we'd still be recomputing for every element. Unless I've misunderstood?

[ellbee]

Nice work!

In my opinion it makes sense to keep reselect-map as a separate package. I think the majority of Reselect users only need to use createSelector, so it is good to keep the API small.

RE: Documentation, I feel like this issue should just be left to languish and those who really need it will eventually land here.

Yes, you are probably right, and the documentation for Reselect is already really long for such a tiny library.

Adding an advanced reselect section to the readme will mean a bunch of people implementing optimizations they don't need.

Agreed. This happened with the new documentation I wrote about the mapStateToProps optimisation in React Redux.

[mindjuice]

Agree that a separate package makes more sense.

IMO though, it might be worth a small section in reselect's README.md to call out the use case for reselect-map, but with an admonition that you very likely don't need it (i.e., if you can't understand the use cases, you don't need it). Or even just a "Related Projects" section with a link and little/no description. Just have the (existing) admonitions in the reselect-map repo.

[ellbee]

Yes, fair point.

[ellbee]

Added a section about reselect-map to the README. Thanks for the input everyone.