Repository: sindresorhus/mem
Pull request #36 (2019-05-10 03:00:25)

Change the default value of the `cachePromiseRejection` option to `true`
As a feature, this already feels weird because caching depends on this specific mechanism and not others like:

- should the function be cached if it returns `false`?
- should the function be cached if its call parameters include `{cache: false}`?

Having it avoid cache just because a promise is rejected feels arbitrary... but in a way I see that it's a flexible mechanism that allows any function (sync or async) to decide whether to avoid the cache.

That said, I don't think it should be a default, because:
- there's a chance that if something errors the first time, it will also error the second time (idempotency is a virtue);
- seen as a mechanism to avoid cache, it should be opt-in if needed, rather than an opaque behavior the developer didn't ask for.

sindresorhus: I can see arguments for having the default both `true` and `false`. Personally, the default I would expect is `true`. I'll need to think about this more. Would be useful with input from some more people.

// @LinusU 
szmarczak: I think it should be set to `true` because most async function aren't dynamic like Got. Decompressing content? Calculating big numbers? Sure. Downloading? Nah. So, if someone wanted to use it with Got then he would need to disable that.

Patch:
diff --git a/index.d.ts b/index.d.ts
index 7866255..172baa5 100644
--- a/index.d.ts
+++ b/index.d.ts
@@ -20,7 +20,7 @@ declare namespace mem {
 		readonly maxAge?: number;
 
 		/**
-		Determines the cache key for storing the result based on the function arguments. By default, if there's only one argument and it's a [primitive](https://developer.mozilla.org/en-US/docs/Glossary/Primitive), it's used directly as a key, otherwise it's all the function arguments JSON stringified as an array.
+		Determines the cache key for storing the result based on the function arguments. By default, if there's only one argument and it's a [primitive](https://developer.mozilla.org/en-US/docs/Glossary/Primitive), it's used directly as a key (if it's a `function`, its reference will be used as key), otherwise it's all the function arguments JSON stringified as an array.
 
 		You could for example change it to only cache on the first argument `x => JSON.stringify(x)`.
 		*/
@@ -36,7 +36,7 @@ declare namespace mem {
 		/**
 		Cache rejected promises.
 
-		@default false
+		@default true
 		*/
 		readonly cachePromiseRejection?: boolean;
 	}
diff --git a/index.js b/index.js
index 51faf01..f76e983 100644
--- a/index.js
+++ b/index.js
@@ -12,11 +12,9 @@ const defaultCacheKey = (...arguments_) => {
 
 	if (arguments_.length === 1) {
 		const [firstArgument] = arguments_;
-		if (
-			firstArgument === null ||
-			firstArgument === undefined ||
-			(typeof firstArgument !== 'function' && typeof firstArgument !== 'object')
-		) {
+		const isObject = typeof firstArgument === 'object' && firstArgument !== null;
+		const isPrimitive = !isObject;
+		if (isPrimitive) {
 			return firstArgument;
 		}
 	}
@@ -28,7 +26,7 @@ const mem = (fn, options) => {
 	options = Object.assign({
 		cacheKey: defaultCacheKey,
 		cache: new Map(),
-		cachePromiseRejection: false
+		cachePromiseRejection: true
 	}, options);
 
 	if (typeof options.maxAge === 'number') {
diff --git a/readme.md b/readme.md
index add4222..e548979 100644
--- a/readme.md
+++ b/readme.md
@@ -101,7 +101,7 @@ Milliseconds until the cache expires.
 
 Type: `Function`
 
-Determines the cache key for storing the result based on the function arguments. By default, if there's only one argument and it's a [primitive](https://developer.mozilla.org/en-US/docs/Glossary/Primitive), it's used directly as a key, otherwise it's all the function arguments JSON stringified as an array.
+Determines the cache key for storing the result based on the function arguments. By default, if there's only one argument and it's a [primitive](https://developer.mozilla.org/en-US/docs/Glossary/Primitive), it's used directly as a key (if it's a `function`, its reference will be used as key), otherwise it's all the function arguments JSON stringified as an array.
 
 You could for example change it to only cache on the first argument `x => JSON.stringify(x)`.
 
@@ -115,7 +115,7 @@ Use a different cache storage. Must implement the following methods: `.has(key)`
 ##### cachePromiseRejection
 
 Type: `boolean`<br>
-Default: `false`
+Default: `true`
 
 Cache rejected promises.
 


Test patch:
diff --git a/test.js b/test.js
index f48e4b7..a85db05 100644
--- a/test.js
+++ b/test.js
@@ -15,6 +15,20 @@ test('memoize', t => {
 	t.is(memoized('foo', 'bar'), 2);
 	t.is(memoized('foo', 'bar'), 2);
 	t.is(memoized('foo', 'bar'), 2);
+	t.is(memoized(1), 3);
+	t.is(memoized(1), 3);
+	t.is(memoized(null), 4);
+	t.is(memoized(null), 4);
+	t.is(memoized(undefined), 5);
+	t.is(memoized(undefined), 5);
+	t.is(memoized(fixture), 6);
+	t.is(memoized(fixture), 6);
+	t.is(memoized(true), 7);
+	t.is(memoized(true), 7);
+
+	// Ensure that functions are stored by reference and not by "value" (e.g. their `.toString()` representation)
+	t.is(memoized(() => i++), 8);
+	t.is(memoized(() => i++), 9);
 });
 
 test('memoize with multiple non-primitive arguments', t => {
@@ -148,7 +162,7 @@ test('promise support', async t => {
 	t.is(await memoized(10), 1);
 });
 
-test('do not cache rejected promises', async t => {
+test('cachePromiseRejection option', async t => {
 	let i = 0;
 	const memoized = mem(async () => {
 		i++;
@@ -158,6 +172,8 @@ test('do not cache rejected promises', async t => {
 		}
 
 		return i;
+	}, {
+		cachePromiseRejection: false
 	});
 
 	await t.throwsAsync(memoized(), 'foo bar');