Skip to content

Commit

Permalink
fix(core): type definition of Dataset.reduce (#2774)
Browse files Browse the repository at this point in the history
Fixes #2773 

I'm seeing type errors and test failures in
`./test/core/sitemap_request_list.test.ts` both before and after my
change.

This PR currently doesn't add tests to check that types resolve
correctly. In the `reduce() uses first value as memo if no memo is
provided` test, the result from reduce was previously inferred to be
`any`, and is now inferred to be `Dictionary`. Adding a type annotation
doesn't help catch this change as implicit any is allowed.

---------

Co-authored-by: Erik Funder Carstensen <[email protected]>
  • Loading branch information
halvko and Erik Funder Carstensen authored Dec 21, 2024
1 parent 49044d4 commit 59bc6d1
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 22 deletions.
76 changes: 62 additions & 14 deletions packages/core/src/storages/dataset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -508,31 +508,79 @@ export class Dataset<Data extends Dictionary = Dictionary> {
/**
* Reduces a list of values down to a single value.
*
* Memo is the initial state of the reduction, and each successive step of it should be returned by `iteratee()`.
* The `iteratee()` is passed three arguments: the `memo`, then the `value` and `index` of the iteration.
* The first element of the dataset is the initial value, with each successive reductions should
* be returned by `iteratee()`. The `iteratee()` is passed three arguments: the `memo`, `value`
* and `index` of the current element being folded into the reduction.
*
* If no `memo` is passed to the initial invocation of reduce, the `iteratee()` is not invoked on the first element of the list.
* The first element is instead passed as the memo in the invocation of the `iteratee()` on the next element in the list.
* The `iteratee` is first invoked on the second element of the list (`index = 1`), with the
* first element given as the memo parameter. After that, the rest of the elements in the
* dataset is passed to `iteratee`, with the result of the previous invocation as the memo.
*
* If `iteratee()` returns a `Promise` it's awaited before a next call.
*
* If the dataset is empty, reduce will return undefined.
*
* @param iteratee
*/
async reduce(iteratee: DatasetReducer<Data, Data>): Promise<Data | undefined>;

/**
* Reduces a list of values down to a single value.
*
* The first element of the dataset is the initial value, with each successive reductions should
* be returned by `iteratee()`. The `iteratee()` is passed three arguments: the `memo`, `value`
* and `index` of the current element being folded into the reduction.
*
* The `iteratee` is first invoked on the second element of the list (`index = 1`), with the
* first element given as the memo parameter. After that, the rest of the elements in the
* dataset is passed to `iteratee`, with the result of the previous invocation as the memo.
*
* If `iteratee()` returns a `Promise` it's awaited before a next call.
*
* If the dataset is empty, reduce will return undefined.
*
* @param iteratee
* @param memo Unset parameter, neccesary to be able to pass options
* @param [options] An object containing extra options for `reduce()`
*/
async reduce(
iteratee: DatasetReducer<Data, Data>,
memo: undefined,
options: DatasetIteratorOptions,
): Promise<Data | undefined>;

/**
* Reduces a list of values down to a single value.
*
* Memo is the initial state of the reduction, and each successive step of it should be returned
* by `iteratee()`. The `iteratee()` is passed three arguments: the `memo`, then the `value` and
* `index` of the iteration.
*
* If `iteratee()` returns a `Promise` then it's awaited before a next call.
*
* @param iteratee
* @param memo Initial state of the reduction.
* @param [options] All `reduce()` parameters.
* @param [options] An object containing extra options for `reduce()`
*/
async reduce<T>(iteratee: DatasetReducer<T, Data>, memo: T, options: DatasetIteratorOptions = {}): Promise<T> {
async reduce<T>(iteratee: DatasetReducer<T, Data>, memo: T, options?: DatasetIteratorOptions): Promise<T>;

async reduce<T = Data>(
iteratee: DatasetReducer<T, Data>,
memo?: T,
options: DatasetIteratorOptions = {},
): Promise<T | undefined> {
checkStorageAccess();

let currentMemo: T = memo;
let currentMemo: T | undefined = memo;

const wrappedFunc: DatasetConsumer<Data> = async (item, index) => {
return Promise.resolve()
.then(() => {
return !index && currentMemo === undefined ? item : iteratee(currentMemo, item, index);
})
.then((newMemo) => {
currentMemo = newMemo as T;
});
if (index === 0 && currentMemo === undefined) {
currentMemo = item;
} else {
// We are guaranteed that currentMemo is instanciated, since we are either not on
// the first iteration, or memo was already set by the user.
currentMemo = await iteratee(currentMemo as T, item, index);
}
};

await this.forEach(wrappedFunc, options);
Expand Down
11 changes: 3 additions & 8 deletions test/core/storages/dataset.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,10 +296,9 @@ describe('dataset', () => {
item.index = index;
item.bar = 'xxx';

// @ts-expect-error FIXME the inference is broken for `reduce()` method
return memo.concat(item);
},
[],
[] as unknown[],
{
limit: 2,
},
Expand All @@ -323,10 +322,9 @@ describe('dataset', () => {
item.index = index;
item.bar = 'xxx';

// @ts-expect-error FIXME the inference is broken for `reduce()` method
return Promise.resolve(memo.concat(item));
},
[],
[] as unknown[],
{
limit: 2,
},
Expand Down Expand Up @@ -369,10 +367,8 @@ describe('dataset', () => {
const calledForIndexes: number[] = [];

const result = await dataset.reduce(
// @ts-expect-error FIXME the inference is broken for `reduce()` method
async (memo, item, index) => {
calledForIndexes.push(index);
// @ts-expect-error FIXME the inference is broken for `reduce()` method
return Promise.resolve(memo.foo > item.foo ? memo : item);
},
undefined,
Expand All @@ -391,8 +387,7 @@ describe('dataset', () => {
offset: 2,
});

// @ts-expect-error FIXME the inference is broken for `reduce()` method
expect(result.foo).toBe(5);
expect(result!.foo).toBe(5);
expect(calledForIndexes).toEqual([1, 2, 3]);
});
});
Expand Down

0 comments on commit 59bc6d1

Please sign in to comment.