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

Doesn't memoize the result for keys returning undefined #66

Open
hargasinski opened this issue Mar 8, 2018 · 4 comments
Open

Doesn't memoize the result for keys returning undefined #66

hargasinski opened this issue Mar 8, 2018 · 4 comments

Comments

@hargasinski
Copy link

hargasinski commented Mar 8, 2018

When the memoized function returns undefined for a specific parameter, another call to the memoized function runs the original function again as opposed to fetching the result from the cache.

Reproducible Example:

const memoize = require('fast-memoize')
const assert = require('assert')

let numCalled = 0
function longProcess(arg) {
  ++numCalled
  return
}

// memoize result
const memoizedProcess = memoize(longProcess)
memoizedProcess("foo")

// get memoized result
memoizedProcess("foo")
assert(numCalled === 1)

It seems like this changed from v2.2.8 to v2.3.0. Is this the intended behaviour?

@hargasinski
Copy link
Author

I see how this could be the expected behaviour though, i.e. if the function returns undefined, it could mean something in the function failed and it should be run again, e.g. it run before a global object was initialised/some data wasn't fetched yet.

@qm3ster
Copy link

qm3ster commented Feb 25, 2019

@hargasinski have you benchmarked using a Map cache with .has() or .hasOwnProperty/in with the current null-prototype object cache?

@simonfan
Copy link

simonfan commented Jul 1, 2021

Hmmm, in my opinion this is an unexpected behaviour if not explicitly stated otherwise.

@thawsitt
Copy link

I agree that it should be documented somewhere.

I checked the source code (line 36) and looks like it calls the original function whenever the cached result is undefined.

(Usually, undefined means something is not in the cache yet, so it calls the original function and caches this result. However, this means it no longer memoizes if the original function actually returns undefined).

https://github.com/caiogondim/fast-memoize.js/blob/master/src/index.js#L36

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

4 participants