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.

Sign up for GitHub

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

Improve the pagination API #1644

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a1b259c
object
szmarczak Feb 26, 2021
c4520ae
Improve readability
szmarczak Feb 26, 2021
9747a03
nitpicks
szmarczak Feb 26, 2021
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
4 changes: 2 additions & 2 deletions readme.md
View file
Open in desktop
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.

It's just a scroll away: https://github.com/sindresorhus/got#paginationstackallitems

- `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 Down
8 changes: 7 additions & 1 deletion source/as-promise/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ All parsing methods supported by Got.
*/
export type ResponseType = 'json' | 'buffer' | 'text';

export interface PaginateData<R, T> {
szmarczak marked this conversation as resolved.
Show resolved Hide resolved
response: Response<R>;
allItems: T[];
currentItems: T[];
}

export interface PaginationOptions<T, R> {
/**
All options accepted by `got.paginate()`.
Expand Down Expand Up @@ -76,7 +82,7 @@ export interface PaginationOptions<T, R> {
})();
```
*/
paginate?: (response: Response<R>, allItems: T[], currentItems: T[]) => Options | false;
paginate?: (paginate: PaginateData<R, T>) => Options | false;

/**
Checks whether the pagination should continue.
Expand Down
28 changes: 16 additions & 12 deletions source/create.ts
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
25 changes: 13 additions & 12 deletions test/pagination.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
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 @@ -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 @@ -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});
}
}
});
Expand All @@ -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});
}
}
});
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