Skip to content
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

fix(types): improve type extraction for namespaced responses and correct async iterator types #637

Merged
merged 7 commits into from
Sep 29, 2024

Conversation

wolfy1339
Copy link
Member

@wolfy1339 wolfy1339 commented Sep 27, 2024

The iterator() function returns an object containing a key of Symbol.asyncIterator, which is an object with a next() function. Only the top-level has the Symbol.asyncIterator, and the next() function is not present at the top-level

The GetResultsType<T> sometimes doesn't return the paginated data and returns the whole data object. For some reason TypeScript sometimes get's confused and returns all the keys in the object in the call KnownKeysMatching<T["data"], any[]>
In order to remedy that, we remove keys that we know do not contain the data ("repository_selection" | "total_count" | "incomplete_results") and we then always get the data.

The NormalizeResponse<T> type would return the intersection of the original data from the request and the paginated data. To remedy that, we use Omit<T, K> to remove the data from the request data and only return the paginated data instead.

Resolves #350


Before the change?

  • Contained wrong types for the return value regarding AsyncIterator
  • The GetResultsType<T> wouldn't always get the key with the data

After the change?

  • Corrects the types for AsyncIterator vs AsyncIterableIterator
  • The GetResultsType<T> correctly gets the pagination data

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes
  • No
    While not strictly a breaking change, it can cause breakage to end users as the types have changed.

The `iterator()` function returns an object containing a key of `Symbol.asyncIterator`, which is an object with a `next()` function.
Only the top-level has the `Symbol.asyncIterator`, and the `next()` function is not present at the top-level
@wolfy1339 wolfy1339 added the Type: Bug Something isn't working as documented label Sep 27, 2024

This comment was marked as off-topic.

@wolfy1339

This comment was marked as resolved.

This helps TypeScript correctly get the keys containing the data
@wolfy1339 wolfy1339 changed the title fix(types): fix bad typing for AsyncIterator vs AsyncIterableIterator fix(types): improve type extraction for namespaced responses and correct async iterator types Sep 27, 2024
@wolfy1339 wolfy1339 merged commit e95444d into main Sep 29, 2024
8 checks passed
@wolfy1339 wolfy1339 deleted the fix-async-iterator-types branch September 29, 2024 22:14
Copy link
Contributor

🎉 This PR is included in version 11.3.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released Type: Bug Something isn't working as documented
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

TypeScript types appear to be incorrect for paginate() on apps.listReposAccessibleToInstallation
2 participants