From 393b11100b67f39d520bd5a33733918705646185 Mon Sep 17 00:00:00 2001 From: Erik Funder Carstensen Date: Mon, 16 Dec 2024 12:27:11 +0100 Subject: [PATCH 1/9] fix(core): bad signature for Dataset.reduce BREAKING CHANGE: Dataset.request return type now correctly reflects that `undefined` may be returned if called on an empty Dataset without the `memo` argument. As calling it without a `memo` argument already lead to an errorinous type error, users probably already have type casts around it in this situation, but if not, the new case can be ignored by asserting that there will be a value. Before: const res = dataset.reduce( async (memo, item, index) => { // example impl return item; } ); After: const res = dataset.reduce( async (memo, item, index) => { // example impl return item; } )!; Note that the original snippet was already rejected by TypeScript for calling reduce with too few arguments. The workaround to this was to pass `undefined` as the second argument, making TypeScript infer the type of `res` as `undefined`. This can be "fixed" in one of two ways: 1. In the DatasetReducer, casting the memo argument and the result of reduce to `Data`. - In this case no change is neccesary to the user code 2. Calling reduce with the memo argument `undefined as Data` - In this case the migration is neccesary --- packages/core/src/storages/dataset.ts | 36 +++++++++++++++++++-------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/packages/core/src/storages/dataset.ts b/packages/core/src/storages/dataset.ts index fe0528a96c57..9ead56714c4a 100644 --- a/packages/core/src/storages/dataset.ts +++ b/packages/core/src/storages/dataset.ts @@ -511,7 +511,8 @@ export class Dataset { * 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 no `memo` is passed to the initial invocation of reduce, the `iteratee()` is not invoked on the first element of the list. + * If no `memo` is passed to the initial invocation of reduce (or `memo` is set as `undefined`), + * 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. * * If `iteratee()` returns a `Promise` then it's awaited before a next call. @@ -520,19 +521,34 @@ export class Dataset { * @param memo Initial state of the reduction. * @param [options] All `reduce()` parameters. */ - async reduce(iteratee: DatasetReducer, memo: T, options: DatasetIteratorOptions = {}): Promise { + async reduce(iteratee: DatasetReducer): Promise; + async reduce( + iteratee: DatasetReducer, + memo: undefined, + options: DatasetIteratorOptions + ): Promise; + async reduce( + iteratee: DatasetReducer, + memo: T, + options: DatasetIteratorOptions + ): Promise; + async reduce( + iteratee: DatasetReducer, + memo?: T, + options: DatasetIteratorOptions = {} + ): Promise { checkStorageAccess(); - let currentMemo: T = memo; + let currentMemo: T | undefined = memo; const wrappedFunc: DatasetConsumer = 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); From cc2eb2e87de81992272bc0ff0cba8ffc213f2c73 Mon Sep 17 00:00:00 2001 From: Erik Funder Carstensen Date: Mon, 16 Dec 2024 18:37:49 +0100 Subject: [PATCH 2/9] fix(core) Dataset.reduce main signature return type --- packages/core/src/storages/dataset.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/storages/dataset.ts b/packages/core/src/storages/dataset.ts index 9ead56714c4a..eda7adddca6c 100644 --- a/packages/core/src/storages/dataset.ts +++ b/packages/core/src/storages/dataset.ts @@ -536,7 +536,7 @@ export class Dataset { iteratee: DatasetReducer, memo?: T, options: DatasetIteratorOptions = {} - ): Promise { + ): Promise { checkStorageAccess(); let currentMemo: T | undefined = memo; From a59b334fea8baadaf18db4f9458cb73de6abed16 Mon Sep 17 00:00:00 2001 From: Erik Funder Carstensen Date: Tue, 17 Dec 2024 13:26:06 +0100 Subject: [PATCH 3/9] chore(core) fix eslint errors --- packages/core/src/storages/dataset.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/core/src/storages/dataset.ts b/packages/core/src/storages/dataset.ts index eda7adddca6c..d8cf1080ff0e 100644 --- a/packages/core/src/storages/dataset.ts +++ b/packages/core/src/storages/dataset.ts @@ -522,20 +522,23 @@ export class Dataset { * @param [options] All `reduce()` parameters. */ async reduce(iteratee: DatasetReducer): Promise; + async reduce( iteratee: DatasetReducer, memo: undefined, - options: DatasetIteratorOptions + options: DatasetIteratorOptions, ): Promise; + async reduce( iteratee: DatasetReducer, memo: T, - options: DatasetIteratorOptions + options: DatasetIteratorOptions, ): Promise; + async reduce( iteratee: DatasetReducer, memo?: T, - options: DatasetIteratorOptions = {} + options: DatasetIteratorOptions = {}, ): Promise { checkStorageAccess(); From 895c1a700131b274af346f4185bbcf7df219fd21 Mon Sep 17 00:00:00 2001 From: Erik Funder Carstensen Date: Tue, 17 Dec 2024 14:48:06 +0100 Subject: [PATCH 4/9] fix(core) document all overloads of Dataset.reduce --- packages/core/src/storages/dataset.ts | 55 +++++++++++++++++++++------ 1 file changed, 44 insertions(+), 11 deletions(-) diff --git a/packages/core/src/storages/dataset.ts b/packages/core/src/storages/dataset.ts index d8cf1080ff0e..40a424db365c 100644 --- a/packages/core/src/storages/dataset.ts +++ b/packages/core/src/storages/dataset.ts @@ -140,7 +140,7 @@ export interface DatasetDataOptions { skipEmpty?: boolean; } -export interface DatasetExportOptions extends Omit {} +export interface DatasetExportOptions extends Omit { } export interface DatasetIteratorOptions extends Omit { @@ -507,28 +507,61 @@ export class Dataset { /** * 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. * - * 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 no `memo` is passed to the initial invocation of reduce (or `memo` is set as `undefined`), - * 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. - * - * If `iteratee()` returns a `Promise` then it's awaited before a next call. + * 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 Initial state of the reduction. - * @param [options] All `reduce()` parameters. */ async reduce(iteratee: DatasetReducer): Promise; + /** + * 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, memo: undefined, options: DatasetIteratorOptions, ): Promise; + /** + * 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] An object containing extra options for `reduce()` + */ async reduce( iteratee: DatasetReducer, memo: T, From 70e8084e484333edf574817ebc36c271bfe9fc17 Mon Sep 17 00:00:00 2001 From: Erik Funder Carstensen Date: Tue, 17 Dec 2024 14:51:32 +0100 Subject: [PATCH 5/9] chore(core) format --- packages/core/src/storages/dataset.ts | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/packages/core/src/storages/dataset.ts b/packages/core/src/storages/dataset.ts index 40a424db365c..3fabcab21d5b 100644 --- a/packages/core/src/storages/dataset.ts +++ b/packages/core/src/storages/dataset.ts @@ -140,7 +140,7 @@ export interface DatasetDataOptions { skipEmpty?: boolean; } -export interface DatasetExportOptions extends Omit { } +export interface DatasetExportOptions extends Omit {} export interface DatasetIteratorOptions extends Omit { @@ -507,7 +507,7 @@ export class Dataset { /** * 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. @@ -515,9 +515,9 @@ export class Dataset { * 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 `iteratee()` returns a `Promise` it's awaited before a next call. + * * If the dataset is empty, reduce will return undefined. * * @param iteratee @@ -526,7 +526,7 @@ export class Dataset { /** * 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. @@ -534,9 +534,9 @@ export class Dataset { * 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 `iteratee()` returns a `Promise` it's awaited before a next call. + * * If the dataset is empty, reduce will return undefined. * * @param iteratee @@ -562,11 +562,7 @@ export class Dataset { * @param memo Initial state of the reduction. * @param [options] An object containing extra options for `reduce()` */ - async reduce( - iteratee: DatasetReducer, - memo: T, - options: DatasetIteratorOptions, - ): Promise; + async reduce(iteratee: DatasetReducer, memo: T, options: DatasetIteratorOptions): Promise; async reduce( iteratee: DatasetReducer, From 56b50ac6e7fe27c0cd47f0a987b24a2dde677efb Mon Sep 17 00:00:00 2001 From: Erik Funder Carstensen Date: Tue, 17 Dec 2024 17:09:55 +0100 Subject: [PATCH 6/9] fix(core) Dataset.reduce last overload to be appropriately permissive @vladfrangu found a missing '?' in the signature --- packages/core/src/storages/dataset.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/storages/dataset.ts b/packages/core/src/storages/dataset.ts index 3fabcab21d5b..47438fe8491f 100644 --- a/packages/core/src/storages/dataset.ts +++ b/packages/core/src/storages/dataset.ts @@ -140,7 +140,7 @@ export interface DatasetDataOptions { skipEmpty?: boolean; } -export interface DatasetExportOptions extends Omit {} +export interface DatasetExportOptions extends Omit { } export interface DatasetIteratorOptions extends Omit { @@ -562,7 +562,7 @@ export class Dataset { * @param memo Initial state of the reduction. * @param [options] An object containing extra options for `reduce()` */ - async reduce(iteratee: DatasetReducer, memo: T, options: DatasetIteratorOptions): Promise; + async reduce(iteratee: DatasetReducer, memo: T, options?: DatasetIteratorOptions): Promise; async reduce( iteratee: DatasetReducer, From 7cbd7ba8a7a656240954e935cde20087126c1862 Mon Sep 17 00:00:00 2001 From: Erik Funder Carstensen Date: Tue, 17 Dec 2024 17:19:24 +0100 Subject: [PATCH 7/9] chore(core) run yarn format --- packages/core/src/storages/dataset.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/storages/dataset.ts b/packages/core/src/storages/dataset.ts index 47438fe8491f..95fcb046efce 100644 --- a/packages/core/src/storages/dataset.ts +++ b/packages/core/src/storages/dataset.ts @@ -140,7 +140,7 @@ export interface DatasetDataOptions { skipEmpty?: boolean; } -export interface DatasetExportOptions extends Omit { } +export interface DatasetExportOptions extends Omit {} export interface DatasetIteratorOptions extends Omit { From aefde9324b1a74468dd221cc6221b68e782b9808 Mon Sep 17 00:00:00 2001 From: Erik Funder Carstensen Date: Sat, 21 Dec 2024 17:37:41 +0100 Subject: [PATCH 8/9] fix(core) remove ts error expectation in dataset tests --- test/core/storages/dataset.test.ts | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/test/core/storages/dataset.test.ts b/test/core/storages/dataset.test.ts index 495ea68190f3..4b6bd756c708 100644 --- a/test/core/storages/dataset.test.ts +++ b/test/core/storages/dataset.test.ts @@ -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); }, - [], + new Array(), { limit: 2, }, @@ -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)); }, - [], + new Array(), { limit: 2, }, @@ -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, @@ -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]); }); }); From ac7679e76e3d9744152eab0b29c527ac7b5f3e16 Mon Sep 17 00:00:00 2001 From: Erik Funder Carstensen Date: Sat, 21 Dec 2024 18:45:08 +0100 Subject: [PATCH 9/9] fix(core) do cast instead of Array constructor to make eslint happy --- test/core/storages/dataset.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/core/storages/dataset.test.ts b/test/core/storages/dataset.test.ts index 4b6bd756c708..a00671c0600e 100644 --- a/test/core/storages/dataset.test.ts +++ b/test/core/storages/dataset.test.ts @@ -298,7 +298,7 @@ describe('dataset', () => { return memo.concat(item); }, - new Array(), + [] as unknown[], { limit: 2, }, @@ -324,7 +324,7 @@ describe('dataset', () => { return Promise.resolve(memo.concat(item)); }, - new Array(), + [] as unknown[], { limit: 2, },