From a1b259cb747c449f6f6daf94b5ed24490fccf010 Mon Sep 17 00:00:00 2001 From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> Date: Fri, 26 Feb 2021 13:24:33 +0100 Subject: [PATCH 1/3] object --- readme.md | 4 ++-- source/as-promise/types.ts | 8 +++++++- source/create.ts | 28 ++++++++++++++++------------ source/index.ts | 2 +- test/hooks.ts | 2 +- test/pagination.ts | 25 +++++++++++++------------ 6 files changed, 40 insertions(+), 29 deletions(-) diff --git a/readme.md b/readme.md index 642c2912e..fb7043805 100644 --- a/readme.md +++ b/readme.md @@ -934,7 +934,7 @@ A function that transform [`Response`](#response) into an array of items. This i Type: `Function`\ Default: [`Link` header logic](source/index.ts) -The function takes three arguments: +The function takes an object with the following properties: - `response` - The current response object. - `allItems` - An array of the emitted items. - `currentItems` - Items from the current response. @@ -955,7 +955,7 @@ const got = require('got'); offset: 0 }, pagination: { - paginate: (response, allItems, currentItems) => { + paginate: ({response, allItems, currentItems}) => { const previousSearchParams = response.request.options.searchParams; const previousOffset = previousSearchParams.get('offset'); diff --git a/source/as-promise/types.ts b/source/as-promise/types.ts index aa52695bd..83564a0d5 100644 --- a/source/as-promise/types.ts +++ b/source/as-promise/types.ts @@ -11,6 +11,12 @@ All parsing methods supported by Got. */ export type ResponseType = 'json' | 'buffer' | 'text'; +export interface PaginateData { + response: Response; + allItems: T[]; + currentItems: T[]; +} + export interface PaginationOptions { /** All options accepted by `got.paginate()`. @@ -76,7 +82,7 @@ export interface PaginationOptions { })(); ``` */ - paginate?: (response: Response, allItems: T[], currentItems: T[]) => Options | false; + paginate?: (paginate: PaginateData) => Options | false; /** Checks whether the pagination should continue. diff --git a/source/create.ts b/source/create.ts index 77fbdaa73..9079d7228 100644 --- a/source/create.ts +++ b/source/create.ts @@ -225,7 +225,7 @@ const create = (defaults: InstanceDefaults): Got => { throw new TypeError('`options.pagination` must be implemented'); } - const all: T[] = []; + const allItems: T[] = []; let {countLimit} = pagination; let numberOfRequests = 0; @@ -236,27 +236,27 @@ const create = (defaults: InstanceDefaults): Got => { } // @ts-expect-error FIXME! - // TODO: Throw when result is not an instance of Response + // TODO: Throw when response is not an instance of Response // eslint-disable-next-line no-await-in-loop - const result = (await got(undefined, undefined, normalizedOptions)) as Response; + const response = (await got(undefined, undefined, normalizedOptions)) as Response; // eslint-disable-next-line no-await-in-loop - const parsed = await pagination.transform(result); - const current: T[] = []; + const parsed = await pagination.transform(response); + const currentItems: T[] = []; for (const item of parsed) { - if (pagination.filter(item, all, current)) { - if (!pagination.shouldContinue(item, all, current)) { + if (pagination.filter(item, allItems, currentItems)) { + if (!pagination.shouldContinue(item, allItems, currentItems)) { return; } yield item as T; if (pagination.stackAllItems) { - all.push(item as T); + allItems.push(item as T); } - current.push(item as T); + currentItems.push(item as T); if (--countLimit <= 0) { return; @@ -264,14 +264,18 @@ const create = (defaults: InstanceDefaults): Got => { } } - const optionsToMerge = pagination.paginate(result, all, current); + const optionsToMerge = pagination.paginate({ + response, + allItems, + currentItems + }); if (optionsToMerge === false) { return; } - if (optionsToMerge === result.request.options) { - normalizedOptions = result.request.options; + if (optionsToMerge === response.request.options) { + normalizedOptions = response.request.options; } else if (optionsToMerge !== undefined) { normalizedOptions = normalizeArguments(undefined, optionsToMerge, normalizedOptions); } diff --git a/source/index.ts b/source/index.ts index 42dce9311..3d33587aa 100644 --- a/source/index.ts +++ b/source/index.ts @@ -77,7 +77,7 @@ const defaults: InstanceDefaults = { return JSON.parse(response.body as string); }, - paginate: response => { + paginate: ({response}) => { if (!Reflect.has(response.headers, 'link')) { return false; } diff --git a/test/hooks.ts b/test/hooks.ts index 55c8d0b79..6a53eb02d 100644 --- a/test/hooks.ts +++ b/test/hooks.ts @@ -1208,7 +1208,7 @@ test('no duplicate hook calls when returning original request options', withServ }; // Test only two requests, one after another - const paginate = (response: Response) => requestNumber++ === 0 ? response.request.options : false; + const paginate = ({response}: {response: Response}) => requestNumber++ === 0 ? response.request.options : false; const instance = got.extend({ hooks, diff --git a/test/pagination.ts b/test/pagination.ts index 754e99888..96ec22bbb 100644 --- a/test/pagination.ts +++ b/test/pagination.ts @@ -126,7 +126,7 @@ test('custom paginate function', withServer, async (t, server, got) => { const result = await got.paginate.all({ pagination: { - paginate: response => { + paginate: ({response}) => { const url = new URL(response.url); if (url.search === '?page=3') { @@ -148,13 +148,14 @@ test('custom paginate function using allItems', withServer, async (t, server, go const result = await got.paginate.all({ pagination: { - paginate: (_response, allItems: number[]) => { + paginate: ({allItems}) => { if (allItems.length === 2) { return false; } return {path: '/?page=3'}; - } + }, + stackAllItems: true } }); @@ -166,7 +167,7 @@ test('custom paginate function using currentItems', withServer, async (t, server const result = await got.paginate.all({ pagination: { - paginate: (_response, _allItems: number[], currentItems: number[]) => { + paginate: ({currentItems}) => { if (currentItems[0] === 3) { return false; } @@ -357,7 +358,7 @@ test('`hooks` are not duplicated', withServer, async (t, server, got) => { const result = await got.paginate.all({ pagination: { - paginate: response => { + paginate: ({response}) => { if ((response.body as string) === '[3]') { return false; // Stop after page 3 } @@ -495,11 +496,11 @@ test('`stackAllItems` set to true', withServer, async (t, server, got) => { return true; }, - paginate: (response, allItems, currentItems) => { + paginate: ({response, allItems, currentItems}) => { itemCount += 1; t.is(allItems.length, itemCount); - return got.defaults.options.pagination!.paginate(response, allItems, currentItems); + return got.defaults.options.pagination!.paginate({response, allItems, currentItems}); } } }); @@ -523,10 +524,10 @@ test('`stackAllItems` set to false', withServer, async (t, server, got) => { return true; }, - paginate: (response, allItems, currentItems) => { + paginate: ({response, allItems, currentItems}) => { t.is(allItems.length, 0); - return got.defaults.options.pagination!.paginate(response, allItems, currentItems); + return got.defaults.options.pagination!.paginate({response, allItems, currentItems}); } } }); @@ -559,7 +560,7 @@ test('next url in json response', withServer, async (t, server, got) => { transform: (response: Response) => { return [response.body.currentUrl]; }, - paginate: (response: Response) => { + paginate: ({response}) => { const {next} = response.body; if (!next) { @@ -608,7 +609,7 @@ test('pagination using searchParams', withServer, async (t, server, got) => { transform: (response: Response) => { return [response.body.currentUrl]; }, - paginate: (response: Response) => { + paginate: ({response}) => { const {next} = response.body; const previousPage = Number(response.request.options.searchParams!.get('page')); @@ -664,7 +665,7 @@ test('pagination using extended searchParams', withServer, async (t, server, got transform: (response: Response) => { return [response.body.currentUrl]; }, - paginate: (response: Response) => { + paginate: ({response}) => { const {next} = response.body; const previousPage = Number(response.request.options.searchParams!.get('page')); From c4520aeab508c8f2efad86da3f1f732ba7d473d7 Mon Sep 17 00:00:00 2001 From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> Date: Fri, 26 Feb 2021 15:32:21 +0100 Subject: [PATCH 2/3] Improve readability --- source/as-promise/types.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/source/as-promise/types.ts b/source/as-promise/types.ts index 83564a0d5..237fe1b40 100644 --- a/source/as-promise/types.ts +++ b/source/as-promise/types.ts @@ -11,13 +11,13 @@ All parsing methods supported by Got. */ export type ResponseType = 'json' | 'buffer' | 'text'; -export interface PaginateData { - response: Response; - allItems: T[]; - currentItems: T[]; +export interface PaginateData { + response: Response; + allItems: ElementType[]; + currentItems: ElementType[]; } -export interface PaginationOptions { +export interface PaginationOptions { /** All options accepted by `got.paginate()`. */ @@ -28,17 +28,17 @@ export interface PaginationOptions { @default response => JSON.parse(response.body) */ - transform?: (response: Response) => Promise | T[]; + transform?: (response: Response) => Promise | ElementType[]; /** Checks whether the item should be emitted or not. @default (item, allItems, currentItems) => true */ - filter?: (item: T, allItems: T[], currentItems: T[]) => boolean; + filter?: (item: ElementType, allItems: ElementType[], currentItems: ElementType[]) => boolean; /** - The function takes three arguments: + The function takes an object with the following properties: - `response` - The current response object. - `allItems` - An array of the emitted items. - `currentItems` - Items from the current response. @@ -82,7 +82,7 @@ export interface PaginationOptions { })(); ``` */ - paginate?: (paginate: PaginateData) => Options | false; + paginate?: (paginate: PaginateData) => Options | false; /** Checks whether the pagination should continue. @@ -92,7 +92,7 @@ export interface PaginationOptions { @default (item, allItems, currentItems) => true */ - shouldContinue?: (item: T, allItems: T[], currentItems: T[]) => boolean; + shouldContinue?: (item: ElementType, allItems: ElementType[], currentItems: ElementType[]) => boolean; /** The maximum amount of items that should be emitted. From 9747a037e72b3e6da91832352452d7c4045582c8 Mon Sep 17 00:00:00 2001 From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> Date: Fri, 26 Feb 2021 16:21:21 +0100 Subject: [PATCH 3/3] nitpicks --- readme.md | 6 +++--- source/as-promise/types.ts | 22 +++++++++++++++------- source/create.ts | 4 ++-- test/pagination.ts | 14 +++++++------- 4 files changed, 27 insertions(+), 19 deletions(-) diff --git a/readme.md b/readme.md index fb7043805..8ff334c0c 100644 --- a/readme.md +++ b/readme.md @@ -980,18 +980,18 @@ const got = require('got'); ###### pagination.filter Type: `Function`\ -Default: `(item, allItems, currentItems) => true` +Default: `({item, allItems, currentItems}) => true` Checks whether the item should be emitted or not. ###### pagination.shouldContinue Type: `Function`\ -Default: `(item, allItems, currentItems) => true` +Default: `({item, allItems, currentItems}) => true` Checks whether the pagination should continue. -For example, if you need to stop **before** emitting an entry with some flag, you should use `(item, allItems, currentItems) => !item.flag`. If you want to stop **after** emitting the entry, you should use `(item, allItems, currentItems) => allItems.some(entry => entry.flag)` instead. +For example, if you need to stop **before** emitting an entry with some flag, you should use `({item}) => !item.flag`. If you want to stop **after** emitting the entry, you should use `({item, allItems}) => allItems.some(item => item.flag)` instead. ###### pagination.countLimit diff --git a/source/as-promise/types.ts b/source/as-promise/types.ts index 237fe1b40..835c1f53d 100644 --- a/source/as-promise/types.ts +++ b/source/as-promise/types.ts @@ -17,6 +17,12 @@ export interface PaginateData { currentItems: ElementType[]; } +export interface FilterData { + item: ElementType; + allItems: ElementType[]; + currentItems: ElementType[]; +} + export interface PaginationOptions { /** All options accepted by `got.paginate()`. @@ -33,9 +39,9 @@ export interface PaginationOptions { /** Checks whether the item should be emitted or not. - @default (item, allItems, currentItems) => true + @default ({item, allItems, currentItems}) => true */ - filter?: (item: ElementType, allItems: ElementType[], currentItems: ElementType[]) => boolean; + filter?: (data: FilterData) => boolean; /** The function takes an object with the following properties: @@ -82,17 +88,19 @@ export interface PaginationOptions { })(); ``` */ - paginate?: (paginate: PaginateData) => Options | false; + paginate?: (data: PaginateData) => Options | false; /** Checks whether the pagination should continue. - For example, if you need to stop **before** emitting an entry with some flag, you should use `(item, allItems, currentItems) => !item.flag`. - If you want to stop **after** emitting the entry, you should use `(item, allItems, currentItems) => allItems.some(entry => entry.flag)` instead. + For example, if you need to stop **before** emitting an entry with some flag, you should use `({item}) => !item.flag`. + + 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, allItems, currentItems}) => true */ - shouldContinue?: (item: ElementType, allItems: ElementType[], currentItems: ElementType[]) => boolean; + shouldContinue?: (data: FilterData) => boolean; /** The maximum amount of items that should be emitted. diff --git a/source/create.ts b/source/create.ts index 9079d7228..3a3de8b70 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, allItems, currentItems})) { + if (!pagination.shouldContinue({item, allItems, currentItems})) { return; } diff --git a/test/pagination.ts b/test/pagination.ts index 96ec22bbb..5a7f028b9 100644 --- a/test/pagination.ts +++ b/test/pagination.ts @@ -85,11 +85,11 @@ test('filters elements', withServer, async (t, server, got) => { const result = await got.paginate.all({ pagination: { - filter: (element: number, allItems: number[], currentItems: number[]) => { + filter: ({item, allItems, currentItems}) => { t.true(Array.isArray(allItems)); t.true(Array.isArray(currentItems)); - return element !== 2; + return item !== 2; } } }); @@ -209,7 +209,7 @@ test('`shouldContinue` works', withServer, async (t, server, got) => { const options = { pagination: { - shouldContinue: (_item: unknown, allItems: unknown[], currentItems: unknown[]) => { + shouldContinue: ({allItems, currentItems}: {allItems: number[]; currentItems: number[]}) => { t.true(Array.isArray(allItems)); t.true(Array.isArray(currentItems)); @@ -486,12 +486,12 @@ test('`stackAllItems` set to true', withServer, async (t, server, got) => { const result = await got.paginate.all({ pagination: { stackAllItems: true, - filter: (_item, allItems, _currentItems) => { + filter: ({allItems}) => { t.is(allItems.length, itemCount); return true; }, - shouldContinue: (_item, allItems, _currentItems) => { + shouldContinue: ({allItems}) => { t.is(allItems.length, itemCount); return true; @@ -514,12 +514,12 @@ test('`stackAllItems` set to false', withServer, async (t, server, got) => { const result = await got.paginate.all({ pagination: { stackAllItems: false, - filter: (_item, allItems, _currentItems) => { + filter: ({allItems}) => { t.is(allItems.length, 0); return true; }, - shouldContinue: (_item, allItems, _currentItems) => { + shouldContinue: ({allItems}) => { t.is(allItems.length, 0); return true;