Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check that caching is enabled before using it #215

Merged
merged 1 commit into from Nov 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/__tests__/dataloader.test.js
Expand Up @@ -542,6 +542,19 @@ describe('Accepts options', () => {
]);
});

it('Does not interact with a cache when cache is disabled', () => {
const promiseX = Promise.resolve('X');
const cacheMap = new Map([ [ 'X', promiseX ] ]);
const [ identityLoader ] = idLoader({ cache: false, cacheMap });

identityLoader.prime('A', 'A');
expect(cacheMap.get('A')).toBe(undefined);
identityLoader.clear('X');
expect(cacheMap.get('X')).toBe(promiseX);
identityLoader.clearAll();
expect(cacheMap.get('X')).toBe(promiseX);
});

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