-
Notifications
You must be signed in to change notification settings - Fork 23
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
TypeScript types appear to be incorrect for paginate() on apps.listReposAccessibleToInstallation #350
Comments
Hmm I have a test for this exact endpoint and TS compiles without error: plugin-paginate-rest.js/test/validate-typescript.ts Lines 173 to 187 in 184262e
Might this be a problem with a different TS config or TS version? Note that we do create a union between the actual endpoint types which is |
closing due to inactivity |
Possibly! I believe I was on latest, not sure if VSCode pulls in a different bin though 👍 Thanks for having a look @gr2m |
Hi, we have exact same issue. Code: repos = await this.appInstallsClient.paginate(
this.appInstallsClient.apps.listReposAccessibleToInstallation,
{
per_page: 100,
headers: {
'If-None-Match': this.reposResponse?.headers.etag,
},
},
response =>
response.data.filter(repository => isActiveRepository(repository)),
); Typescript expects an object ( { repositories, total_count, ..} ). |
I am also seeing this issue. @gr2m can you reopen please? |
Note that @octokit is currently unmaintained, subscribe to octokit/octokit.js#620 (comment) for updates |
I saw the same problem today, after updating to Anyway @reececomo thanks a lot for providing a workaround. That works flawless 👍 |
No problem @Shegox! Sorry it has to be this way 😓 |
I can confirm that this is happening to me locally when cloning this repo in VSCode with TypeScript 5.0.4, but not when running the The type of the returned value for the pagination is not an array. plugin-paginate-rest.js/src/types.ts Lines 178 to 189 in 9240b2f
Maybe the returned type should be |
I've stumbled into this issue as well. If it helps anyone else until this is resolved, I'm doing this as a workaround: // N.B. I've only included the properties I happen to use here.
type InstallationRepository = {
name: string;
full_name: string;
owner: {
login: string;
};
default_branch: string;
archived: boolean;
fork: boolean;
html_url: string;
is_template: boolean;
topics: string[];
};
const repositories = (await octokit.paginate(
octokit.rest.apps.listReposAccessibleToInstallation,
{ per_page: 100 }
)) as unknown as InstallationRepository[]; |
I have the same issue i did this to solve const repositories = (await octokit.paginate(
octokit.rest.apps.listReposAccessibleToInstallation,
)) as unknown as Awaited<
ReturnType<typeof octokit.rest.apps.listReposAccessibleToInstallation>
>["data"]["repositories"]; |
For completeness, other than |
Here is what I've figured, In the pagination code, we check for and delete the following keys from the data object. plugin-paginate-rest.js/src/normalize-paginated-list-response.ts Lines 39 to 41 in 0de1011
That isn't represented in the types. Here is a little snippet that can return only the paginated data for the types for any endpoint type Data = Awaited<ReturnType<typeof octokit.rest.apps.listReposAccessibleToInstallation>>['data'];
type GetPaginationData<Data> = Data extends { total_count: number }
? Data extends { url: any }
? Data
: Data[Exclude<
keyof Data,
"repository_selection" | "total_count" | "incomplete_results"
>]
: Data;
type PaginationDataResult = GetPaginationData<Data>; Simply adding this code to the definitions should help with these situations. |
🎉 This issue has been resolved in version 11.3.5 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Hey there.
When running
await paginate(...)
onapps.listReposAccessibleToInstallation
, the TypeScript inferred data type is not correct.But really at runtime it contains
repositories
data array:Workaround
The text was updated successfully, but these errors were encountered: