Improve the pagination API (#1644)

szmarczak · web-flow · commit 2675046a83c7 · 2021-02-26T23:00:49.000+07:00
diff --git a/readme.md b/readme.md
@@ -937,7 +937,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.
@@ -958,7 +958,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');
 
@@ -983,18 +983,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
@@ -11,7 +11,19 @@ All parsing methods supported by Got.
 */
 export type ResponseType = 'json' | 'buffer' | 'text';
 
-export interface PaginationOptions<T, R> {
+export interface PaginateData<BodyType, ElementType> {
+	response: Response<BodyType>;
+	allItems: ElementType[];
+	currentItems: ElementType[];
+}
+
+export interface FilterData<ElementType> {
+	item: ElementType;
+	allItems: ElementType[];
+	currentItems: ElementType[];
+}
+
+export interface PaginationOptions<ElementType, BodyType> {
 	/**
 	All options accepted by `got.paginate()`.
 	*/
@@ -22,17 +34,17 @@ export interface PaginationOptions<T, R> {
 
 		@default response => JSON.parse(response.body)
 		*/
-		transform?: (response: Response<R>) => Promise<T[]> | T[];
+		transform?: (response: Response<BodyType>) => Promise<ElementType[]> | ElementType[];
 
 		/**
 		Checks whether the item should be emitted or not.
 
-		@default (item, allItems, currentItems) => true
+		@default ({item, allItems, currentItems}) => true
 		*/
-		filter?: (item: T, allItems: T[], currentItems: T[]) => boolean;
+		filter?: (data: FilterData<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.
@@ -76,17 +88,19 @@ export interface PaginationOptions<T, R> {
 		})();
 		```
 		*/
-		paginate?: (response: Response<R>, allItems: T[], currentItems: T[]) => Options | false;
+		paginate?: (data: PaginateData<BodyType, ElementType>) => 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: T, allItems: T[], currentItems: T[]) => boolean;
+		shouldContinue?: (data: FilterData<ElementType>) => boolean;
 
 		/**
 		The maximum amount of items that should be emitted.
diff --git 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,42 +236,46 @@ 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;
 					}
 				}
 			}
 
-			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
@@ -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
@@ -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
@@ -85,11 +85,11 @@ test('filters elements', withServer, async (t, server, got) => {
 
 	const result = await got.paginate.all<number>({
 		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;
 			}
 		}
 	});
@@ -126,7 +126,7 @@ test('custom paginate function', withServer, async (t, server, got) => {
 
 	const result = await got.paginate.all<number>({
 		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<number>({
 		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<number>({
 		pagination: {
-			paginate: (_response, _allItems: number[], currentItems: number[]) => {
+			paginate: ({currentItems}) => {
 				if (currentItems[0] === 3) {
 					return false;
 				}
@@ -208,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));
 
@@ -357,7 +358,7 @@ test('`hooks` are not duplicated', withServer, async (t, server, got) => {
 
 	const result = await got.paginate.all<number>({
 		pagination: {
-			paginate: response => {
+			paginate: ({response}) => {
 				if ((response.body as string) === '[3]') {
 					return false; // Stop after page 3
 				}
@@ -485,21 +486,21 @@ test('`stackAllItems` set to true', withServer, async (t, server, got) => {
 	const result = await got.paginate.all<number>({
 		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;
 			},
-			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});
 			}
 		}
 	});
@@ -513,20 +514,20 @@ test('`stackAllItems` set to false', withServer, async (t, server, got) => {
 	const result = await got.paginate.all<number>({
 		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;
 			},
-			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<Page>) => {
 				return [response.body.currentUrl];
 			},
-			paginate: (response: Response<Page>) => {
+			paginate: ({response}) => {
 				const {next} = response.body;
 
 				if (!next) {
@@ -608,7 +609,7 @@ test('pagination using searchParams', withServer, async (t, server, got) => {
 			transform: (response: Response<Page>) => {
 				return [response.body.currentUrl];
 			},
-			paginate: (response: Response<Page>) => {
+			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<Page>) => {
 				return [response.body.currentUrl];
 			},
-			paginate: (response: Response<Page>) => {
+			paginate: ({response}) => {
 				const {next} = response.body;
 				const previousPage = Number(response.request.options.searchParams!.get('page'));
 

Original file line number	Diff line number	Diff line change
`@@ -77,7 +77,7 @@ const defaults: InstanceDefaults = {`
`77`	`77`
`78`	`78`	`return JSON.parse(response.body as string);`
`79`	`79`	`},`
`80`		`- paginate: response => {`
	`80`	`+ paginate: ({response}) => {`
`81`	`81`	`if (!Reflect.has(response.headers, 'link')) {`
`82`	`82`	`return false;`
`83`	`83`	`}`