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

Cache maxAge is evaluated wrong for async functions #81

Closed
krystofwoldrich opened this issue Jul 19, 2021 · 4 comments
Closed

Cache maxAge is evaluated wrong for async functions #81

krystofwoldrich opened this issue Jul 19, 2021 · 4 comments

Comments

@krystofwoldrich
Copy link

I've noticed, that when memoizing long lasting async functions, the maxAge is evaluated relative to the moment when function call started. But in case of sync functions it is evaluated relative to the moment the function was resolved.

Expected: Sync functions as is. Async functions (functions that return promise) maxAge should be evaluated relative to when promise is resolved.

Current: Async functions maxAge is evaluated relative to the moment when Promise is returned. (Not resolved, as I would expect!)

Fix: Dirty fix is add await, but then all memoized function will be async. Clean fix is to change beaviour base on function return type. If Promise then wait till it's resolved. Else as is now.

I really appriciate your work, it's great library, just I can't use it because of this unexpected behaviour. Or if I miss understood something and it's working correctly I'm in advance sorry.

@fregante
Copy link
Collaborator

fregante commented Jul 19, 2021

mem caches the result of the function, which in this case is just a (Promise) object. What the object does isn't important as far as mem is concerned. What you're looking for is p-memoize, which wraps mem. You can probably open an issue there (or hope that Sindre can transfer this one to that repo first)

@krystofwoldrich
Copy link
Author

I understand that the function returns Promise so the behaviour is correct for sync code. But not for async, where unresolved Promise is just "not finished" function.

And since p-memoize is build on top of mem the same unexpected behaviour occours there too.

@fregante
Copy link
Collaborator

unresolved Promise is just "not finished" function.

No, Promises are objects that are returned immediately. mem stores what the function returns, which is an object. If you want Promise-aware memoization, p-memoize exists for that reason.

And since p-memoize is build on top of mem the same unexpected behaviour occours there too.

I didn't say that it works there, but that it can be asked/reported there as it would be more appropriate

@krystofwoldrich
Copy link
Author

Okay. Then the mention in doc about works fine with async fucntions should dissapear.

I was asking about this, because in JS async functions and functions returning Promise, could be looked at as the same. But their intention is very different (in one promsie is the result, in one the resolved Promise is the wanted result), I think you would agree. But this library takes it as the same.

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

2 participants