Skip to content

Conversation

wolfy1339
Copy link
Member

  • Pre-compute the keys of the Endpoint type in order to not have to compute them multiple times
  • Use interface extends instead of intersections

See https://github.com/microsoft/TypeScript/wiki/Performance
Part of #666


Before the change?

  • Utilized keyof Endpoint directly in the code many times across different files
  • Used intersections for extending types

After the change?

  • Pre-compute the keys of the Endpoint type that can be shared across many files
  • Utilize interfaces and extends for better performance

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

* Pre-compute the keys of the `Endpoint` type in order to not have to compute them multiple times
* Use interface extends instead of intersections

See https://github.com/microsoft/TypeScript/wiki/Performance
Part of #666
@wolfy1339 wolfy1339 added the Type: Maintenance Any dependency, housekeeping, and clean up Issue or PR label Mar 13, 2025
Copy link
Contributor

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

@MisanthropicBit
Copy link

I'll try out this branch in my projects and see if it fixes in the issue in #666 🙂

@wolfy1339
Copy link
Member Author

It should definitely improve things, that's for sure.

When I tested some small test code for Octokit, the TypeScript profiler did identify the hotspot with the keyof Endpoint, other than that there wasn't anything else

@MisanthropicBit
Copy link

I tried testing with the branch locally (outside docker) but it seems the issue persists with the same tracing profile as before. I think this PR is still an overall improvement though.

@wolfy1339 wolfy1339 changed the base branch from main to openapi-update May 25, 2025 22:36
@wolfy1339 wolfy1339 merged commit a75f952 into openapi-update May 25, 2025
5 checks passed
@wolfy1339 wolfy1339 deleted the performance branch May 25, 2025 22:36
@github-project-automation github-project-automation bot moved this from 🆕 Triage to ✅ Done in 🧰 Octokit Active May 25, 2025
wolfy1339 added a commit that referenced this pull request May 25, 2025
…entials/revoke` endpoints, endpoint type updates, type performance fixes (#675)

* WIP octokit/openapi updated

* fix: implement some performance improvements (#667)

* Pre-compute the keys of the `Endpoint` type in order to not have to compute them multiple times
* Use interface extends instead of intersections

See https://github.com/microsoft/TypeScript/wiki/Performance
Part of #666

---------

Co-authored-by: Octokit Bot <[email protected]>
Co-authored-by: wolfy1339 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Maintenance Any dependency, housekeeping, and clean up Issue or PR
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants