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

Suggestion: provide first class support for array selectors #451

Closed
OliverJAsh opened this issue Apr 4, 2020 · 8 comments
Closed

Suggestion: provide first class support for array selectors #451

OliverJAsh opened this issue Apr 4, 2020 · 8 comments
Labels

Comments

@OliverJAsh
Copy link

OliverJAsh commented Apr 4, 2020

Currently, we have this problem with selectors which derive and return arrays, whereby the array reference is not preserved even if the array contents have not changed:

const assert = require('assert');
const { createSelector } = require('reselect');

const state1 = {
  users: {
    foo: { id: 'foo' },
    bar: { id: 'bar' },
  },
};
const state2 = {
  users: {
    foo: { id: 'foo', age: 1 },
    bar: { id: 'bar' },
  },
};

const getUsersById = state => state.users;

const getUserIds = createSelector(getUsersById, users => Object.keys(users));

// error!
// they are deep equal but not reference equal
assert(getUserIds(state1) === getUserIds(state2));

We would like to write our selectors so that the array reference is preserved if the contents of the array have not changed.

As far as I understand, selectors like this (which derive and return arrays) are very common in React Redux apps. In React Redux, the best practice is for list components to pass IDs down to each child, and for the list item child to connect and lookup its own state value using the provided ID. The reason this is good for performance is because then the parent list component (and all of its children) will not have to re-render when one of the items changes. (More information in this article).

However, this performance optimisation only works if the selector that returns an array of IDs is correctly memoized—and currently there's just no straightforward way to do that with reselect, out of the box.

(In this case, we could store an array of IDs in Redux, as well as the dictionary, but that wouldn't work in many other cases e.g. where we're filtering an array inside a selector.)

Given this is a common pattern, I believe reselect should provide a direct solution, out of the box.

Related discussions:

@nathggns
Copy link

Has there been any thought on this?

@markerikson
Copy link
Contributor

I don't think there's any real active development going on with Reselect atm.

That said, looking at the issue, I agree that it's a common use case that would be useful to handle in some way.

@amsterdamharu
Copy link

There is a pretty simple work around for this:

import { defaultMemoize, createSelector } from 'reselect';

const createMemoizeArray = (array) => {
  const memArray = defaultMemoize((...array) => array);
  return (array) => memArray.apply(null, array);
};
const getUsersById = (state) => state.users;
const getUserIds = ((memArray) =>
  createSelector(getUsersById, (users) =>
    memArray(Object.keys(users))
  ))(createMemoizeArray()); //IIFE

// works
assert(getUserIds(state1) === getUserIds(state2));

@OliverJAsh
Copy link
Author

The best workaround I've found so far is the one discussed in #74. That is, a custom memoize function called resultCheckMemoize.

Here it is applied to the example I gave in the original post.

I think resultCheckMemoize should be added to the library. WDYT @ellbee?

const assert = require('assert');
const { createSelectorCreator } = require('reselect');
const shallowEqual = require('shallowequal');

// https://github.com/reduxjs/reselect/blob/c1fa2caee69dd31d3185bc5aa30d4166f040f3de/src/index.js#L1
function defaultEqualityCheck(a, b) {
  return a === b
}

function resultCheckMemoize(
  func,
  resultCheck = defaultEqualityCheck,
  argsCheck = defaultEqualityCheck,
) {
  let lastArgs = null;
  let lastResult = null;
  return (...args) => {
    if (
      lastArgs !== null &&
      lastArgs.length === args.length &&
      args.every((value, index) => argsCheck(value, lastArgs[index]))
    ) {
      return lastResult;
    }
    lastArgs = args;
    const result = func(...args);
    return resultCheck(lastResult, result) ? lastResult : (lastResult = result);
  };
}

const state1 = {
  users: {
    foo: { id: 'foo' },
    bar: { id: 'bar' },
  },
};
const state2 = {
  users: {
    foo: { id: 'foo', age: 1 },
    bar: { id: 'bar' },
  },
};

const getUsersById = state => state.users;

const createArraySelector = createSelectorCreator(resultCheckMemoize, shallowEqual)

const getUserIds = createArraySelector(getUsersById, users => Object.keys(users));

assert(getUserIds(state1) === getUserIds(state2));

OliverJAsh added a commit to OliverJAsh/reselect that referenced this issue Jun 4, 2020
@OliverJAsh
Copy link
Author

OliverJAsh commented Jun 4, 2020

PR to add something like resultCheckMemoize to the library: #456

spautz added a commit to spautz/dynamic-selectors that referenced this issue Oct 15, 2020
These features may arrive in Reselect in the future (discussed in reduxjs/reselect#74, reduxjs/reselect#451,
and reduxjs/reselect#456)
@akmjenkins
Copy link

akmjenkins commented Dec 12, 2020

@OliverJAsh have you checked out: #441?

The functional equivalent of adding a resultCheckMemoize to reselect, is to simply to memoize (in a manner of your choosing) your resultFunc:

const mySelector = createSelector(
    func1,
    func2.
    memoize(func3) <- memoize the "resultFunc"
);

There's a neat example in that issue of memoizing an array producing selector. You could easily create a function to memoize an object producing function

EDIT: Ok, it's a bit of a misnomer, you aren't really memoizing it - redux does this by not calling the function if the arguments haven't changed. What you're actually doing is using a higher-order-function (which I've called memoize above) to return a function that return's it's previous output if you've deemed it's previous output to be equal to it's next output.

@OliverJAsh
Copy link
Author

@akmjenkins That's actually the same as what I settled on in the end: #441 (comment)

@markerikson
Copy link
Contributor

This should be resolved by #513 , albeit slightly indirectly, in that it now has a resultCheckMemoize option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants