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

[BREAKING] loadMany() returns individual Error instead of rejecting promise #216

Merged
merged 1 commit into from Nov 15, 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
11 changes: 10 additions & 1 deletion README.md
Expand Up @@ -314,7 +314,7 @@ Loads multiple keys, promising an array of values:
const [ a, b ] = await myLoader.loadMany([ 'a', 'b' ])
```

This is equivalent to the more verbose:
This is similar to the more verbose:

```js
const [ a, b ] = await Promise.all([
Expand All @@ -323,6 +323,15 @@ const [ a, b ] = await Promise.all([
])
```

However it is different in the case where any load fails. Where
Promise.all() would reject, loadMany() always resolves, however each result
is either a value or an Error instance.

```js
var [ a, b, c ] = await myLoader.loadMany([ 'a', 'b', 'badkey' ]);
// c instanceof Error
```

- *keys*: An array of key values to load.

##### `clear(key)`
Expand Down
14 changes: 14 additions & 0 deletions src/__tests__/dataloader.test.js
Expand Up @@ -62,6 +62,20 @@ describe('Primary API', () => {
expect(empty).toEqual([]);
});

it('supports loading multiple keys in one call with errors', async () => {
const identityLoader = new DataLoader(keys =>
Promise.resolve(
keys.map(key => (key === 'bad' ? new Error('Bad Key') : key))
)
);

const promiseAll = identityLoader.loadMany([ 'a', 'b', 'bad' ]);
expect(promiseAll).toBeInstanceOf(Promise);

const values = await promiseAll;
expect(values).toEqual([ 'a', 'b', new Error('Bad Key') ]);
});

it('batches multiple requests', async () => {
const [ identityLoader, loadCalls ] = idLoader<number>();

Expand Down
2 changes: 1 addition & 1 deletion src/index.d.ts
Expand Up @@ -37,7 +37,7 @@ declare class DataLoader<K, V, C = K> {
* ]);
*
*/
loadMany(keys: ArrayLike<K>): Promise<V[]>;
loadMany(keys: ArrayLike<K>): Promise<Array<V | Error>>;

/**
* Clears the value at `key` from the cache, if it exists. Returns itself for
Expand Down
13 changes: 10 additions & 3 deletions src/index.js
Expand Up @@ -120,15 +120,22 @@ class DataLoader<K, V, C = K> {
*
* var [ a, b ] = await myLoader.loadMany([ 'a', 'b' ]);
*
* This is equivalent to the more verbose:
* This is similar to the more verbose:
*
* var [ a, b ] = await Promise.all([
* myLoader.load('a'),
* myLoader.load('b')
* ]);
*
* However it is different in the case where any load fails. Where
* Promise.all() would reject, loadMany() always resolves, however each result
* is either a value or an Error instance.
*
* var [ a, b, c ] = await myLoader.loadMany([ 'a', 'b', 'badkey' ]);
* // c instanceof Error
*
*/
loadMany(keys: $ReadOnlyArray<K>): Promise<Array<V>> {
loadMany(keys: $ReadOnlyArray<K>): Promise<Array<V | Error>> {
if (!isArrayLike(keys)) {
throw new TypeError(
'The loader.loadMany() function must be called with Array<key> ' +
Expand All @@ -138,7 +145,7 @@ class DataLoader<K, V, C = K> {
// Support ArrayLike by using only minimal property access
const loadPromises = [];
for (let i = 0; i < keys.length; i++) {
loadPromises.push(this.load(keys[i]));
loadPromises.push(this.load(keys[i]).catch(error => error));
}
return Promise.all(loadPromises);
}
Expand Down