Try adding resultEqualityCheck to weakMapMemoize

[markerikson]

This PR:

• Adds RTL, latest RTK and React-Redux, and jsdom
• Reorders the InputSelectors generic arg of OutputSelector to be last, because it seemed like it was more ergonomic that way. But then I used it and I'm not sure about that.
• Drops the @internal prefix from a couple imports because Vitest seemed to be having trouble with this and it doesn't seem to serve any useful purpose
• Changed the path definitions for Vitest because it for some reason couldn't resolve 'reselect' in the new .tsx test file I added
• Added a resultsCount() getter to defaultMemoize. We currently track recomputations at the selector level, but not how many new results the memoized function returned (because it may have reused existing results if resultsEqualityCheck found a match)
• Reworked some of the selector recomputations logging
• Hacks up weakMapMemoize to try to add resultsEqualityCheck as an option

tbh I'm really not happy with where this stands. It seems to work, mostly. But we're inherently limited by the way we can't check vs existing cache entries, since we don't have the args lying around. With defaultMemoize we have all the existing cache entries in an array.

I could imagine writing some code that keeps existing results in a WeakRef of some kind (Lenz's suggestion), or traversing the cache nodes tree. But there could be N existing cache entries to compare against, and that's also the behavior that causes defaultMemoize with a maxSize > 1 to be kinda slow.

So I ended up with a single lastResult value for the whole memoized function, and we compare against that. My working assumption is that either you need this to be a cache size of 1 and you want consistent reused results if possible, or you want an infinite cache size and aren't going to be trying to reuse anything.

I added a test case with a React todo list. It currently runs a few combos of default vs weakMap, standard vs with resultsEqualityCheck, and logs the resulting renders and recomputations to the console. I specifically forced selectTodoById to return [todo] as an array to force a new reference. This wouldn't normally be the case for a straight lookup.

Here's my latest results:

Recomputations after render (default):
selectTodoIds:
memoized result function recalculated: { resultFunc: 1, inputSelectors: 1, newResults: 1 }
selectTodoById:
memoized result function recalculated: { resultFunc: 200, inputSelectors: 200, newResults: 200 }
Render count:  { listRenders: 1, listItemRenders: 200, listItemMounts: 200 }

Recomputations after toggle completed (default):
selectTodoIds:
memoized result function recalculated: { resultFunc: 2, inputSelectors: 2, newResults: 2 }
selectTodoById:
memoized result function recalculated: { resultFunc: 400, inputSelectors: 400, newResults: 400 }
Render count:  { listRenders: 2, listItemRenders: 400, listItemMounts: 200 }

Recomputations after added (default):
selectTodoIds:
memoized result function recalculated: { resultFunc: 3, inputSelectors: 3, newResults: 3 }
selectTodoById:
memoized result function recalculated: { resultFunc: 601, inputSelectors: 601, newResults: 601 }
Render count:  { listRenders: 3, listItemRenders: 601, listItemMounts: 201 }

stdout | test/computationComparisons.spec.tsx > Computations and re-rendering with React components > resultEquality
Recomputations after render (resultEquality):
selectTodoIds:
memoized result function recalculated: { resultFunc: 1, inputSelectors: 1, newResults: 1 }
selectTodoById:
memoized result function recalculated: { resultFunc: 200, inputSelectors: 200, newResults: 200 }
Render count:  { listRenders: 1, listItemRenders: 200, listItemMounts: 200 }

Recomputations after toggle completed (resultEquality):
selectTodoIds:
memoized result function recalculated: { resultFunc: 2, inputSelectors: 2, newResults: 1 }
selectTodoById:
memoized result function recalculated: { resultFunc: 400, inputSelectors: 400, newResults: 201 }
Render count:  { listRenders: 1, listItemRenders: 201, listItemMounts: 200 }

Recomputations after added (resultEquality):
selectTodoIds:
memoized result function recalculated: { resultFunc: 3, inputSelectors: 3, newResults: 2 }
selectTodoById:
memoized result function recalculated: { resultFunc: 601, inputSelectors: 601, newResults: 202 }
Render count:  { listRenders: 2, listItemRenders: 202, listItemMounts: 201 }

stdout | test/computationComparisons.spec.tsx > Computations and re-rendering with React components > weakMap
Recomputations after render (weakMap):
selectTodoIds:
memoized result function recalculated: { resultFunc: 1, inputSelectors: 1, newResults: 1 }
selectTodoById:
memoized result function recalculated: { resultFunc: 200, inputSelectors: 200, newResults: 200 }
Render count:  { listRenders: 1, listItemRenders: 200, listItemMounts: 200 }

Recomputations after toggle completed (weakMap):
selectTodoIds:
memoized result function recalculated: { resultFunc: 2, inputSelectors: 2, newResults: 2 }
selectTodoById:
memoized result function recalculated: { resultFunc: 400, inputSelectors: 400, newResults: 400 }
Render count:  { listRenders: 2, listItemRenders: 400, listItemMounts: 200 }

Recomputations after added (weakMap):
selectTodoIds:
memoized result function recalculated: { resultFunc: 3, inputSelectors: 3, newResults: 3 }
selectTodoById:
memoized result function recalculated: { resultFunc: 601, inputSelectors: 601, newResults: 601 }
Render count:  { listRenders: 3, listItemRenders: 601, listItemMounts: 201 }

stdout | test/computationComparisons.spec.tsx > Computations and re-rendering with React components > weakMapResultEquality
Last results were equal:  {}
Recomputations after render (weakMapResultEquality):
selectTodoIds:
memoized result function recalculated: { resultFunc: 1, inputSelectors: 1, newResults: 1 }
selectTodoById:
memoized result function recalculated: { resultFunc: 0, inputSelectors: 200, newResults: 0 }
Render count:  { listRenders: 1, listItemRenders: 200, listItemMounts: 200 }
Last results were equal:  [
   0,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10, 11,
  12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23,
  24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35,
  36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47,
  48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59,
  60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71,
  72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83,
  84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95,
  96, 97, 98, 99,
  ... 100 more items
]

Recomputations after toggle completed (weakMapResultEquality):
selectTodoIds:
memoized result function recalculated: { resultFunc: 2, inputSelectors: 2, newResults: 1 }
selectTodoById:
memoized result function recalculated: { resultFunc: 200, inputSelectors: 400, newResults: 200 }
Render count:  { listRenders: 1, listItemRenders: 400, listItemMounts: 200 }

Recomputations after added (weakMapResultEquality):
selectTodoIds:
memoized result function recalculated: { resultFunc: 3, inputSelectors: 3, newResults: 2 }
selectTodoById:
memoized result function recalculated: { resultFunc: 401, inputSelectors: 601, newResults: 401 }
Render count:  { listRenders: 2, listItemRenders: 601, listItemMounts: 201 }

I think this says that with weakMapResultEquality, the list only re-rendered when we added an item (expected) and not when we toggled completed (good).

Concerns

Right now I have no tests. I am also not 100% sure the actual logic is correct.

I'm really feeling uncomfortable about where things stand atm.

I'd still seriously like to make the switch to weakMapMemoize as default in Reselect 5.0. But I think it needs to have some form of resultEqualityCheck , both to keep API compat with (the relatively few) folks who are doing createSelector(a, b, {memoizeOptions: {resultEqualityCheck}}) in the wild, and also because it's a use case that defaultMemoize made handleable in 4.1 and I don't want to regress on that aspect.

but what I've got over here is very hacked up and doesn't particularly feel ready.

@aryaemami59 , can you take a look at this and give some thoughts?

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 8a6eb42:

Sandbox	Source
Vanilla Typescript	Configuration

[markerikson]

Cleaned this up some:

• Deleted the dead code and logging in weakMapMemoize
• Changed the ["object", "function"].includes() call to be if statements and inside the if (resultEqualityCheck clause
• Removed the OutputSelector generic rearranging

[aryaemami59]

Suggested change

	let lastResult: WeakRef<object> | undefined = undefined
	let lastResult: unknown