From 2e2dd37660a80aa3c5274b36467316338d234025 Mon Sep 17 00:00:00 2001 From: PopGoesTheWza Date: Fri, 26 Feb 2021 12:23:38 +0100 Subject: [PATCH 01/11] feat: `stackAllItems` default to `false` BREAKING CHANGE --- readme.md | 8 ++++---- source/as-promise/types.ts | 6 +++--- source/index.ts | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/readme.md b/readme.md index 7fe4c2422..54e2b00ef 100644 --- a/readme.md +++ b/readme.md @@ -934,7 +934,7 @@ Default: [`Link` header logic](source/index.ts) The function takes three arguments: - `response` - The current response object. -- `allItems` - An array of the emitted items. +- `allItems` - An array of the emitted items if [pagination.stackAllItems](#pagination.stackAllItems) is set to `true`. An empty array otherwise. - `currentItems` - Items from the current response. It should return an object representing Got options pointing to the next page. The options are merged automatically with the previous request, therefore the options returned `pagination.paginate(...)` must reflect changes only. If there are no more pages, `false` should be returned. @@ -1017,11 +1017,11 @@ For example, it can be helpful during development to avoid an infinite number of ###### pagination.stackAllItems Type: `boolean`\ -Default: `true` +Default: `false` -Defines how the parameter `allItems` in [pagination.paginate](#pagination.paginate), [pagination.filter](#pagination.filter) and [pagination.shouldContinue](#pagination.shouldContinue) is managed. When set to `false`, the parameter `allItems` is always an empty array. +Defines how the parameter `allItems` in [pagination.paginate](#pagination.paginate), [pagination.filter](#pagination.filter) and [pagination.shouldContinue](#pagination.shouldContinue) is managed. When set to `true`, the parameter `allItems` is an of the emitted items. -This option can be helpful to save on memory usage when working with a large dataset. +When set to `false`, the parameter `allItems` is always an empty array. This option can be helpful to save on memory usage when working with a large dataset. ##### localAddress diff --git a/source/as-promise/types.ts b/source/as-promise/types.ts index aa52695bd..31aa0db5d 100644 --- a/source/as-promise/types.ts +++ b/source/as-promise/types.ts @@ -34,7 +34,7 @@ export interface PaginationOptions { /** The function takes three arguments: - `response` - The current response object. - - `allItems` - An array of the emitted items. + - `allItems` - An array of the emitted items if `pagination.stackAllItems` is set to `true`. An empty array otherwise. - `currentItems` - Items from the current response. It should return an object representing Got options pointing to the next page. @@ -113,9 +113,9 @@ export interface PaginationOptions { /** Defines how the parameter `allItems` in pagination.paginate, pagination.filter and pagination.shouldContinue is managed. - When set to `false`, the parameter `allItems` is always an empty array. + When set to `true`, the parameter `allItems` is an of the emitted items. - This option can be helpful to save on memory usage when working with a large dataset. + When set to `false`, the parameter `allItems` is always an empty array. This option can be helpful to save on memory usage when working with a large dataset. */ stackAllItems?: boolean; }; diff --git a/source/index.ts b/source/index.ts index 9e9629ca3..debc5b226 100644 --- a/source/index.ts +++ b/source/index.ts @@ -113,7 +113,7 @@ const defaults: InstanceDefaults = { countLimit: Number.POSITIVE_INFINITY, backoff: 0, requestLimit: 10000, - stackAllItems: true + stackAllItems: false }, parseJson: (text: string) => JSON.parse(text), stringifyJson: (object: unknown) => JSON.stringify(object), From e0b9af2e2d6b6f41411cdec8744511d192630ecf Mon Sep 17 00:00:00 2001 From: PopGoesTheWza Date: Fri, 26 Feb 2021 14:33:40 +0100 Subject: [PATCH 02/11] test: fix broken case --- test/pagination.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/pagination.ts b/test/pagination.ts index 754e99888..3e08ae1e1 100644 --- a/test/pagination.ts +++ b/test/pagination.ts @@ -154,7 +154,8 @@ test('custom paginate function using allItems', withServer, async (t, server, go } return {path: '/?page=3'}; - } + }, + stackAllItems: true } }); From 027e70d90bcd4c05227709cc48b0ef67a155a943 Mon Sep 17 00:00:00 2001 From: PopGoesTheWza Date: Fri, 26 Feb 2021 17:37:43 +0100 Subject: [PATCH 03/11] feat: `allItems` goes last --- readme.md | 8 ++++---- source/as-promise/types.ts | 12 ++++++------ source/create.ts | 8 ++++---- test/pagination.ts | 10 +++++----- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/readme.md b/readme.md index af5a38f82..3e0c6b0b8 100644 --- a/readme.md +++ b/readme.md @@ -939,8 +939,8 @@ Default: [`Link` header logic](source/index.ts) The function takes an object with the following properties: - `response` - The current response object. -- `allItems` - An array of the emitted items if [pagination.stackAllItems](#pagination.stackAllItems) is set to `true`. An empty array otherwise. - `currentItems` - Items from the current response. +- `allItems` - An array of the emitted items if [pagination.stackAllItems](#pagination.stackAllItems) is set to `true`. An empty array otherwise. It should return an object representing Got options pointing to the next page. The options are merged automatically with the previous request, therefore the options returned `pagination.paginate(...)` must reflect changes only. If there are no more pages, `false` should be returned. @@ -958,7 +958,7 @@ const got = require('got'); offset: 0 }, pagination: { - paginate: ({response, allItems, currentItems}) => { + paginate: ({response, currentItems, allItems}) => { const previousSearchParams = response.request.options.searchParams; const previousOffset = previousSearchParams.get('offset'); @@ -983,14 +983,14 @@ const got = require('got'); ###### pagination.filter Type: `Function`\ -Default: `({item, allItems, currentItems}) => true` +Default: `({item, currentItems, allItems}) => true` Checks whether the item should be emitted or not. ###### pagination.shouldContinue Type: `Function`\ -Default: `({item, allItems, currentItems}) => true` +Default: `({item, currentItems, allItems}) => true` Checks whether the pagination should continue. diff --git a/source/as-promise/types.ts b/source/as-promise/types.ts index de188f338..af1e8a17b 100644 --- a/source/as-promise/types.ts +++ b/source/as-promise/types.ts @@ -13,14 +13,14 @@ export type ResponseType = 'json' | 'buffer' | 'text'; export interface PaginateData { response: Response; - allItems: ElementType[]; currentItems: ElementType[]; + allItems: ElementType[]; } export interface FilterData { item: ElementType; - allItems: ElementType[]; currentItems: ElementType[]; + allItems: ElementType[]; } export interface PaginationOptions { @@ -39,15 +39,15 @@ export interface PaginationOptions { /** Checks whether the item should be emitted or not. - @default ({item, allItems, currentItems}) => true + @default ({item, currentItems, allItems}) => true */ filter?: (data: FilterData) => boolean; /** The function takes an object with the following properties: - `response` - The current response object. - - `allItems` - An array of the emitted items if `pagination.stackAllItems` is set to `true`. An empty array otherwise. - `currentItems` - Items from the current response. + - `allItems` - An array of the emitted items if `pagination.stackAllItems` is set to `true`. An empty array otherwise. It should return an object representing Got options pointing to the next page. The options are merged automatically with the previous request, therefore the options returned `pagination.paginate(...)` must reflect changes only. @@ -66,7 +66,7 @@ export interface PaginationOptions { offset: 0 }, pagination: { - paginate: (response, allItems, currentItems) => { + paginate: ({response, currentItems}) => { const previousSearchParams = response.request.options.searchParams; const previousOffset = previousSearchParams.get('offset'); @@ -98,7 +98,7 @@ export interface PaginationOptions { If you want to stop **after** emitting the entry, you should use `({item, allItems}) => allItems.some(item => item.flag)` instead. - @default ({item, allItems, currentItems}) => true + @default ({item, currentItems, allItems}) => true */ shouldContinue?: (data: FilterData) => boolean; diff --git a/source/create.ts b/source/create.ts index 3a3de8b70..0da5e64af 100644 --- a/source/create.ts +++ b/source/create.ts @@ -245,8 +245,8 @@ const create = (defaults: InstanceDefaults): Got => { const currentItems: T[] = []; for (const item of parsed) { - if (pagination.filter({item, allItems, currentItems})) { - if (!pagination.shouldContinue({item, allItems, currentItems})) { + if (pagination.filter({item, currentItems, allItems})) { + if (!pagination.shouldContinue({item, currentItems, allItems})) { return; } @@ -266,8 +266,8 @@ const create = (defaults: InstanceDefaults): Got => { const optionsToMerge = pagination.paginate({ response, - allItems, - currentItems + currentItems, + allItems }); if (optionsToMerge === false) { diff --git a/test/pagination.ts b/test/pagination.ts index 5a7f028b9..6843d4bbb 100644 --- a/test/pagination.ts +++ b/test/pagination.ts @@ -85,7 +85,7 @@ test('filters elements', withServer, async (t, server, got) => { const result = await got.paginate.all({ pagination: { - filter: ({item, allItems, currentItems}) => { + filter: ({item, currentItems, allItems}) => { t.true(Array.isArray(allItems)); t.true(Array.isArray(currentItems)); @@ -209,7 +209,7 @@ test('`shouldContinue` works', withServer, async (t, server, got) => { const options = { pagination: { - shouldContinue: ({allItems, currentItems}: {allItems: number[]; currentItems: number[]}) => { + shouldContinue: ({currentItems, allItems}: {allItems: number[]; currentItems: number[]}) => { t.true(Array.isArray(allItems)); t.true(Array.isArray(currentItems)); @@ -496,11 +496,11 @@ test('`stackAllItems` set to true', withServer, async (t, server, got) => { return true; }, - paginate: ({response, allItems, currentItems}) => { + paginate: ({response, currentItems, allItems}) => { itemCount += 1; t.is(allItems.length, itemCount); - return got.defaults.options.pagination!.paginate({response, allItems, currentItems}); + return got.defaults.options.pagination!.paginate({response, currentItems, allItems}); } } }); @@ -524,7 +524,7 @@ test('`stackAllItems` set to false', withServer, async (t, server, got) => { return true; }, - paginate: ({response, allItems, currentItems}) => { + paginate: ({response, currentItems, allItems}) => { t.is(allItems.length, 0); return got.defaults.options.pagination!.paginate({response, allItems, currentItems}); From f39599899071ded527039f774344f8bb339b416d Mon Sep 17 00:00:00 2001 From: PopGoesTheWza Date: Fri, 26 Feb 2021 18:04:23 +0100 Subject: [PATCH 04/11] doc: clarify behavior --- readme.md | 9 ++++++--- source/as-promise/types.ts | 9 +++++---- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/readme.md b/readme.md index 3e0c6b0b8..39eb88b02 100644 --- a/readme.md +++ b/readme.md @@ -940,7 +940,7 @@ Default: [`Link` header logic](source/index.ts) The function takes an object with the following properties: - `response` - The current response object. - `currentItems` - Items from the current response. -- `allItems` - An array of the emitted items if [pagination.stackAllItems](#pagination.stackAllItems) is set to `true`. An empty array otherwise. +- `allItems` - An empty array, unless when [pagination.stackAllItems](#pagination.stackAllItems) is set to `true`. In the later case, an array of the emitted items. It should return an object representing Got options pointing to the next page. The options are merged automatically with the previous request, therefore the options returned `pagination.paginate(...)` must reflect changes only. If there are no more pages, `false` should be returned. @@ -1024,9 +1024,12 @@ For example, it can be helpful during development to avoid an infinite number of Type: `boolean`\ Default: `false` -Defines how the parameter `allItems` in [pagination.paginate](#pagination.paginate), [pagination.filter](#pagination.filter) and [pagination.shouldContinue](#pagination.shouldContinue) is managed. When set to `true`, the parameter `allItems` is an of the emitted items. +Defines how the property `allItems` in [pagination.paginate](#pagination.paginate), [pagination.filter](#pagination.filter) and [pagination.shouldContinue](#pagination.shouldContinue) is managed. + +By default, the property `allItems` is always an empty array. This setting can be helpful to save on memory usage when working with a large dataset. + +When set to `true`, the property `allItems` is an array of the emitted items. -When set to `false`, the parameter `allItems` is always an empty array. This option can be helpful to save on memory usage when working with a large dataset. ##### localAddress diff --git a/source/as-promise/types.ts b/source/as-promise/types.ts index af1e8a17b..56203bae1 100644 --- a/source/as-promise/types.ts +++ b/source/as-promise/types.ts @@ -47,7 +47,7 @@ export interface PaginationOptions { The function takes an object with the following properties: - `response` - The current response object. - `currentItems` - Items from the current response. - - `allItems` - An array of the emitted items if `pagination.stackAllItems` is set to `true`. An empty array otherwise. + - `allItems` - An empty array, unless when `pagination.stackAllItems` is set to `true`. In the later case, an array of the emitted items. It should return an object representing Got options pointing to the next page. The options are merged automatically with the previous request, therefore the options returned `pagination.paginate(...)` must reflect changes only. @@ -126,10 +126,11 @@ export interface PaginationOptions { requestLimit?: number; /** - Defines how the parameter `allItems` in pagination.paginate, pagination.filter and pagination.shouldContinue is managed. - When set to `true`, the parameter `allItems` is an of the emitted items. + Defines how the property `allItems` in pagination.paginate, pagination.filter and pagination.shouldContinue is managed. - When set to `false`, the parameter `allItems` is always an empty array. This option can be helpful to save on memory usage when working with a large dataset. + By default, the property `allItems` is always an empty array. This setting can be helpful to save on memory usage when working with a large dataset. + + When set to `true`, the property `allItems` is an array of the emitted items. */ stackAllItems?: boolean; }; From ca5d464f845f6680f261d0c58a6764630d456c71 Mon Sep 17 00:00:00 2001 From: PopGoesTheWza <32041843+PopGoesTheWza@users.noreply.github.com> Date: Fri, 26 Feb 2021 18:18:44 +0100 Subject: [PATCH 05/11] Update readme.md Co-authored-by: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> --- readme.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readme.md b/readme.md index 39eb88b02..d93d0ab1f 100644 --- a/readme.md +++ b/readme.md @@ -1028,7 +1028,7 @@ Defines how the property `allItems` in [pagination.paginate](#pagination.paginat By default, the property `allItems` is always an empty array. This setting can be helpful to save on memory usage when working with a large dataset. -When set to `true`, the property `allItems` is an array of the emitted items. +When set to `false`, the parameter `allItems` is always an empty array. ##### localAddress From 683693ec68a65d591df67a0f581502136a1f8d5d Mon Sep 17 00:00:00 2001 From: PopGoesTheWza <32041843+PopGoesTheWza@users.noreply.github.com> Date: Fri, 26 Feb 2021 18:19:00 +0100 Subject: [PATCH 06/11] Update readme.md Co-authored-by: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> --- readme.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readme.md b/readme.md index d93d0ab1f..87e935ce9 100644 --- a/readme.md +++ b/readme.md @@ -1024,7 +1024,7 @@ For example, it can be helpful during development to avoid an infinite number of Type: `boolean`\ Default: `false` -Defines how the property `allItems` in [pagination.paginate](#pagination.paginate), [pagination.filter](#pagination.filter) and [pagination.shouldContinue](#pagination.shouldContinue) is managed. +Defines how the property `allItems` in [`pagination.paginate`](#pagination.paginate), [`pagination.filter`](#pagination.filter) and [`pagination.shouldContinue`](#pagination.shouldContinue) is managed. By default, the property `allItems` is always an empty array. This setting can be helpful to save on memory usage when working with a large dataset. From 3dfeb221274decb9a660f24cf4ae0f7a211e8aa5 Mon Sep 17 00:00:00 2001 From: PopGoesTheWza <32041843+PopGoesTheWza@users.noreply.github.com> Date: Fri, 26 Feb 2021 18:19:08 +0100 Subject: [PATCH 07/11] Update readme.md Co-authored-by: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> --- readme.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readme.md b/readme.md index 87e935ce9..573e0272d 100644 --- a/readme.md +++ b/readme.md @@ -940,7 +940,7 @@ Default: [`Link` header logic](source/index.ts) The function takes an object with the following properties: - `response` - The current response object. - `currentItems` - Items from the current response. -- `allItems` - An empty array, unless when [pagination.stackAllItems](#pagination.stackAllItems) is set to `true`. In the later case, an array of the emitted items. +- `allItems` - An empty array, unless when [`pagination.stackAllItems`](#pagination.stackAllItems) is set to `true`. In the later case, an array of the emitted items. It should return an object representing Got options pointing to the next page. The options are merged automatically with the previous request, therefore the options returned `pagination.paginate(...)` must reflect changes only. If there are no more pages, `false` should be returned. From ec97764dec656a1dabea75a17bef479cad548365 Mon Sep 17 00:00:00 2001 From: PopGoesTheWza <32041843+PopGoesTheWza@users.noreply.github.com> Date: Fri, 26 Feb 2021 18:37:06 +0100 Subject: [PATCH 08/11] Update readme.md Co-authored-by: Sindre Sorhus --- readme.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readme.md b/readme.md index 573e0272d..8780b57cc 100644 --- a/readme.md +++ b/readme.md @@ -940,7 +940,7 @@ Default: [`Link` header logic](source/index.ts) The function takes an object with the following properties: - `response` - The current response object. - `currentItems` - Items from the current response. -- `allItems` - An empty array, unless when [`pagination.stackAllItems`](#pagination.stackAllItems) is set to `true`. In the later case, an array of the emitted items. +- `allItems` - An empty array, unless [`pagination.stackAllItems`](#pagination.stackAllItems) is set to `true`, in which case, it's an array of the emitted items. It should return an object representing Got options pointing to the next page. The options are merged automatically with the previous request, therefore the options returned `pagination.paginate(...)` must reflect changes only. If there are no more pages, `false` should be returned. From 01acfbf79e195ca6c824e6b15081c46d2a356ee3 Mon Sep 17 00:00:00 2001 From: PopGoesTheWza Date: Fri, 26 Feb 2021 18:41:21 +0100 Subject: [PATCH 09/11] doc: excuse my french --- readme.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/readme.md b/readme.md index 8780b57cc..47c8757ce 100644 --- a/readme.md +++ b/readme.md @@ -1026,9 +1026,9 @@ Default: `false` Defines how the property `allItems` in [`pagination.paginate`](#pagination.paginate), [`pagination.filter`](#pagination.filter) and [`pagination.shouldContinue`](#pagination.shouldContinue) is managed. -By default, the property `allItems` is always an empty array. This setting can be helpful to save on memory usage when working with a large dataset. +By default, the property `allItems` is always an empty array. -When set to `false`, the parameter `allItems` is always an empty array. +When set to `false`, the parameter `allItems` is always an empty array. This setting can impact memory usage when working with a large dataset. ##### localAddress From 4366927a3b32d0911984732c86983a6cac0161a8 Mon Sep 17 00:00:00 2001 From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> Date: Fri, 26 Feb 2021 19:10:02 +0100 Subject: [PATCH 10/11] Update readme.md --- readme.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readme.md b/readme.md index 47c8757ce..1d2f3aab4 100644 --- a/readme.md +++ b/readme.md @@ -1028,7 +1028,7 @@ Defines how the property `allItems` in [`pagination.paginate`](#pagination.pagin By default, the property `allItems` is always an empty array. -When set to `false`, the parameter `allItems` is always an empty array. This setting can impact memory usage when working with a large dataset. +When set to `false`, the `allItems` parameter is always an empty array. If `true`, it can hugely increase memory usage when working with a large dataset. ##### localAddress From 3cda93945f19af23d8aee3823f0f735d749ecbc1 Mon Sep 17 00:00:00 2001 From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> Date: Fri, 26 Feb 2021 19:10:59 +0100 Subject: [PATCH 11/11] Update readme.md --- readme.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/readme.md b/readme.md index 1d2f3aab4..bc2a0cb2f 100644 --- a/readme.md +++ b/readme.md @@ -1026,8 +1026,6 @@ Default: `false` Defines how the property `allItems` in [`pagination.paginate`](#pagination.paginate), [`pagination.filter`](#pagination.filter) and [`pagination.shouldContinue`](#pagination.shouldContinue) is managed. -By default, the property `allItems` is always an empty array. - When set to `false`, the `allItems` parameter is always an empty array. If `true`, it can hugely increase memory usage when working with a large dataset.