Skip to content

Only cache if an object as a parameter is the SAME object (===), not if it has the same content #9

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

Closed
ChrisCinelli opened this issue Mar 14, 2017 · 8 comments

Comments

@ChrisCinelli
Copy link

ChrisCinelli commented Mar 14, 2017

It looks like that you only have a cache hit if an object in the parameters is === across calls.

const a = {a: 1};
const a2 = {a: 1};

const myMemoizedFunc = memoizerific(10000)(myFunc);

myMemoizedFunc(a);
// This is a cache miss
myMemoizedFunc(a2);

See this test: ChrisCinelli@b7083c1

@thinkloop
Copy link
Owner

thinkloop commented Mar 14, 2017

You are right. For complex objects to be considered equal they have to refer to the exact same references in memory. There is no performant or reliable way to softly compare complex objects in JavaScript. Consider circular objects that recursively refer back to themselves. That's why there's been a big movement recently to write immutable js.

@ChrisCinelli
Copy link
Author

The objects in the test above are not mutated objects.
memoizerific will not work when the parameters are objects.
For example a real use case is ajax(url, opt). To memoize you will use: const ajax = memoizerific(1000)(networkAjax).
If you have:
const getData = () => ajax(''https:/myserver/myendpoint", {timeout: 10000}), multiple requests to the getData will all call networkAjax .
This limits a lot the use of memoizerific.

@ChrisCinelli
Copy link
Author

You should at least document the behavior. Even better provide an interface similar to https://github.com/erikras/lru-memoize

@thinkloop
Copy link
Owner

thinkloop commented Mar 14, 2017

You should at least document the behavior

That's a good idea.

Even better provide an interface similar to https://github.com/erikras/lru-memoize

Also a reasonable idea, I will look into that, but:

const getData = () => ajax("https:/myserver/myendpoint", {timeout: 10000}), multiple requests to the getData will all call networkAjax

Isn't this easily worked around with:

const opts = {timeout: 10000};
const getData = () => ajax('https:/myserver/myendpoint', opts);

If I did implement custom comparators, there would be a performance hit for using them, risk of recursive objects leaking in, risk of bad compare functions being passed in (it's not easy making a good compare function, there are a lot of oddities like NaN !== NaN, etc) - using them should probably only be reserved for when absolutely necessary, it can be avoided in the aforementioned case.

@thinkloop
Copy link
Owner

I added documentation about it here: https://github.com/thinkloop/memoizerific#strict-equality

And created this issue: #10

@thinkloop
Copy link
Owner

New comment relevant to this discussion: #11 (comment)

@thinkloop
Copy link
Owner

Snippet from #11 (comment):

Another technique is to wrap the memoized function with another function that does any needed preprocessing. For example, say you wanted to memoize ajax(url, opts) where opts is an object of options. Typically this would be called with an inline object for the options like this: ajax(''https://domain.com", {timeout: 10000}) . Since a new object is created for opts on every invocation, it never gets cached. The solution is to wrap it with a function that breaks apart arguments as needed:

function callAjax(domain, timeout) {
  return ajax(domain, {timeout: timeout});
}

And now callAjax(''https://domain.com", 10000) is properly memoized. This allows for much finer grained control of exactly which arguments need to be transformed in what way. With normalizerFn you quickly start to bump into issues like blacklisting/whitelisting different properties to apply it to, etc.

@thinkloop
Copy link
Owner

thinkloop commented Aug 19, 2017

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