-
-
Notifications
You must be signed in to change notification settings - Fork 935
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Pagination feature feedback #1052
Comments
Just used this with the WordPress Rest API and it just works. I've come back to the documentation to check because it was too easy and I don't know if I believe it! I feel like I've missed something - I didn't even have to specify any of the callbacks. let posts = await got.paginate(`${process.env.CRAWL_URL}wp-json/wp/v2/posts`);
console.log(posts.length); // outputs 152
for await (const post of posts) {
...
} |
I think you confused let posts = await got.paginate.all(`${process.env.CRAWL_URL}wp-json/wp/v2/posts`);
console.log(posts.length); // outputs 152
for (const post of posts) {
...
} Otherwise |
Yes sorry you are right, the example I pasted isn't quite what I've ended up with - I trimmed it down for pasting into here and effectively broke it. Thanks! |
This comment has been minimized.
This comment has been minimized.
I've tried the pagination feature out today on a Google API and it works really nicely. Thank you for implementing this ❤️ I have a couple of bits of feedback:
As a sidenote, I would imagine the major use case for the pagination feature is for Edit: after refining the code, I am now using |
For reference here's my code snippet for interfacing with the Google My Business API, using TypeScript. interface Account {
name: string;
accountName: string;
}
interface AccountsListResponseBody {
accounts: Account[];
nextPageToken?: string;
}
async function fetchLocations(accessToken: string) {
const myBusinessAPIClient = got.extend({
prefixUrl: 'https://mybusiness.googleapis.com/v4',
headers: {
authorization: `Bearer ${accessToken}`,
},
responseType: 'json',
});
const accountsSearchParams = {
pageSize: 20,
};
const accounts = await myBusinessAPIClient.paginate.all<Account>(
'accounts',
{
searchParams: accountsSearchParams,
pagination: {
transform: (response) =>
(response.body as AccountsListResponseBody).accounts,
paginate: (response) => {
const { nextPageToken } = response.body as AccountsListResponseBody;
if (!nextPageToken) {
return false;
}
return {
searchParams: {
...accountsSearchParams,
pageToken: nextPageToken,
},
};
},
},
}
);
console.log(accounts);
} |
You can (and should) cast it via a template: Lines 53 to 63 in 3da16e0
I strongly disagree. People may want to use
IMO that's a good idea. +1 on this one. You can open an issue about this so we don't forget.
That's what many people do. They expect their code to just work. If they don't use TypeScript, they should always read the docs unless they have memorized the API. Related with #920 |
I am using paginate on Sugar CRM api with Typescript node applications. First thing, it's a brilliant feature which would benefit from more documentation, example, etc. (then again it is still maturing) Using got@10.x it works like a charm (aside some minor typing things maybe and possibly a heap crash when processing a large set of pages, but I have not fully investigate this yet) Migrating to got@11.x is promising but currently broken for my usage. I opened several issues. Once I can have got@11.x to work, I will likely contribute a got template for Sugar CRM. Thanks for the many great repos. |
@szmarczak thank you for your comments.
Yes, that is a good feature - however the casting is only for the list of results. In some cases (such as the Google My Business API), the response body is an object with a property containing the list, like so: {
"accounts": [...],
"nextPageToken": "some_token"
} I believe the typings currently account for this possible difference, but they use the got/source/as-promise/types.ts Lines 73 to 81 in 3da16e0
I understand where you are coming from, but I actually did read the documentation on the pagination API several times, and still didn't fully understand until I had stepped through the code to see what was going on. I think this is simply an issue with the documentation - I would be happy to submit a PR to improve this if it would be acceptable. As also discussed above, I think that having I hope my original comment didn't offend in any way - I think this is a fantastic feature which has saved me a lot of time, and for that I thank all of the contributors. 💜 |
That's an interesting one. But I think you can just return
Exactly!
A PR would be lovely, don't hestitate to send one ;)
No, it did not. You don't have anything to worry about. Actually your points are good! |
Indeed, this is totally possible, it's just that TypeScript complains as Here is a screenshot of VSCode complaining: Casting manually in I wonder if we could pass in a second (optional) generic to Something like: export type OptionsWithPagination<T = unknown, B = unknown> = Merge<Options, PaginationOptions<T, B>>;
export interface GotPaginate {
<T, B>(url: string | URL, options?: OptionsWithPagination<T, B>): AsyncIterableIterator<T>;
}
export interface PaginationOptions<T, B> {
pagination?: {
transform?: (response: Response<B>) => Promise<T[]> | T[];
paginate?: (response: Response<B>, allItems: T[], currentItems: T[]) => Options | false;
};
}
I will try to put something together this week. 😄 |
Can you try transform: (response: Response<AccountsListResponseBody>) => {
👍 |
I'm trying to use this new feature but can't make it work, not sure what I'm doing wrong 😞
(async () => {
const limit = 10;
const items = got.paginate('https://example.com/items', {
searchParams: {
limit,
offset: 0
},
pagination: {
paginate: (response, allItems, currentItems) => {
const previousSearchParams = response.request.options.searchParams;
const {offset: previousOffset} = previousSearchParams;
if (currentItems.length < limit) {
return false;
}
return {
searchParams: {
...previousSearchParams,
offset: previousOffset + limit,
}
};
}
}
});
})(); 2 issues here:
Even though I fixed those 2 issues, the pagination does not work properly for me, search params keep getting appended each iteration, ie:
This is my code, any help appreciated here: const result = await got.paginate.all(`${apiUrl}/articles/me/all`, {
searchParams: { per_page: paginationLimit, page: 1 },
headers: { 'api-key': devtoKey },
responseType: 'json',
pagination: {
paginate: (response, allItems, currentItems) => {
const previousPage = Number(response.request.options.searchParams.get('page'));
if (currentItems.length < paginationLimit) {
return false;
}
return {
searchParams: {
per_page: paginationLimit,
page: previousPage + 1
}
};
}
}
}); |
Nice find. Please open an issue about this.
I disagree: https://github.com/sindresorhus/got#gotpaginateurl-options |
Another one 😅 Please make an issue for this too.
ATM I'm fixing bugs, will take a look later. |
This is where I landed from a google search, and the first occurence of it in the docs: https://github.com/sindresorhus/got#pagination |
This isn't the nicest, as the import { Response } from 'got/dist/source/core'; Totally do-able, just doesn't look very nice 😄 |
got/source/as-promise/types.ts Line 43 in 46cd61b
got/source/as-promise/index.ts Line 264 in 46cd61b
Line 128 in 46cd61b
You sure? |
I've hit another interesting use case today when working with the Facebook Graph API. As per the Facebook docs, the API returns the entire 'next' page URL in the response body. The idea is that you only need this URL to fetch the next page of results, saving the hassle of constructing it yourself. Taking the endpoint const { body } = await got.get('https://graph.facebook.com/v6.0/me/accounts', {
searchParams: {
access_token: '<access_token>',
fields: 'id,name',
limit: 1
},
responseType: 'json'
}); Here is the response body for this request: {
"data": [
{
"id": "<id>",
"name": "<name>"
}
],
"paging": {
"cursors": {
"before": "<cursor_before>",
"after": "<cursor_after>"
},
"next": "https://graph.facebook.com/v6.0/<id>/accounts?access_token=<redacted>&fields=id%2Cname&limit=1&after=<cursor_after>"
}
} Notice that So now let's hook up the pagination API: const accounts = await got.paginate.all('https://graph.facebook.com/v6.0/me/accounts', {
searchParams: {
access_token: '<access_token>',
fields: 'id,name',
limit: 1
},
responseType: 'json',
pagination: {
transform: (response) => response.body.data,
paginate: (response) => {
const { paging } = response.body;
if(!paging.next) {
// As per the Facebook docs, if there is no `next`, there is no more data.
return false;
}
// Otherwise, we should return the whole URL.
return {
url: paging.next
};
}
}
}); This almost works, but the issue is that the specified The workaround is to return both a url and a blank string as the searchParams from the return {
url: paging.next,
searchParams: '' // undefined / null / {} does not work here, it has to be a blank string
}; This is also an issue for other options such as I wonder if there needs to be a way for got to accept a full URL as a return from the |
Sorry you are right. VSCode didn't seem to pick this up via auto importing. Thanks for clarifying 🙂 |
Sometimes I experience this too with other TS projects. |
That works as intended. It's in the docs.
That works as intended too. You can avoid this behavior by passing |
Strangely, setting to I will submit an issue.
Thanks for the tip! That works nicely. |
There are two more relevant issues: Any feedback is greatly appreciated ❤️ |
For now I simply do the following until I wrap my head around pagination
great module! thanks! |
Would it make sense for We use pagination to query various rest API which will typically return thousands of records, and most of the time the goal is "per record" processing/transformation. In such use cases, we would rather have One possible approach (and perhaps recommended) could be to use
Working at the Is this use case meaningful to others? Should it be covered by the documentation with some sample code? Would extending the pagination API to propose event emitter or readable stream interface be a valuable feature request? Edit There is of cause the issue of closing the stream / emitting the |
You can use the functions in
Well, in that case you can just make your own loop. |
I will experiment on a large set of data this week, with custom
Would a convenient wrapper around the stream API make sense for |
I just noticed that import got from '../../dist/source/index.js';
import Bourne from '@hapi/bourne';
const max = Date.now() - 1000 * 86400 * 7;
const iterator = got.paginate('https://api.github.com/repos/sindresorhus/got/commits', {
pagination: {
transform: response => Bourne.parse(response.body),
filter: ({item}) => {
const date = new Date(item.commit.committer.date);
return date.getTime() - max >= 0;
},
shouldContinue: ({item}) => {
const date = new Date(item.commit.committer.date);
return date.getTime() - max >= 0;
},
countLimit: 50,
backoff: 100,
requestLimit: 1_000,
stackAllItems: false
}
});
console.log('Last 50 commits from now to week ago:');
for await (const item of iterator) {
console.log(item.commit.message.split('\n')[0]);
} I mean it works but it makes 40 requests instead of 2. The workaround here is: paginate: data => {
const found = data.currentItems.find(item => {
const date = new Date(item.commit.committer.date);
return date.getTime() - max >= 0;
});
if (found) {
return false;
}
return got.defaults.options.pagination.paginate(data);
} We could make the first approach do only 2 requests if @sindresorhus What do you think? |
@fiznool Indeed it makes sense to disable |
In what way would we do that? Would it be a breaking change? If so how? |
Yes, it would be a breaking change. There is a scenario where If anybody is concerned about this issue, please open a new one. I'm closing this one as pagination got stable and v12 is being released very soon. |
Try out the new pagination feature and let us know how it works. We will improve it based on your feedback. The feature lets you more easily handle the case where you need to request multiple pages from an API.
Here are relevant issues:
pagination()
topagination.each()
.pagination
APIThe text was updated successfully, but these errors were encountered: