search

sindresorhus / p-memoize

Memoize promise-returning & async functions
MIT License
396 stars 30 forks source link

Add `cachePromiseRejection` option #13

Closed jdiamond closed 4 years ago

jdiamond commented 4 years ago

Hello, here is my attempt at adding the cachePromiseRejection option removed from mem.

I think the default should be false because rejected promises are like exceptions and an exception thrown from a function memoized with mem doesn't cache anything.

I used the same name that was used in mem, but it sounds awkward to me and wonder if cacheRejectedPromises might be more natural.

Fixes #11

jdiamond commented 4 years ago

Do you still want to support Node.js 6? I can use Object.assign() instead of the object spread operator. I was trying not to mutate the options argument that gets passed in.

sindresorhus commented 4 years ago

Do you still want to support Node.js 6?

No, fixed: https://github.com/sindresorhus/p-memoize/commit/d11b692353b196827cb8978672bda2a32386c2f8

jdiamond commented 4 years ago

Update to address your comments. Let me know if you'd rather I rebase/squash instead of merge.

sindresorhus commented 4 years ago

This looks good. Thank you 👍