Fix `resultEqualityCheck` behavior in `weakMapMemoize`

aryaemami59 commented 8 months ago

Overview

Previously using weakMapMemoizewith resultEqualityCheck would cause the resultEqualityCheck to be called with empty objects as arguments due to how inputStabilityCheck works. Here's a summary of the changes:

This PR:

[X] Fixes resultEqualityCheck in weakMapMemoize so that when input selectors are stable, resultEqualityCheck does not get called on the first run of the output selector.
[X] Adds runtime unit tests to verify that:
- resultEqualityCheck works correctly when set to shallowEqual.
- resultEqualityCheck is not called on the first output selector call.
- weakMapMemoize/lruMemoize with resultEqualityCheck set to referenceEqualityCheck works the same as weakMapMemoize/lruMemoize without resultEqualityCheck.
[X] Adds benchmarks to measure the resultEqualityCheck performance impact.
[X] Adds unit tests to test the dynamics between inputStabilityCheck and resultEqualityCheck.
[X] Fixes a specific bug within weakMapMemoize where setting resultEqualityCheck to referenceEqualityCheck would prevent memoizedResultFunc.resultsCount() from incrementing upon result recalculation.
[X] Fixes a performance issue in weakMapMemoize when comparing two memoized functions, one with resultEqualityCheck set to referenceEqualityCheck and the other without resultEqualityCheck. Previously, there was a noticeable discrepancy between the two, which has now been corrected to achieve the desired effect.

netlify[bot] commented 8 months ago

Deploy Preview for reselect-docs canceled.

Name	Link
Latest commit	cc884ccdc70eb04faa401b5baa5abf6bb5cb117b
Latest deploy log	https://app.netlify.com/sites/reselect-docs/deploys/65d620ac0efa7c0008b51fa0

codesandbox-ci[bot] commented 8 months ago

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.

keegan-lillo commented 8 months ago

Thanks for working on this! I believe this PR is related to this issue: https://github.com/reduxjs/reselect/issues/693

EskiMojo14 commented 8 months ago

related, but not a complete fix - if inputs aren't stable, your resultEqualityCheck will still be called with an empty object

nonetheless the improvements here are nice

aryaemami59 commented 8 months ago

@keegan-lillo This should mitigate the problem as long as your input selectors are stable. It will at least make the default configs a tiny bit less problematic. But @EskiMojo14 is correct, it doesn't fully address the issue.

EskiMojo14 commented 8 months ago

this looks good, but has conflicts - happy to merge once fixed

EskiMojo14 commented 8 months ago

thanks!

braeden commented 5 months ago

Hey @EskiMojo14, any chance a release could be cut which contains this change?

markerikson commented 5 months ago

Sorry for the delay on this. I should have time to get it out on Saturday.

markerikson commented 5 months ago

Okay, out in https://github.com/reduxjs/reselect/releases/tag/v5.1.1 !

reduxjs / reselect