WIP 1.0.0 (Configurable memoize / variadic dependencies / pass props to selector)

[ellbee]

Work in progress experiments to address issues #7 , #20 , #27. The API section of the README has some examples.

There is one breaking change. createSelectorCreator now takes a memoize function instead of a valueEquals function.

[gaearon]

Let's capitalize as React Redux (like in docs)

[ellbee]

Agreed

[ellbee]

@speedskater @faassen @heyimalex (and anyone else reading!)

If anyone has some spare time on their hands it would be great if I get could get some opinions on this PR.

• There is a breaking change in createSelectorCreator, are we happy to make a breaking change?
• It was suggested by @gaearon that we should consider moving to 1.0.0, does anyone have any objections to doing so? (I don't)
• I am currently rewriting the readme, but I made some quick changes to it in this PR to give a couple of example of the new API

[gaearon]

I am currently rewriting the readme, but I made some quick changes to it in this PR to give a couple of example of the new API

I think everybody's waiting for “how to use this with React Redux ownProps parameter”

[ellbee]

The last couple of examples for createSelector should show roughly how it works at the moment. I am holding off on writing the docs for this as I'm not sure if it is going to stick, let me know if further explanation is needed. I have added a note to the defaultMemoizeFunc section with some thoughts about why I'm not happy with it.

[speedskater]

Can this result in problems, as the selector suddenly depends on all properties. Therefore changing whenever a single property - which might not influence the selector at all - changes.
Wouldn't it be better to have the following signature

createSelector(
state, props => state.values.value1,
state, props => state.values.value2,
state, props => props.value3,
value1, value2, value3 => value1 + value2 + value3
)

[ellbee]

@speedskater Thanks for the feedback. Inspired by your first suggestion, I simply changed memoizedResultFunc so that it doesn't take the props argument. This gets rid of the special casing for defaultMemoize, so I think it is an improvement.

[speedskater]

@ellbee Thanks. Do you really think we need multiple arguments? I think the composite object instead of a state might be a really elegant solution. In combination with destructuring assignment it seems really powerful. Or am I missing something?

[ellbee]

@speedskater I went with multiple arguments because it makes createSelector backwards compatible with 0.0.2 and also because it matches the signature of mapStateToProps.

I'm not sure I see the advantages of the composite object being that great, but I am open to be persuaded.

[speedskater]

@ellbee I get your point.

On the other hand using the composite object, the signature of a selector doesn't change at all. I think it should even work with the current version of reselect.

I think having a single object as an argument encourages better composability and flexibility.
E.g. instead of providing an object you could provide an array which will result in the same syntax you just proposed and is analogous to mapStateToProps.

var mySelector = createSelector(
  ([ state, props ]) => state.value1,
  ([ state, props ]) => state.value2,
  ([ state, props ]) => props.value3,
  value1, value2, value3 => value1 + value2 + value3
);
mySelector([state, props]);

Using this pattern the arguments of mapStateToProps can directly be passed to such kind of selector using the spread operator.

In combination with the nice features you just added like different memoization functions i think it allows even more usage scenarios.

So for me the main advantages for this kind of pattern are:

• Keeping the selector simple
• Supporting many different usage scenarios not only tied to react or redux
• Keep it flexibel in case more than state and props should be taken into consideration

If we keep selector simple I think we should emphasize this flexiblity in the documentation. Hopefully this will help the users with mapStateToProps and in other advanced scenarios as well.

What do you think ?

[ellbee]

@speedskater How does the array (or object) parameter work with the connect decorator? You couldn't just pass the selector straight in if you wanted to access the props right? I feel like I am missing something.

What if we don't take an opinion on this and have a variadic parameter list. Then it would be possible to use (state, props), ({state, props}), ([state, props]) or whatever else.

export function createSelectorCreator(memoize, ...memoizeOptions) {
    return (...selectors) => {
        const memoizedResultFunc = memoize(selectors.pop(), ...memoizeOptions);
        const dependencies = Array.isArray(selectors[0]) ?
            selectors[0] : selectors;
        return (...args) => {
            const params = dependencies.map(dependency => dependency(...args));
            return memoizedResultFunc(...params);
        };
    };
}

[speedskater]

@elbee You are right directly passing the selector without some kind of wrapper is impossible. (haven't thought about it this way).
I like your proposal of having a general purpose variadic parameter list, will make reselect in general more flexibel.

[volrath]

Hi @ellbee and @speedskater,

I'm running some tests on the latest version of reselect yet, and the last commit (31a30bd) breaks the ability for react-redux to pass down props as the second argument to selectors.

You can find the reason here:
https://github.com/rackt/react-redux/blob/master/src/components/createConnect.js#L33-L43

As you can see, react-redux calculates whether to pass props or not based on the selector's arguments's length, and having a Rest Parameter in the function definition makes its length be 0.

> (a => ({})).length
1
> ((a, b) => ({})).length
2
> ((...args) => ({})).length
0

I also took a look at all the other react-redux's branches and it's the same situation.

I created a pull request that reverts the change: #40

Awesome work here btw, loving it :)
Hope this helps.

[faassen]

I'm certainly not up to date on this issue, but I found it a bit odd that defaultMemoize has a valueEquals parameter. Shouldn't that be "createMemoize" or something? Though of course we should make sure people can create memoize functions without using it.

[ellbee]

@faassen You mean rename defaultMemoize to createMemoize? I don't think createMemoize is quite right. defaultMemoize is itself the memioze function that takes the function to be memoized. The signature is similar to that of lodash.memoize. Maybe the API itself is a little off here?

[ellbee]

In essence, defaultMemoize is our simple memoize function that has a non-configurable cache size of 1 and a configurable valueEquals function. So we need a name to sum that up. :-|

[ellbee]

Oh, and the thinking behind giving defaultMemoize the valueEquals function was so that there was an easy upgrade path for createSelectorCreator.

Before

import { isEqual } from 'lodash';
import { createSelectorCreator } from 'reselect';

const deepEqualsSelectorCreator = createSelectorCreator(isEqual);

After

import { isEqual } from 'lodash';
import { createSelectorCreator, defaultMemoizeFunc } from 'reselect';

const deepEqualsSelectorCreator = createSelectorCreator(
  defaultMemoizeFunc,
  isEqual
);

[speedskater]

Sorry for the late response but I have been on a short holiday for the last days.

@volrath, @ellbee As far as i undestand it is not that simple.
Enforcing to always have a selector of arity two has the disadvantage, that the selector will always be invoked on every property changes eventhough it does not depend on the properties at all.

As far as i know, the reason that the signature check was implemented in react-redux was to prevent such calls. But i think this check should be done by the selector and not by the higher order component.

Another possiblity would be to create a selector with the same arity as the selector with the largest arity. The problem is that creating such a function is quite expensive and i would vote highly against it.

@gaearon what do you think. Could we remove the signature check in react-redux from the connect HOC and let the selector alone decide whether it does resepect props or not? This would be equal to a reselect selector like this.

createSelector((state, props) => state, state => .....);

[ellbee]

@speedskater Hey, no worries. Hope you had a good holiday.

I am aware that the selector is going to be called every time the props change whether we are using them or not. My thinking is that if the dependencies aren't using the props, a prop change won't cause the results function to be re-run because non of the parameters to the results function will have changed. For this reason I am presuming it won't be too big a performance hit. Am I missing something?

[speedskater]

@ellbee you are right. I think it wouldn't be a performance hit at all. I even think, that the connect method should always provide the props parameter. As it is the duty of the selector to decide whether its result changes or not.

[ellbee]

@speedskater (and everyone else), is there anything you think should be looked at before releasing 1.0.0? I'd like to get the release done in the next couple of days if everyone is happy.

[speedskater]

@ellbee Thanks for your effort. From my point of view I think we could ship 1.0.0.

[heyimalex]

Nice work!

[volrath]

👍 !

[speedskater]

Great: 👍 !