Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve the pagination API #1644

Merged
merged 3 commits into from
Feb 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a reference to pagination.stackAllItems behaviour?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- `currentItems` - Items from the current response.
Expand All @@ -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');

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

Expand Down
34 changes: 24 additions & 10 deletions source/as-promise/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()`.
*/
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
28 changes: 16 additions & 12 deletions source/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion source/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion test/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
39 changes: 20 additions & 19 deletions test/pagination.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
});
Expand Down Expand Up @@ -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') {
Expand All @@ -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
}
});

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

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

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

Expand Down