Feature request: have shouldRecompute instead of custom equality function

[alamothe]

I would like to be able to provide a function:

shouldRecompute: (lastResult: T, lastArgs: Array<unknown>, newArgs: Array<unknown>) => boolean

instead of custom equality function. In my case, whether the result should be recomputed depends on the previous result.

Use case:
I am building a calendar with monthly view. For input data (events etc.) and given date, I filter events and compute begin and end, which are first and last timestamp for that month. In shouldRecompute, I would like just to check if new date is between these two values. Alternative is to have a custom equality which would compare months of previous and new date argument, but this is pretty costly (in general they are unix timestamps).

In general, my opinion is that shouldRecompute is better than custom equality even for general case. Most of the time, arguments have different types, so custom equality ends up being a branchy if statement checking for types of arguments and performing duty.

With shouldRecompute, it would be possible to test all arguments at once, which is much easier to reason about.

E.g. before:

if (typeof a === 'string') {
  return a === b;
} else if (a instanceof Date) {
  return isSameMonth(a, b);
} else {
  ...
} 

after:

return lastArgs[0] === newArgs[0] && isSameMonth(lastArgs[1], newArgs[1]) && ...;

[alexreardon]

Very interesting! Worth thinking about more for sure

[alexreardon]

@alamothe what if we passed the index in as the third argument to the isEqual function?

[alexreardon]

- type EqualityFn = (a: mixed, b: mixed) => boolean;
+ type EqualityFn = (a: mixed, b: mixed, index: number) => boolean;

[alexreardon]

this seems like a reasonable path forward 👍

[alamothe]

Thanks! Would love to use the library for this use case as well 🙂

It serves really well for memoizing things based on props as recommended by React

[alexreardon]

Would an index be a reasonable path forward for you?

[alamothe]

In my opinion, it could improve the general case a bit (i.e. check for index instead of type), but it doesn't solve the use case I mentioned (where previous, cached result is also necessary to make a decision whether it is still valid).

[alexreardon]

Closed by #42

[alexreardon]

You would be able to achieve what you are after @alamothe by using a higher order function #48

[alamothe]

I like the latest API but still doesn't work for my case. How about just pass lastResult as a third argument? That won't break anything since previous arguments are unchanged.

[OliverJAsh]

I also need something like lastResult. This is necessary whenever the function receives a wide parameter, and the return value is an object (non-primitive) so it will never be reference equal to the previous value, even if the contents have not changed.

Reduced test case:

import assert from 'assert';
import memoizeOne from 'memoize-one';

type User = { age: number; name?: string };

const getAges = (user1: User, user2: User) => [user1.age, user2.age];
const getAgesMemoized = memoizeOne(getAges);

const user1: User = { age: 10 };
const user1b: User = { ...user1, name: 'bob' };
const user2: User = { age: 20 };

// No error ✅
assert(getAgesMemoized(user1, user2) === getAgesMemoized(user1, user2));

// Error ❌
// Arrays do not have the same references, but their contents have not changed.
assert(getAgesMemoized(user1, user2) === getAgesMemoized(user1b, user2));

/cc @alexreardon Do you think we reconsider adding something like lastResult?

[OliverJAsh]

As a workaround incase it helps others, I ended up creating a helper to achieve this:

import { identity } from 'fp-ts/lib/function';
import memoizeOne from 'memoize-one';
import { pipe } from 'pipe-ts';

// https://github.com/alexreardon/memoize-one/issues/39#issuecomment-609927050
export const memoizeOneByResult = <A extends unknown[], R>(
  fn: (...args: A) => R,
  isResultEqual: (a: R, b: R) => boolean,
) => pipe(fn, memoizeOne(identity, isResultEqual));