[FEAT]: Add pagination support for `compareCommitsWithBasehead()`

[LykinsN]

What happened?

Our team is using the @octokit/rest library to handle several different calls to our private GHE platform and we are using the paginate wrapper to handle larger datasets. This functionality is working on several other API calls (i.e. listPullRequestsAssociatedWithCommit, listBranches, etc) but is not working correctly with the compareCommitsWithBasehead call. For example, if I reference two branches in the following snippet:

import { Octokit } from '@octokit/rest';
...
const octokit = new Octokit({ auth: githubToken, baseUrl: githubBaseUrl });
...
const data = await octokit.paginate(
   octokit.rest.repos.compareCommitsWithBasehead,
  {
    owner: githubOwner,
    repo: githubRepo,
    basehead: `${branchOne}...${branchTwo}`,
  },
);

If this results in more than 250 records, I'd expect to get the full set back, i.e. 500. But instead, the call only seems to return a single page, and only returns the maximum allowed number from the compareCommitsWithBasehead API call. I've been able to write a custom pagination handler to do the work for me in this case, but I'd expect the paginate wrapper should handle it as well.

Versions

@octokit/rest: v20.1.0, Node: v18.20.4

Relevant log output

No response

Code of Conduct

• I agree to follow this project's Code of Conduct

[github-actions]

👋 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 labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

[wolfy1339]

I recommend upgrading to @octokit/rest v21, do note that it is ESM only now.

Can you try again with that version?

[LykinsN]

I tried reverting back to my original code and upgraded @octokit/rest to 21.0.2, but I'm seeing the same behavior, unfortunately. The paginate response is still limiting the dataset to 250 items.

[wolfy1339]

can you share your custom pagination code? it could help us debug the issue further

[LykinsN]

Sure thing, I based it off a sample I found elsewhere in the GitHub docs, where an example was provided. I just tuned it a bit to match my use-case. I'm not sure it's relevant but we're running in Typescript rather than just native Javascript. I'll include the code sections that are relevant to the pagination process.

import { Octokit as OctokitBasePackage } from 'octokit';

const octokitBase = new OctokitBasePackage({ auth: githubToken, baseUrl: githubBaseUrl });

export const parseData = (data) => {
  // If the data is an array, return that
  if (Array.isArray(data)) {
    return data;
  }

  // Some endpoints respond with 204 No Content instead of empty array
  //   when there is no data. In that case, return an empty array.
  if (!data) {
    return [];
  }

  // Pull out the array of items
  const namespaceKey = Object.keys(data)[0];
  return data[namespaceKey];
};

export const getDiffedPullRequests = async (
  octokitBase: OctokitBasePackage,
  githubClient: Octokit,
  githubOwner,
  githubRepo,
  releaseBranch: string,
  previousBranch: string,
) => {
  const nextPattern = /(?<=<)([\S]*)(?=>; rel="Next")/i;
  let pagesRemaining = true;
  let data = [];
  const pullRequestList = [];

  let url = 'GET /repos/{owner}/{repo}/compare/{basehead}';

  while (pagesRemaining) {
    const response = await octokitBase.request(url, {
      owner: githubOwner,
      repo: githubRepo,
      basehead: `${previousBranch}...${releaseBranch}`,
      per_page: 100,
      headers: {
        'X-GitHub-Api-Version': '2022-11-28',
      },
    });

    const parsedData = parseData(response.data.commits);

    data = [...data, ...(Array.isArray(parsedData) ? parsedData : [parsedData])];

    const linkHeader = response.headers.link;

    pagesRemaining = linkHeader && linkHeader.includes('rel="next"');

    if (pagesRemaining) {
      [url] = linkHeader.match(nextPattern);
    }
  }
  return data;
};

const data = await getDiffedPullRequests(octokitBase, githubClient, githubOwner, githubRepo, releaseBranch, previousBranch);

[github-actions]

👋 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 labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

[wolfy1339]

Looking through documentation, and the code, it seems that it is a special case.

plugin-paginate-rest.js/scripts/update-endpoints/typescript.js

Lines 31 to 35 in a73883f

	// The [docs](https://docs.github.com/en/rest/commits/commits#compare-two-commits) make
	// these ones sound like a special case too - they must be because they support pagination
	// but doesn't return an array.
	{ scope: "repos", id: "compare-commits" },
	{ scope: "repos", id: "compare-commits-with-basehead" },

It is explicitly filtered out from the types.

plugin-paginate-rest.js/src/normalize-paginated-list-response.ts

Lines 30 to 31 in a73883f

	const responseNeedsNormalization =
	"total_count" in response.data && !("url" in response.data);

As seen there, the pagination plugin doesn't currently handle other special cases. In this case there is no total_count but total_commits