Skip to content

Commit

Permalink
Check that caching is enabled before using it
Browse files Browse the repository at this point in the history
  • Loading branch information
leebyron committed Nov 13, 2019
1 parent 68c8b14 commit 3850137
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 22 deletions.
9 changes: 9 additions & 0 deletions src/__tests__/dataloader.test.js
Expand Up @@ -542,6 +542,15 @@ describe('Accepts options', () => {
]);
});

it('Does not create a cache when cache is disabled', () => {
const [ identityLoader ] = idLoader({ cache: false });
expect(() => {
identityLoader.prime('A', 'A');
identityLoader.clear();
identityLoader.clearAll();
}).not.toThrow();
});

it('Complex cache behavior via clearAll()', async () => {
// This loader clears its cache as soon as a batch function is dispatched.
const loadCalls = [];
Expand Down
56 changes: 34 additions & 22 deletions src/index.js
Expand Up @@ -60,7 +60,7 @@ class DataLoader<K, V, C = K> {
// Private
_batchLoadFn: BatchLoadFn<K, V>;
_options: ?Options<K, V, C>;
_promiseCache: CacheMap<C, Promise<V>>;
_promiseCache: ?CacheMap<C, Promise<V>>;
_queue: LoaderQueue<K, V>;

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

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

// If caching, cache this promise.
if (shouldCache) {
this._promiseCache.set(cacheKey, promise);
if (cache) {
cache.set(cacheKey, promise);
}

return promise;
Expand Down Expand Up @@ -148,8 +148,11 @@ class DataLoader<K, V, C = K> {
* method chaining.
*/
clear(key: K): this {
var cacheKey = getCacheKey(this._options, key);
this._promiseCache.delete(cacheKey);
var cache = this._promiseCache;
if (cache) {
var cacheKey = getCacheKey(this._options, key);
cache.delete(cacheKey);
}
return this;
}

Expand All @@ -159,7 +162,10 @@ class DataLoader<K, V, C = K> {
* method chaining.
*/
clearAll(): this {
this._promiseCache.clear();
var cache = this._promiseCache;
if (cache) {
cache.clear();
}
return this;
}

Expand All @@ -168,19 +174,21 @@ class DataLoader<K, V, C = K> {
* exists, no change is made. Returns itself for method chaining.
*/
prime(key: K, value: V): this {
var cacheKey = getCacheKey(this._options, key);

// Only add the key if it does not already exist.
if (this._promiseCache.get(cacheKey) === undefined) {
// Cache a rejected promise if the value is an Error, in order to match
// the behavior of load(key).
var promise = value instanceof Error ?
Promise.reject(value) :
Promise.resolve(value);

this._promiseCache.set(cacheKey, promise);
var cache = this._promiseCache;
if (cache) {
var cacheKey = getCacheKey(this._options, key);

// Only add the key if it does not already exist.
if (cache.get(cacheKey) === undefined) {
// Cache a rejected promise if the value is an Error, in order to match
// the behavior of load(key).
var promise = value instanceof Error ?
Promise.reject(value) :
Promise.resolve(value);

cache.set(cacheKey, promise);
}
}

return this;
}
}
Expand Down Expand Up @@ -327,7 +335,11 @@ function getCacheKey<K, V, C>(
// Private: given the DataLoader's options, produce a CacheMap to be used.
function getValidCacheMap<K, V, C>(
options: ?Options<K, V, C>
): CacheMap<C, Promise<V>> {
): ?CacheMap<C, Promise<V>> {
var shouldCache = !options || options.cache !== false;
if (!shouldCache) {
return null;
}
var cacheMap = options && options.cacheMap;
if (!cacheMap) {
return new Map();
Expand Down

0 comments on commit 3850137

Please sign in to comment.