reselect icon indicating copy to clipboard operation
reselect copied to clipboard

clear lastResult between tests

Open alissaVrk opened this issue 2 years ago • 10 comments

I have a selector which looks at the lastResult value and decides whether it needs to recalculate. the problem is I can't find a way to clear this value between tests.

I'm running selector.memoizedResultFunc.clearCache() in beforeEach but it doesn't seem to help

alissaVrk avatar Mar 15 '22 20:03 alissaVrk

This actually works for me...

what doesnt work for me is using the clearCache method on the selector itself when using a maxSize of more than 1 to support several variants of an argument per:

  • https://github.com/reduxjs/reselect#q-how-do-i-create-a-selector-that-takes-an-argument
  • https://github.com/reduxjs/reselect#defaultmemoizefunc-equalitycheckoroptions--defaultequalitycheck

Also the clearCache() method does not seem to be included on the memoizedResultFunc type definition, which makes for a painful casting work around

export const getEscrowsByCategory = createSelector(
  [
    getEscrows,
    (_state: CommonReducerShape, escrowCateorgy: EscrowRequestCategory) =>
      escrowCateorgy,
  ],
  (escrowsById, escrowCategory) =>
    Object.values(escrowsById).filter(escrow =>
      escrowCategoryToStateMap[escrowCategory].includes(escrow.state),
    ),
  {
    memoizeOptions: {
      equalityCheck: isEqual,
      maxSize: Object.keys(EscrowRequestCategory).length,
    },
  },
)

// Unit test

    beforeEach(() => {
       // does not clear cache but is on the type definition :(
       // getEscrowsByCategory.clearCache()
       
       // works but is not on type definition of `memoizedResultFunc`
      ;(
        getEscrowsByCategory.memoizedResultFunc as unknown as typeof getEscrowsByCategory
      ).clearCache()
      // this works great though :)
      getEscrowsByCategory.resetRecomputations()
    })

ghost avatar Mar 23 '22 02:03 ghost

@alexburkowskypolysign Hmm. clearCache() should be on both functions. Can you put together a CodeSandbox that shows the issue?

markerikson avatar Mar 23 '22 03:03 markerikson

@markerikson see the type error here: https://codesandbox.io/s/tender-lederberg-l1d1jr?file=/src/index.ts

And update to my previous comment - the type issue seems to hold true for selectors created without memoization options as well per the codesandbox

The method itself I believe IS on both functions in JS land (console.log says both are Functions) although I cant get it to work in tests without accessing memoizedResultFunc but unfortunately its not on the type definition of memoizedResultFunc

I think its because the generic argument Combiner is usually extending UnknownFunction: https://github.com/reduxjs/reselect/blob/2f892bb92e1e7f2a525b3adc7d213f2d1e570f32/src/types.ts#L30

Is it intended that clearCache() should work directly on the selector without the need to access memoizedResultFunc? Its not clear from the README but it seems implied that it shouldnt (at least how I read it):

https://github.com/reduxjs/reselect#defaultmemoizefunc-equalitycheckoroptions--defaultequalitycheck

The returned memoized function will have a .clearCache() method attached.

My concern overall is mostly with the type definitions, I kinda dont have a need for clearCache outside of tests anyway. Just bringing the behavior to your attention

ghost avatar Mar 23 '22 03:03 ghost

Uh... this is really bizarre, because TS says it should be there as part of the typedef of memoizedResultFunc:

Property 'clearCache' does not exist on type '(args_0: any, args_1: string) => void & { clearCache: () => void; }'.ts(2339)

As far as the runtime behavior: the issue here is that createSelector actually uses your provided memoization function twice: once to memoize against the actual arguments you pass in (and potentially skip doing any work at all), and again to memoize the results of the output functions (to potentially skip running the final calculation). It's the latter that actually matters for clearing cache, hence memoizedResultFunc.clearCache().

markerikson avatar Mar 23 '22 04:03 markerikson

It's the latter that actually matters for clearing cache, hence memoizedResultFunc.clearCache()

Makes sense this should probably be clarified in the README

To add to the weirdness @markerikson - I cannot get clearCache to work at runtime in my updated codeSandbox with either usage now. See the tests - https://codesandbox.io/s/tender-lederberg-l1d1jr?file=/src/index.ts

Maybe im using it incorrectly in the sandbox but correctly in my IDE?

ghost avatar Mar 23 '22 04:03 ghost

Uh... this is really bizarre, because TS says it should be there as part of the typedef of memoizedResultFunc

Is it possible TS is reading that additional { clearCache: () => void; } as part of the return type void & { clearCache: () => void; } rather than the Function type def + { clearCache: () => void; }

ghost avatar Mar 23 '22 04:03 ghost

Maybe? Unfortunately this isn't something I really have much time to look into in the near future. If you can identify an issue with the types or runtime behavior, definitely interested in anything we can fix.

markerikson avatar Mar 23 '22 04:03 markerikson

I dont have a ton of time until probs end of the week. Im definitely happy to look into the type issue and put up a PR. The cache issue (if it is an issue) may be over my head. If anyone else here that has worked on the core codebase has any thoughts on the issue in the codesandbox, additional input would be appreciated

ghost avatar Mar 23 '22 04:03 ghost

I dont have a ton of time until probs end of the week. Im definitely happy to look into the type issue and put up a PR. The cache issue (if it is an issue) may be over my head. If anyone else here that has worked on the core codebase has any thoughts on the issue in the codesandbox, additional input would be appreciated

The cache issue, maybe is not an issue, can be fixed when you run "getEscrowsByCategory.clearCache()" as well as "getEscrowsByCategory.memoizedResultFunc.clearCache()" before "getEscrowsByCategory(state, "category");":

` getEscrowsByCategory.clearCache(); // @ts-ignore getEscrowsByCategory.memoizedResultFunc.clearCache(); getEscrowsByCategory(state, "category");

console.log( "You could get the correct count you want", getEscrowsByCategory.recomputations() === 1 ); `

Because when getEscrowsByCategory runs, it will check the cache both in selector and memoizedResultFunc.

I'm sorry that I'm not good at English, hope you can understand what I'm saying.

yuningjiang123 avatar Jul 06 '22 06:07 yuningjiang123

I think this one can be closed, just need to call selector.clearCache() and selector.MemoizedResultFunc.clearCache().

aryaemami59 avatar Mar 16 '24 18:03 aryaemami59