-
Notifications
You must be signed in to change notification settings - Fork 208
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
[Draft] Memoize limit #281
base: master
Are you sure you want to change the base?
Conversation
Thanks for sharing your proposal! So this is really two enhancements? 1) limiting cache size 2) being able to cache by object key instead of string key. On (1) |
The improvement I had initially thought of only covered limiting cache size, but you are right that using Map we could use any kind of type as cache key.
|
Sorry for the delay! To be clear I think it's probably to restrict this PR to cache size limit. I like to keep just functions simple and with limited options :) |
@angus-c sorry for the delay! |
Hey @EvandroLG! I would prefer just to not use map at all (or a polyfill). The problem is I want to support pre-map environments while also keeping the code short and simple. So can we just limit the change to the cache size limit? |
Yes, we can change the implementation, but in the end we would use a solution using Object and Array, just like this fallback class, right? Or would you have another approach for this problem? |
Maybe hold onto this plan for a short while. I'm thinking of allowing ES code soon, once I move to all ESM |
My intention is to add an option to limit the maximum number of results being cached, this way we can guarantee that the in-memory object will never be storing more results than we want.
It's just a draft of the solution using
Map
. In the final solution, I think we should support environments that don't implementMap
. I would like to hear your thoughts before jumping to the final solution @angus-c :)