perf: call cacheKeyFn only when it is needed (if caching) (#342)

* perf: call and reuse cacheKeyFn only when it is needed (if caching)

* test: add a test case for cacheKeyFn not being called

* chore: fix flow error

* docs: add changeset

Author: abendi

.changeset/smooth-crabs-lie.md

@@ -0,0 +1,5 @@
+---
+'dataloader': patch
+---
+
+Ensure `cacheKeyFn` is not called when caching is disabled, since the key is not utilized in that case.

src/__tests__/dataloader.test.js

@@ -750,6 +750,20 @@ describe('Accepts options', () => {
     expect(cacheMap.get('X')).toBe(promiseX);
   });
 
+  it('Does not call cacheKeyFn when cache is disabled', async () => {
+    const cacheKeyFnCalls = [];
+    const [identityLoader] = idLoader<string>({
+      cache: false,
+      cacheKeyFn: key => {
+        cacheKeyFnCalls.push(key);
+        return key;
+      },
+    });
+
+    await identityLoader.load('A');
+    expect(cacheKeyFnCalls).toEqual([]);
+  });
+
   it('Complex cache behavior via clearAll()', async () => {
     // This loader clears its cache as soon as a batch function is dispatched.
     const loadCalls = [];

src/index.js

@@ -81,10 +81,11 @@ class DataLoader<K, V, C = K> {
 
     const batch = getCurrentBatch(this);
     const cacheMap = this._cacheMap;
-    const cacheKey = this._cacheKeyFn(key);
+    let cacheKey: ?C;
 
     // If caching and there is a cache-hit, return cached Promise.
     if (cacheMap) {
+      cacheKey = this._cacheKeyFn(key);
       const cachedPromise = cacheMap.get(cacheKey);
       if (cachedPromise) {
         const cacheHits = batch.cacheHits || (batch.cacheHits = []);
@@ -105,7 +106,7 @@ class DataLoader<K, V, C = K> {
 
     // If caching, cache this promise.
     if (cacheMap) {
-      cacheMap.set(cacheKey, promise);
+      cacheMap.set((cacheKey: any), promise);
     }
 
     return promise;