Skip to content

Commit 7521777

Browse files
authored
Check that caching is enabled before using it (#215)
1 parent 3c29974 commit 7521777

File tree

2 files changed

+47
-22
lines changed

2 files changed

+47
-22
lines changed

src/__tests__/dataloader.test.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,19 @@ describe('Accepts options', () => {
542542
]);
543543
});
544544

545+
it('Does not interact with a cache when cache is disabled', () => {
546+
const promiseX = Promise.resolve('X');
547+
const cacheMap = new Map([ [ 'X', promiseX ] ]);
548+
const [ identityLoader ] = idLoader({ cache: false, cacheMap });
549+
550+
identityLoader.prime('A', 'A');
551+
expect(cacheMap.get('A')).toBe(undefined);
552+
identityLoader.clear('X');
553+
expect(cacheMap.get('X')).toBe(promiseX);
554+
identityLoader.clearAll();
555+
expect(cacheMap.get('X')).toBe(promiseX);
556+
});
557+
545558
it('Complex cache behavior via clearAll()', async () => {
546559
// This loader clears its cache as soon as a batch function is dispatched.
547560
const loadCalls = [];

src/index.js

Lines changed: 34 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ class DataLoader<K, V, C = K> {
6060
// Private
6161
_batchLoadFn: BatchLoadFn<K, V>;
6262
_options: ?Options<K, V, C>;
63-
_promiseCache: CacheMap<C, Promise<V>>;
63+
_promiseCache: ?CacheMap<C, Promise<V>>;
6464
_queue: LoaderQueue<K, V>;
6565

6666
/**
@@ -77,12 +77,12 @@ class DataLoader<K, V, C = K> {
7777
// Determine options
7878
var options = this._options;
7979
var shouldBatch = !options || options.batch !== false;
80-
var shouldCache = !options || options.cache !== false;
80+
var cache = this._promiseCache;
8181
var cacheKey = getCacheKey(options, key);
8282

8383
// If caching and there is a cache-hit, return cached Promise.
84-
if (shouldCache) {
85-
var cachedPromise = this._promiseCache.get(cacheKey);
84+
if (cache) {
85+
var cachedPromise = cache.get(cacheKey);
8686
if (cachedPromise) {
8787
return cachedPromise;
8888
}
@@ -108,8 +108,8 @@ class DataLoader<K, V, C = K> {
108108
});
109109

110110
// If caching, cache this promise.
111-
if (shouldCache) {
112-
this._promiseCache.set(cacheKey, promise);
111+
if (cache) {
112+
cache.set(cacheKey, promise);
113113
}
114114

115115
return promise;
@@ -148,8 +148,11 @@ class DataLoader<K, V, C = K> {
148148
* method chaining.
149149
*/
150150
clear(key: K): this {
151-
var cacheKey = getCacheKey(this._options, key);
152-
this._promiseCache.delete(cacheKey);
151+
var cache = this._promiseCache;
152+
if (cache) {
153+
var cacheKey = getCacheKey(this._options, key);
154+
cache.delete(cacheKey);
155+
}
153156
return this;
154157
}
155158

@@ -159,7 +162,10 @@ class DataLoader<K, V, C = K> {
159162
* method chaining.
160163
*/
161164
clearAll(): this {
162-
this._promiseCache.clear();
165+
var cache = this._promiseCache;
166+
if (cache) {
167+
cache.clear();
168+
}
163169
return this;
164170
}
165171

@@ -168,19 +174,21 @@ class DataLoader<K, V, C = K> {
168174
* exists, no change is made. Returns itself for method chaining.
169175
*/
170176
prime(key: K, value: V): this {
171-
var cacheKey = getCacheKey(this._options, key);
172-
173-
// Only add the key if it does not already exist.
174-
if (this._promiseCache.get(cacheKey) === undefined) {
175-
// Cache a rejected promise if the value is an Error, in order to match
176-
// the behavior of load(key).
177-
var promise = value instanceof Error ?
178-
Promise.reject(value) :
179-
Promise.resolve(value);
180-
181-
this._promiseCache.set(cacheKey, promise);
177+
var cache = this._promiseCache;
178+
if (cache) {
179+
var cacheKey = getCacheKey(this._options, key);
180+
181+
// Only add the key if it does not already exist.
182+
if (cache.get(cacheKey) === undefined) {
183+
// Cache a rejected promise if the value is an Error, in order to match
184+
// the behavior of load(key).
185+
var promise = value instanceof Error ?
186+
Promise.reject(value) :
187+
Promise.resolve(value);
188+
189+
cache.set(cacheKey, promise);
190+
}
182191
}
183-
184192
return this;
185193
}
186194
}
@@ -327,7 +335,11 @@ function getCacheKey<K, V, C>(
327335
// Private: given the DataLoader's options, produce a CacheMap to be used.
328336
function getValidCacheMap<K, V, C>(
329337
options: ?Options<K, V, C>
330-
): CacheMap<C, Promise<V>> {
338+
): ?CacheMap<C, Promise<V>> {
339+
var shouldCache = !options || options.cache !== false;
340+
if (!shouldCache) {
341+
return null;
342+
}
331343
var cacheMap = options && options.cacheMap;
332344
if (!cacheMap) {
333345
return new Map();

0 commit comments

Comments
 (0)