Regression when using lodash for an unbounded cache and reselect 4.0

[nicolaserny]

I have the following unit test inspired from the reselect documentation (Use memoize function from lodash for an unbounded cache section)

import { createSelectorCreator } from "reselect";
import memoize from "lodash.memoize";

// generic hash function provided by reselect to create custom selector
const hashFn = (...args) =>
  args.reduce((acc, val) => acc + "-" + JSON.stringify(val), "");

const unlimitedCacheSelectorCreator = createSelectorCreator(memoize, hashFn);

describe("unlimitedCacheSelectorCreator", () => {
  const selectorCreatorTest = [
    { id: 0, expected: ["test0"], recomputations: 1 },
    { id: 2, expected: ["test2"], recomputations: 2 },
    { id: 0, expected: ["test0"], recomputations: 2 },
    { id: 1, expected: ["test1"], recomputations: 3 },
    { id: 0, expected: ["test0"], recomputations: 3 },
    { id: 2, expected: ["test2"], recomputations: 3 },
    { id: 1, expected: ["test1"], recomputations: 3 }
  ];

  const getId = (_, id) => id;
  const stateExample = [
    { id: 0, name: "test0" },
    { id: 1, name: "test1" },
    { id: 2, name: "test2" }
  ];

  const selector = unlimitedCacheSelectorCreator(
    [state => state, getId],
    (stateValue, id) =>
      stateValue.filter(val => val.id === id).map(val => val.name)
  );

  selectorCreatorTest.forEach(test => {
    it("Selector should get values using an unlimited cache", () => {
      expect(selector(stateExample, test.id)).toEqual(test.expected);

      expect(selector.recomputations()).toEqual(test.recomputations);
    });
  });
});

All the tests passed with reselect 3.0.1.
After upgrading to reselect 4.0, 6 tests keep on failing.
It seems that the hash function is called once. Therefore the selector always returns the same value (instead of the right value).
Is there anything wrong with my code ?

[nicolaserny]

A quick look at the code shows that my problem is related to this change.

[mcrawshaw]

I can confirm the same issue here. Tested with both versions. Your pull @mosho1.

[srolel]

From lodash docs:

By default, the first argument provided to the memoized function is used as the map cache key.

In the test case, the first argument is the state object, and all calls return the same memoized value after the first call as expected. This should make the test cases pass by having memoization done with the ids as the cache keys:

const unlimitedCacheSelectorCreator = createSelectorCreator(fn => memoize(fn, getId), hashFn);

[nicolaserny]

In this solution, we mix the map cache key definition and the memoization function.

The purpose of the hashFn function is to compute the map cache key.
From reselect documentation:

const customSelectorCreator = createSelectorCreator(
  customMemoize, // function to be used to memoize resultFunc
  option1, // option1 will be passed as second argument to customMemoize
  option2, // option2 will be passed as third argument to customMemoize
  option3 // option3 will be passed as fourth argument to customMemoize
)

Therefore, memoize should use my hashFn function to get the map cache key.

[srolel]

Yes, you are right. This PR makes the required change: #359

[markerikson]

Went out in https://github.com/reduxjs/reselect/releases/tag/v4.0.0 .

[mcrawshaw]

@markerikson What do you mean? The PR was released in that tag? I believe the issue was raised due to that tag.

[markerikson]

You're right, my bad. I was looking at the wrong commit diff and got confused, sorry!