-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
How to deal with functions taking dict args? #120
Comments
@dheera I've faced a similar problem today when solving a data inconsistency bug, what I understand is that this library does not honor dict args however I was able to fix it by providing @medikoo can you please shine some light here? Is this package actively maintained? If I open a PR to fix this would you able to review/approve and rerelease? |
@dheera @dankolesnikov I'm not sure how I missed this issue (btw, yes this package is maintained). Sorry for responding so late Yes, currently there's no dedicated option for that, but it can be achieved relatively simply. See: https://github.com/medikoo/memoizee/blob/master/README.md#memoizing-on-dictionary-object-hash-arguments |
thank you so much for a quick response @medikoo !
Additionally, @dheera @medikoo I've realized that the cause for my bug stemmed from the fact that @medikoo The problem I face is that I am dealing with auto-generated code and I don't have good control of the arguments (and their ordering) of the functions I pass into |
I think it might be nice to support it out of the box, but when we speak of well designed generic solution, it may appear it's not trival to implement, e.g. we may want to go beyond JSON, to e.g. support well types as BigInt, etc. or recognise properly non JSON objects in collection (while after JSON stringification they're just cleared). You've already noticed that plain simple solution as proposed in docs, is too optimistic for your case So main problems I see is:
Sorry, I don't understand this question
In that case I believe you need to normalize against whole |
@medikoo Thank you so much for quick replies! |
I see. If function signature is |
Functions that take a dictionary of named arguments is extremely common in JS, for example the entire AWS API does it:
Memoizing these functions using non-primitive mode results in effectively this comparison after an intended cache hit:
which even though the arguments are exactly the same, returns false because "===" does NOT do deep comparisons on dictionaries and only compares the pointers to the dictionaries, which are different here.
Using primitive mode doesn't work either, since it results in effectively this comparison (based on primitive.js lines 8-18):
which evaluates to
which although returns true, it is not what we want, because it will evaluate any two dictionaries as equal even if their contents are different.
What I really want is a deep compare, and I think that can be effectively accomplished for serializable arguments by using JSON.stringify instead of Array.prototype.join, which will serialize the dictionaries to strings instead of converting them to the literal string "[object Object]".
Let me know if you agree and I'll be happy to create a pull request, or let me know if you already have plans to resolve this use case another way.
The text was updated successfully, but these errors were encountered: