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

feat(typings): allow to reference the loader instance in the batch function #288

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
1 change: 0 additions & 1 deletion .flowconfig
Expand Up @@ -10,5 +10,4 @@
[libs]

[options]
suppress_comment=\\(.\\|\n\\)*\\$FlowExpectError
include_warnings=true
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -47,7 +47,7 @@
"babel-eslint": "10.0.3",
"coveralls": "3.0.7",
"eslint": "6.6.0",
"flow-bin": "0.112.0",
"flow-bin": "0.167.0",
"jest": "24.9.0",
"sane": "4.1.0"
}
Expand Down
27 changes: 14 additions & 13 deletions src/__tests__/abuse.test.js
Expand Up @@ -13,15 +13,15 @@ describe('Provides descriptive error messages for API abuse', () => {

it('Loader creation requires a function', () => {
expect(() => {
// $FlowExpectError
// $FlowExpectedError[incompatible-call]
new DataLoader(); // eslint-disable-line no-new
}).toThrow(
'DataLoader must be constructed with a function which accepts ' +
'Array<key> and returns Promise<Array<value>>, but got: undefined.'
);

expect(() => {
// $FlowExpectError
// $FlowExpectedError[prop-missing]
new DataLoader({}); // eslint-disable-line no-new
}).toThrow(
'DataLoader must be constructed with a function which accepts ' +
Expand All @@ -33,15 +33,15 @@ describe('Provides descriptive error messages for API abuse', () => {
const idLoader = new DataLoader<number, number>(async keys => keys);

expect(() => {
// $FlowExpectError
// $FlowExpectedError[incompatible-call]
idLoader.load();
}).toThrow(
'The loader.load() function must be called with a value, ' +
'but got: undefined.'
);

expect(() => {
// $FlowExpectError
// $FlowExpectedError[incompatible-call]
idLoader.load(null);
}).toThrow(
'The loader.load() function must be called with a value, ' +
Expand All @@ -58,15 +58,16 @@ describe('Provides descriptive error messages for API abuse', () => {
const idLoader = new DataLoader<number, number>(async keys => keys);

expect(() => {
// $FlowExpectError
// $FlowExpectedError[incompatible-call]
idLoader.loadMany();
}).toThrow(
'The loader.loadMany() function must be called with Array<key> ' +
'but got: undefined.'
);

expect(() => {
// $FlowExpectError
// $FlowExpectedError[incompatible-call]
// $FlowExpectedError[extra-arg]
idLoader.loadMany(1, 2, 3);
}).toThrow(
'The loader.loadMany() function must be called with Array<key> ' +
Expand All @@ -80,7 +81,7 @@ describe('Provides descriptive error messages for API abuse', () => {
});

it('Batch function must return a Promise, not null', async () => {
// $FlowExpectError
// $FlowExpectedError[incompatible-call]
const badLoader = new DataLoader<number, number>(() => null);

let caughtError;
Expand All @@ -99,7 +100,7 @@ describe('Provides descriptive error messages for API abuse', () => {

it('Batch function must return a Promise, not a value', async () => {
// Note: this is returning the keys directly, rather than a promise to keys.
// $FlowExpectError
// $FlowExpectedError[incompatible-call]
const badLoader = new DataLoader<number, number>(keys => keys);

let caughtError;
Expand All @@ -118,7 +119,7 @@ describe('Provides descriptive error messages for API abuse', () => {

it('Batch function must return a Promise of an Array, not null', async () => {
// Note: this resolves to undefined
// $FlowExpectError
// $FlowExpectedError[incompatible-call]
const badLoader = new DataLoader<number, number>(async () => null);

let caughtError;
Expand Down Expand Up @@ -162,9 +163,9 @@ describe('Provides descriptive error messages for API abuse', () => {
}

expect(() => {
// $FlowExpectError
const incompleteMap = new IncompleteMap();
const options = { cacheMap: incompleteMap };
// $FlowExpectedError[incompatible-call]
new DataLoader(async keys => keys, options); // eslint-disable-line no-new
}).toThrow(
'Custom cacheMap missing methods: set, delete, clear'
Expand All @@ -173,7 +174,7 @@ describe('Provides descriptive error messages for API abuse', () => {

it('Requires a number for maxBatchSize', () => {
expect(() =>
// $FlowExpectError
// $FlowExpectedError[incompatible-call]
new DataLoader(async keys => keys, { maxBatchSize: null })
).toThrow('maxBatchSize must be a positive number: null');
});
Expand All @@ -186,14 +187,14 @@ describe('Provides descriptive error messages for API abuse', () => {

it('Requires a function for cacheKeyFn', () => {
expect(() =>
// $FlowExpectError
// $FlowExpectedError[incompatible-call]
new DataLoader(async keys => keys, { cacheKeyFn: null })
).toThrow('cacheKeyFn must be a function: null');
});

it('Requires a function for batchScheduleFn', () => {
expect(() =>
// $FlowExpectError
// $FlowExpectedError[incompatible-call]
new DataLoader(async keys => keys, { batchScheduleFn: null })
).toThrow('batchScheduleFn must be a function: null');
});
Expand Down
3 changes: 2 additions & 1 deletion src/__tests__/dataloader.test.js
Expand Up @@ -34,7 +34,7 @@ describe('Primary API', () => {
});

it('references the loader as "this" in the batch function', async () => {
let that;
let that: DataLoader<number, number>;
const loader = new DataLoader<number, number>(async function (keys) {
that = this;
return keys;
Expand All @@ -50,6 +50,7 @@ describe('Primary API', () => {
let that;
const loader = new DataLoader<number, number>(async keys => keys, {
cacheKeyFn(key) {
// $FlowIgnore[object-this-reference]
that = this;
return key;
}
Expand Down
6 changes: 3 additions & 3 deletions src/index.d.ts
Expand Up @@ -17,7 +17,7 @@
*/
declare class DataLoader<K, V, C = K> {

constructor(batchLoadFn: DataLoader.BatchLoadFn<K, V>, options?: DataLoader.Options<K, V, C>);
constructor(batchLoadFn: DataLoader.BatchLoadFn<K, V, C>, options?: DataLoader.Options<K, V, C>);

/**
* Loads a key, returning a `Promise` for the value represented by that key.
Expand Down Expand Up @@ -70,8 +70,8 @@ declare namespace DataLoader {

// A Function, which when given an Array of keys, returns a Promise of an Array
// of values or Errors.
export type BatchLoadFn<K, V> =
(keys: ReadonlyArray<K>) => PromiseLike<ArrayLike<V | Error>>;
export type BatchLoadFn<K, V, C> =
(this: DataLoader<K, V, C>, keys: ReadonlyArray<K>) => PromiseLike<ArrayLike<V | Error>>;

// Optionally turn off batching or caching or provide a cache key function or a
// custom cache instance.
Expand Down
15 changes: 9 additions & 6 deletions src/index.js
Expand Up @@ -9,8 +9,8 @@

// A Function, which when given an Array of keys, returns a Promise of an Array
// of values or Errors.
export type BatchLoadFn<K, V> =
(keys: $ReadOnlyArray<K>) => Promise<$ReadOnlyArray<V | Error>>;
export type BatchLoadFn<K, V, C> =
(this: DataLoader<K, V, C>, keys: $ReadOnlyArray<K>) => Promise<$ReadOnlyArray<V | Error>>;

// Optionally turn off batching or caching or provide a cache key function or a
// custom cache instance.
Expand All @@ -24,12 +24,12 @@ export type Options<K, V, C = K> = {
};

// If a custom cache is provided, it must be of this type (a subset of ES6 Map).
export type CacheMap<K, V> = {
export interface CacheMap<K, V> {
get(key: K): V | void;
set(key: K, value: V): any;
delete(key: K): any;
clear(): any;
};
}

/**
* A `DataLoader` creates a public API for loading data from a particular
Expand All @@ -43,7 +43,7 @@ export type CacheMap<K, V> = {
*/
class DataLoader<K, V, C = K> {
constructor(
batchLoadFn: BatchLoadFn<K, V>,
batchLoadFn: BatchLoadFn<K, V, C>,
options?: Options<K, V, C>
) {
if (typeof batchLoadFn !== 'function') {
Expand All @@ -61,7 +61,7 @@ class DataLoader<K, V, C = K> {
}

// Private
_batchLoadFn: BatchLoadFn<K, V>;
_batchLoadFn: BatchLoadFn<K, V, C>;
_maxBatchSize: number;
_batchScheduleFn: (() => void) => void;
_cacheKeyFn: K => C;
Expand Down Expand Up @@ -303,6 +303,7 @@ function dispatchBatch<K, V>(
var batchPromise = loader._batchLoadFn(batch.keys);

// Assert the expected response from batchLoadFn
// $FlowIgnore[method-unbinding]
if (!batchPromise || typeof batchPromise.then !== 'function') {
return failedDispatch(loader, batch, new TypeError(
'DataLoader must be constructed with a function which accepts ' +
Expand Down Expand Up @@ -435,6 +436,7 @@ function getValidCacheMap<K, V, C>(
if (cacheMap !== null) {
var cacheFunctions = [ 'get', 'set', 'delete', 'clear' ];
var missingFunctions = cacheFunctions
// $FlowIgnore[prop-missing]
.filter(fnName => cacheMap && typeof cacheMap[fnName] !== 'function');
if (missingFunctions.length !== 0) {
throw new TypeError(
Expand All @@ -452,6 +454,7 @@ function isArrayLike(x: mixed): boolean {
x !== null &&
typeof x.length === 'number' &&
(x.length === 0 ||
// $FlowIgnore[method-unbinding]
(x.length > 0 && Object.prototype.hasOwnProperty.call(x, x.length - 1)))
);
}
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
Expand Up @@ -2010,10 +2010,10 @@ flatted@^2.0.0:
resolved "https://registry.yarnpkg.com/flatted/-/flatted-2.0.1.tgz#69e57caa8f0eacbc281d2e2cb458d46fdb449e08"
integrity sha512-a1hQMktqW9Nmqr5aktAux3JMNqaucxGcjtjWnZLHX7yyPCmlSV3M54nGYbqT8K+0GhF3NBgmJCc3ma+WOgX8Jg==

flow-bin@0.112.0:
version "0.112.0"
resolved "https://registry.yarnpkg.com/flow-bin/-/flow-bin-0.112.0.tgz#6a21c31937c4a2f23a750056a364c598a95ea216"
integrity sha512-vdcuKv0UU55vjv0e2EVh1ZxlU+TSNT19SkE+6gT1vYzTKtzYE6dLuAmBIiS3Rg2N9D9HOI6TKSyl53zPtqZLrA==
flow-bin@0.167.0:
version "0.167.0"
resolved "https://registry.yarnpkg.com/flow-bin/-/flow-bin-0.167.0.tgz#65f42d962707b015f6165b2f3536c4a8701fa929"
integrity sha512-/J64i+BN1Y8xzpdzRsML/Mij/k2G1d0UGUnMBUxN8ALrClPTQOxsOgDlh9wpEzNM/5yY7iu6eQYw8TnLbw6H5Q==

for-in@^1.0.2:
version "1.0.2"
Expand Down