Skip to content

Conversation

@pschutz93
Copy link
Contributor

@pschutz93 pschutz93 commented Oct 25, 2025

This PR adds a helper to allow fetching SDK artifacts directly from Github. This helper works inline with other codeSample SDK configs. It takes the standard SDK config with additional parameters to specify repo and version tag and an optional Github token. While I can definitely see use cases where you want a unique token per SDK, this helper also allows defaulting to a token set as the environment variable $GITHUB_TOKEN to avoid getting too verbose when that level of configuration isn't needed.

.It returns a promise to a code sample instance with sdkTarballPath set to a tarball of the SDK repo downloaded directly from Github. I updated the types so this helper can only be used with codeSample types that require an SDK artifact (which right now just means not curl).

To gracefully handle promises to config values and the underlying values themselves, I am using zod's preprocess() function to allow awaiting resolution of the config value before parsing the config with safeParseAsync(). I went with this approach because when reading the docs, I noticed z.promise() is deprecated in the new v4, and I think unwrapping the value before parsing is cleaner anyway and perfectly well supported by preprocess() since it supports async pre-processing functions.

This PR uses octokit.js to download a Tarball archive for the specified SDK repo using the Personal Access Token for auth.

export default {
  spec: "../specs/glean.yaml",
  output: {
    pageOutDir: "./docs/glean/api",
    embedOutDir: "./src/components/glean-embeds",
    framework: "docusaurus",
  },
  display: {
    maxNestingLevel: 2,
  },
  codeSamples: [
    withGithubSdk(
      {
        language: "typescript",
        owner: "gleanwork",
        repo: "api-client-typescript",
        version: "v0.11.2",
        tryItNow: {
          outDir: "./public/glean-try-it-now",
          urlPrefix: "/glean-try-it-now",
        },
      },
      process.env.GITHUB_TOKEN
    ),
    {
      language: "curl",
    },
  ],
};

@vercel
Copy link

vercel bot commented Oct 25, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
docs-demo-custom-nextjs Ready Ready Preview Comment Oct 28, 2025 10:35pm
docs-demo-docusaurus Ready Ready Preview Comment Oct 28, 2025 10:35pm
docs-demo-nextra Ready Ready Preview Comment Oct 28, 2025 10:35pm

@vercel vercel bot temporarily deployed to Preview – docs-demo-docusaurus October 25, 2025 01:12 Inactive
@vercel vercel bot temporarily deployed to Preview – docs-demo-custom-nextjs October 25, 2025 01:12 Inactive
@vercel vercel bot temporarily deployed to Preview – docs-demo-nextra October 25, 2025 01:12 Inactive
@pschutz93 pschutz93 force-pushed the pschutz-github-fetch branch from 2077026 to 539a7a5 Compare October 27, 2025 14:36
@vercel vercel bot temporarily deployed to Preview – docs-demo-custom-nextjs October 27, 2025 14:36 Inactive
@vercel vercel bot temporarily deployed to Preview – docs-demo-nextra October 27, 2025 14:36 Inactive
@vercel vercel bot temporarily deployed to Preview – docs-demo-docusaurus October 27, 2025 14:36 Inactive
@vercel vercel bot temporarily deployed to Preview – docs-demo-custom-nextjs October 27, 2025 15:30 Inactive
@vercel vercel bot temporarily deployed to Preview – docs-demo-nextra October 27, 2025 15:31 Inactive
@vercel vercel bot temporarily deployed to Preview – docs-demo-docusaurus October 27, 2025 15:31 Inactive
@pschutz93 pschutz93 marked this pull request as ready for review October 27, 2025 15:44
@vercel vercel bot temporarily deployed to Preview – docs-demo-docusaurus October 27, 2025 21:25 Inactive
@vercel vercel bot temporarily deployed to Preview – docs-demo-custom-nextjs October 27, 2025 21:25 Inactive
@vercel vercel bot temporarily deployed to Preview – docs-demo-nextra October 27, 2025 21:26 Inactive
Comment on lines 3 to 20
const sdkConfigs = await withGithubSdks(
[
{
language: "typescript",
owner: "gleanwork",
repo: "api-client-typescript",
version: "v0.11.2",
tryItNow: {
outDir: "./public/glean-try-it-now",
urlPrefix: "/glean-try-it-now",
},
},
{
language: "curl",
},
],
process.env.GITHUB_TOKEN
);
Copy link
Contributor

@nebrius nebrius Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know we originally specced out this helper to exist at this level, but now that I see it in practice I'm not sure I love it.

What if instead we did something like this?

export default {
  spec: "../specs/glean.yaml",
  output: {
    pageOutDir: "./docs/glean/api",
    embedOutDir: "./src/components/glean-embeds",
    framework: "docusaurus",
  },
  display: {
    maxNestingLevel: 2,
  },
  codeSamples: [
    withGitHubSdk({
      language: "typescript",
      owner: "gleanwork",
      repo: "api-client-typescript",
      version: "v0.11.2",
      tryItNow: {
        outDir: "./public/glean-try-it-now",
        urlPrefix: "/glean-try-it-now",
      },
    })
  ]
};

This way, we allow more flexibility by scoping the GitHub part just to the code sample in question. Who knows, maybe folks might want to use different providers for different languages...if nothing else GitHub definitely doesn't apply to cURL samples.

We could implement this by changing the signature of codeSamples in the main Settings type to be something like CodeSample | (config: unknown) => Promise<CodeSample> or something

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I like this. And accepting config values (like codeSamples in this case) as promises at the main Settings level adds a lot of flexibility for what customers can do too, which I like.

I'll make those updates and we can go from there!

@pschutz93 pschutz93 force-pushed the pschutz-github-fetch branch from cbc9b94 to 36e00ef Compare October 27, 2025 22:02
@vercel vercel bot temporarily deployed to Preview – docs-demo-docusaurus October 27, 2025 22:02 Inactive
@vercel vercel bot temporarily deployed to Preview – docs-demo-custom-nextjs October 27, 2025 22:03 Inactive
@vercel vercel bot temporarily deployed to Preview – docs-demo-nextra October 27, 2025 22:03 Inactive
@pschutz93
Copy link
Contributor Author

This change is part of the following stack:

Change managed by git-spice.

@pschutz93 pschutz93 mentioned this pull request Oct 27, 2025
@pschutz93 pschutz93 force-pushed the pschutz-github-fetch branch from 36e00ef to c2d1c88 Compare October 28, 2025 22:23
@vercel vercel bot temporarily deployed to Preview – docs-demo-docusaurus October 28, 2025 22:24 Inactive
@vercel vercel bot temporarily deployed to Preview – docs-demo-custom-nextjs October 28, 2025 22:24 Inactive
@vercel vercel bot temporarily deployed to Preview – docs-demo-nextra October 28, 2025 22:24 Inactive
@vercel vercel bot temporarily deployed to Preview – docs-demo-docusaurus October 28, 2025 22:34 Inactive
@vercel vercel bot temporarily deployed to Preview – docs-demo-custom-nextjs October 28, 2025 22:35 Inactive
@vercel vercel bot temporarily deployed to Preview – docs-demo-nextra October 28, 2025 22:35 Inactive
@pschutz93 pschutz93 requested a review from nebrius October 28, 2025 22:53
@pschutz93
Copy link
Contributor Author

pschutz93 commented Oct 28, 2025

@nebrius I revised the PR to inline the helper. In particular, I was curious about your thoughts on the pattern I am using to unwrap promise values from the config.

Could you take another pass when you get the chance?

And I'll update the withGithubSpec PR next (it's still just a draft)! Just wanted to get the overall helper pattern nailed down before updating it in the same manner.

Copy link
Contributor

@nebrius nebrius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just one last little thing

process.exit(1);
}
}
throw new InternalError(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
throw new InternalError(
throw new Error(

I bet the most likely reason for this will be stuff like not being able to connect to the server, lookup DNS, etc., which could be due to internet issues on the user's end.

In this case, it's not a bug on our part, and so should be a normal error.

@adaam2 adaam2 closed this Nov 7, 2025
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

Successfully merging this pull request may close these issues.

4 participants