Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clear lastResult between tests #569

Open
alissaVrk opened this issue Mar 15, 2022 · 10 comments
Open

clear lastResult between tests #569

alissaVrk opened this issue Mar 15, 2022 · 10 comments

Comments

@alissaVrk
Copy link

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

@ghost
Copy link

ghost commented Mar 23, 2022

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:

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()
    })

@markerikson
Copy link
Contributor

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

@ghost
Copy link

ghost commented Mar 23, 2022

@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:

memoizedResultFunc: Combiner

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

@markerikson
Copy link
Contributor

markerikson commented Mar 23, 2022

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().

@ghost
Copy link

ghost commented Mar 23, 2022

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
Copy link

ghost commented Mar 23, 2022

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; }

@markerikson
Copy link
Contributor

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.

@ghost
Copy link

ghost commented Mar 23, 2022

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

@yuningjiang123
Copy link

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.

@aryaemami59
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants