Skip to content

Commit

Permalink
Change the stackAllItems option to be false by default (#1645)
Browse files Browse the repository at this point in the history
  • Loading branch information
PopGoesTheWza authored Feb 26, 2021
1 parent 31d80ef commit 1120370
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 26 deletions.
15 changes: 8 additions & 7 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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');

Expand All @@ -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.

Expand Down Expand Up @@ -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

Expand Down
19 changes: 10 additions & 9 deletions source/as-promise/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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.
Expand All @@ -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');
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
};
Expand Down
8 changes: 4 additions & 4 deletions source/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -266,8 +266,8 @@ const create = (defaults: InstanceDefaults): Got => {

const optionsToMerge = pagination.paginate({
response,
allItems,
currentItems
currentItems,
allItems
});

if (optionsToMerge === false) {
Expand Down
2 changes: 1 addition & 1 deletion source/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
10 changes: 5 additions & 5 deletions test/pagination.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -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});
}
}
});
Expand All @@ -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});
Expand Down

0 comments on commit 1120370

Please sign in to comment.