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

TypeScript performance #40

Open
gr2m opened this issue Sep 22, 2021 · 4 comments
Open

TypeScript performance #40

gr2m opened this issue Sep 22, 2021 · 4 comments

Comments

@gr2m
Copy link
Contributor

gr2m commented Sep 22, 2021

I'm happy with the TypeScript IntelliSense we can provide for different REST API versions in the upcoming Octokit, however the current implementations seems to overwhelm the TypeScript engine. It gets very slow at times, you can see it in the videos I posted at octokit/octokit.js#2127 (comment)

I can understand that TypeScript gets slow, we do work with hundreds of REST API endpoints which are referenced and combined with new definitions for versions other than github.com. But I hope that we can find a way to optimize the speed of the TypeScript lookups to make it usable.

I could really use help with this, as I have no idea how to profile TypeScript when working with VS Code.

To see what I mean, try the following:

  1. In a new folder, run npm init -y && npm install @octokit-next/core
  2. Create an index.ts file with the following content
// import "@octokit-next/types-rest-api";

import { Octokit } from "@octokit-next/core";

export async function test() {
  const octokit = new Octokit({
    auth: "token 0000000000000000000000000000000000000001",
  });

  const responseRepo = await octokit.request("GET /repos/{owner}/{repo}", {
    owner: "octokit-fixture-org",
    repo: "hello-world",
  });
  expectType<string>(responseRepo.data.owner.login);
}

function expectType<T>(value: T): void {}

Now the import in first line and see how long it takes for TypeScript to catch up with the new types for the GET /repos/{owner}/{repo} endpoint.

There are other examples that can be seen in the video, but figuring out how we could improve the performance for this particular case would be a great start.

@gr2m
Copy link
Contributor Author

gr2m commented Sep 22, 2021

The 2nd video is probably a good 2nd step. You can follow along by starting with this code

import "@octokit-next/types-rest-api

import { Octokit } from "@octokit-next/core";

export async function test() {
  const octokit = new Octokit({
    auth: "token 0000000000000000000000000000000000000001",
  });

  const responseRepo = await octokit.request("GET /repos/{owner}/{repo}", {
    owner: "octokit-fixture-org",
    repo: "hello-world",
  });
  expectType<string>(responseRepo.data.owner.login);

  octokit.request("GET /admin/tokens");
}

function expectType<T>(value: T): void {}

Then

  1. Replace the import in first line with

    import "@octokit-next/types-rest-api-ghes-3.0";

    Look how long it takes for the compiler to update the error for "GET /admin/tokens" to octokit.request("GET /admin/tokens"); expecting 2 parameters.

  2. Then add an {} as the 2nd parameter, put the cursor between the brackets and press ctrl + space to invoke TypeScripts suggestions, see how long it takes to get the suggestions for the request parameter.

  3. Set the 2nd argument to { request: {}}, put the cursor between the {} brackets, press ctrl + space and see how long it takes for TypeScript to suggest version.

  4. Set the 2nd argument to { request: { version: ""}, put the cursor between the "" quotes, press ctrl + space and see how long it takes for TypeScript to suggest the supported versions

  5. Once once request.version is set to ghes-3.0 or one of the other supported versions, see how long it takes for the TypeScript compiler error to be removed from the code view.

@gr2m
Copy link
Contributor Author

gr2m commented Sep 23, 2021

@DanielRosenwasser
Copy link

DanielRosenwasser commented Oct 4, 2021

You might also want to peek at @amcasey's https://github.com/amcasey/ts-analyze-trace which can help narrow down major hot spots from the output of tsc --generateTrace traceOutputDir.

@gr2m
Copy link
Contributor Author

gr2m commented Oct 4, 2021

There is no TypeScript code, I do write the .js and .d.ts files by hand. Not sure if tsc can be still used for that use case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants