Change the stackAllItems option to be false by default (#1645)

Author: PopGoesTheWza

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.
 - `currentItems` - Items from the current response.
+- `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.
 
@@ -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.
 
@@ -1022,11 +1022,12 @@ 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 property `allItems` in [`pagination.paginate`](#pagination.paginate), [`pagination.filter`](#pagination.filter) and [`pagination.shouldContinue`](#pagination.shouldContinue) is managed.
 
-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.
+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.
 
-This option can be helpful to save on memory usage when working with a large dataset.
 
 ##### localAddress
 

source/as-promise/types.ts

@@ -13,14 +13,14 @@ export type ResponseType = 'json' | 'buffer' | 'text';
 
 export interface PaginateData<BodyType, ElementType> {
 	response: Response<BodyType>;
-	allItems: ElementType[];
 	currentItems: ElementType[];
+	allItems: ElementType[];
 }
 
 export interface FilterData<ElementType> {
 	item: ElementType;
-	allItems: ElementType[];
 	currentItems: ElementType[];
+	allItems: ElementType[];
 }
 
 export interface PaginationOptions<ElementType, BodyType> {
@@ -39,15 +39,15 @@ export interface PaginationOptions<ElementType, BodyType> {
 		/**
 		Checks whether the item should be emitted or not.
 
-		@default ({item, allItems, currentItems}) => true
+		@default ({item, currentItems, allItems}) => true
 		*/
 		filter?: (data: FilterData<ElementType>) => boolean;
 
 		/**
 		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.
+		- `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.
@@ -66,7 +66,7 @@ export interface PaginationOptions<ElementType, BodyType> {
 					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<ElementType, BodyType> {
 		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<ElementType>) => boolean;
 
@@ -126,10 +126,11 @@ export interface PaginationOptions<ElementType, BodyType> {
 		requestLimit?: number;
 
 		/**
-		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.
+		Defines how the property `allItems` in pagination.paginate, pagination.filter and 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.
 
-		This option 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;
 	};

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) {

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),

test/pagination.ts

@@ -85,7 +85,7 @@ test('filters elements', withServer, async (t, server, got) => {
 
 	const result = await got.paginate.all<number>({
 		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});