dai-shi / proxy-memoize

Intuitive magical memoization library with Proxy and WeakMap
http://proxy-memoize.js.org/
MIT License
715 stars 16 forks source link

memoizeWithArgs returning wrongly cached result #76

Closed IPWright83 closed 1 year ago

IPWright83 commented 1 year ago

I'm trying to use memoizeWithArgs and I'm finding that it's returning the wrong results - essentially returning results for a different set of args.

I'm calling it like so:

getScale: memoizeWithArgs((state: IState, field: string, type: "plot" | "brush") => {
  ... // returns a new function
}, { size: 50 });

It works to start with, returning correct values (see the [70, 760] in the watch window). image

Once a call with a different type (brush) is used, I correctly get different results: image

However each call there-after that has the same args as the first call (using "plot", now starts returning the cached result from the call using "brush").

image

I'm trying to debug it, but not having a huge amount of joy so far.

To reproduce the easiest way is to currently:

At this point you should be able to see the incorrect calls. You can verify it's being memoised (and not the selector logic) by placing breakpoints in chartSelectors lines 182 and 198.

dai-shi commented 1 year ago

Thanks for reporting. I don't think many people use memoizeWithArgs compared to memoize, so it can be a bug. If it's a bug, it should probably be reproducible with a few lines of code. Can anyone help reproducing it minimally from the provided repo?

IPWright83 commented 1 year ago

@dai-shi I'll give it a quick try and see if I can make any progress. Would you recommend using memoize and using an object instead then?

getScale: memoize(({ state: IState, field: string, type: "plot" | "brush" }) => {
dai-shi commented 1 year ago

If you look at the code, it's basically the same.

https://github.com/dai-shi/proxy-memoize/blob/9c157d73b2cf7cc6c3df745a43e1b7443b74022b/src/memoizeWithArgs.ts#L19-L28

IPWright83 commented 1 year ago

Not having much joy with a simple replication https://codesandbox.io/s/nifty-sea-pcycvm?file=/src/proxy-memoize.test.ts this seems to be working fine. Any more thoughts on how I might debug this further?

IPWright83 commented 1 year ago

Nevermind I think I'm being daft. I just realised that the 3rd party function I'm using underneath is stateful 🤦🏻‍♂️