Skip to content

[FEAT]: Give user control to stop pagination earlier #165

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

Open
1 task done
jetzhou opened this issue Feb 15, 2024 · 2 comments · May be fixed by #163
Open
1 task done

[FEAT]: Give user control to stop pagination earlier #165

jetzhou opened this issue Feb 15, 2024 · 2 comments · May be fixed by #163
Labels
Type: Feature New feature or request

Comments

@jetzhou
Copy link

jetzhou commented Feb 15, 2024

Describe the need

Use case

The default behavior right now is to paginate through all pages always. However, in certain cases users may want to stop the pagination earlier. For example, it could be useful to stop after a certain number of pages or to stop after a particular item shows up in the results. This use case can be solved by using the iterator function/interface, but it then forces the users to do manual merging on their end, which reduces the benefit of using this plugin.

Prior art

The REST plugin provides such a control mechanism via a mapFunction parameter. However, it's called a mapFunction even though it actually serves 2 purposes: mapping/transforming and stopping early. In GraphQL world, mapping/transforming is not necessary nor expected.

Proposal

Add a stopFunction as the third argument to iterator and paginate. After each page is fetched, the function will be invoked with two parameters, first is the response of the most recently fetched page, and the second is a done function that stops the iteration if invoked.

The usage scenarios would be covered like the following snippets.

  • To stop after a max 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();
    }
  },
);

Alternative implementation

Instead modeling after the REST plugin's API, we could just add an options object as the third argument and allow users to pass in more explicit "stop conditions". For example:

  • for max number of pages:
{ maxPages: 3 }
  • or, for stopping after finding an item
{ stopIteration: (response) => response?.repository?.issues?.nodes?.[0].title === "Issue 2" }

This could allow other options to be added in the future so it's more extensible. But this will also mean that there is a divergence in API between REST and GraphQL paginate plugins.

Related PR

#163

SDK Version

No response

API Version

No response

Relevant log output

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@jetzhou jetzhou added Status: Triage This is being looked at and prioritized Type: Feature New feature or request labels Feb 15, 2024
Copy link

👋 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! 🚀

@kfcampbell kfcampbell moved this from 🆕 Triage to 🔥 Backlog in 🧰 Octokit Active Feb 20, 2024
@kfcampbell kfcampbell removed the Status: Triage This is being looked at and prioritized label Feb 20, 2024
@ScottKayeNeo
Copy link

ScottKayeNeo commented May 7, 2025

I support adding this feature.

but it then forces the users to do manual merging on their end

This is what prompted me to start searching for this feature and finding your thread. I've solved this by hackily grabbing the mergeResponses function and calling it myself:

import type { mergeResponses as mergeResponsesType } from './node_modules/@octokit/plugin-paginate-graphql/dist-types/merge-responses.d.ts';

const mergeResponses = require('./node_modules/@octokit/plugin-paginate-graphql/dist-src/merge-responses.js')
  .mergeResponses as typeof mergeResponsesType;

Obviously this breaks if this package is updated and that file moves, the function is renamed, etc.

Function (basically this):

async function getNPages<T extends object>(iterable: AsyncIterable<T>, maxPages: number): Promise<T> {
  let mergedResponse = {} as T;
  let pages = 0;

  for await (const response of iterable) {
    mergedResponse = mergeResponses(mergedResponse, response);

    if (++pages >= maxPages) {
      break;
    }
  }

  return mergedResponse;
}

Usage (grab the 6 most recent PRs on this repository, with a page size of 2, max 3 pages):

const iter = this.octokit.graphql.paginate.iterator(`
  query ($cursor: String) {
    organization(login: "octokit") {
      repository(name: "plugin-paginate-graphql.js") {
        pullRequests(
          first: 2,
          states: MERGED,
          after: $cursor,
          orderBy: {
            field: CREATED_AT,
            direction: DESC
          }) {
          pageInfo {
            endCursor
            hasNextPage
          }
          nodes {
            id
            number
          }
        }
      }
    }
  }
`);

const pullRequests = await getNPages(iter, 3);

Now, pullRequests has a nodes array with 6 items in it, as expected.

I think a great short-term solution, if there's apprehension around adding a maxPages API as seen in the PR thread, is to properly export the mergeResponses function at the package level. Then we don't need to do this ugly hack - the rest of it is fairly clean for what it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New feature or request
Projects
Status: 🔥 Backlog
3 participants