plugin-paginate-graphql.js
plugin-paginate-graphql.js copied to clipboard
Create a third `options` argument and allow users to specify a `maxPages` setting
Resolves https://github.com/octokit/plugin-paginate-graphql.js/issues/165
Before the change?
- There is no way to stop the pagination early even if the user doesn't need the full list of items.
After the change?
- An optional third parameter is added that allows users to specify a
maxPagessetting. This will allow users to take advantage of the auto-merging, but stop the iteration early if needed.
Pull request checklist
- [x] Tests for the changes have been added (for bug fixes / features)
- [x] Docs have been reviewed and added / updated if needed (for bug fixes / features)
Does this introduce a breaking change?
- [ ] Yes
- [x] No
As a side note, I've also considered adding the maxPage into the existing GraphQL parameter object. That feels a bit dirty because maxPages doesn't really have anything to do with the query itself. Adding a third parameter seems like a cleaner API.
👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀
You can implement max pages using the iterator https://github.com/octokit/plugin-paginate-graphql.js?tab=readme-ov-file#octokitgraphqlpaginateiterator, would that be an option? Using iterators is recommended anyway, as it's much more memory efficient.
Yes, using the iterator can certainly achieve the same goal but then the user will most likely have to do manual merging. I figured this is way more convenient and users won’t have to change the flow/structure of their code if they simply wanted to add a limit later.
I'm reluctant to add this new API because it diverges from what we have in the REST API pagination method: https://github.com/octokit/plugin-paginate-rest.js?tab=readme-ov-file#octokitpaginate
It's fairly simple to add this API. But once we do, it will be much hard to change. We can see what others are thinking but that's my point of view.
Maybe create an issue describing what you would like to achive, leave the pull request open for context, but let's have a higher-level discussion about it first
That's totally fair!
I was not aware of the related API on the REST side. Looking at it, I do actually like that it's more flexible in terms of when done can be called. Their example of looking for a particular item and stopping early seems useful too.
However, in the GraphQL world, calling this function a mapping function isn't as applicable, since the query itself should already be specifying only the relevant fields.
An analogous API would work out to be something like this:
To stop after a certain number of pages:
const maxPages = 2;
let pages = 0;
await octokit.graphql.paginate(
`query paginate ($cursor: String) {
repository(owner: "octokit", name: "rest.js") {
issues(first: 10, after: $cursor) {
nodes {
title
}
pageInfo {
hasNextPage
endCursor
}
}
}
}`,
{},
(_, done) => {
pages += 1;
if (pages >= maxPages) {
done();
}
},
);
Or, to stop after you find a certain item:
await octokit.graphql.paginate(
`query paginate ($cursor: String) {
repository(owner: "octokit", name: "rest.js") {
issues(first: 10, after: $cursor) {
nodes {
title
}
pageInfo {
hasNextPage
endCursor
}
}
}
}`,
{},
(response, done) => {
if (response?.repository?.issues?.nodes?.[0].title === "Issue 2") {
done();
}
},
);
I will create an issue with these options listed and see if there are strong opinions.