Skip to content

Commit

Permalink
feat: drop dependency on gh (#1000)
Browse files Browse the repository at this point in the history
<!-- 👋 Hi, thanks for sending a PR to create-typescript-app! 💖.
Please fill out all fields below and make sure each item is true and [x]
checked.
Otherwise we may not be able to review your PR. -->

## PR Checklist

- [x] Addresses an existing open issue: fixes #667
- [x] That issue was marked as [`status: accepting
prs`](https://github.com/JoshuaKGoldberg/create-typescript-app/issues?q=is%3Aopen+is%3Aissue+label%3A%22status%3A+accepting+prs%22)
- [x] Steps in
[CONTRIBUTING.md](https://github.com/JoshuaKGoldberg/create-typescript-app/blob/main/.github/CONTRIBUTING.md)
were taken

## Overview

Replaces the end-user dependency on `gh` (the GitHub CLI) with more
manual Octokit calls. It's a bit less convenient this way but is more
type-safe and means users don't need to have some random GitHub utility
installed to use the template.

Update December 2023: this works but I don't like how the user has to
either have `gh` logged in or use a `process.env.GH_TOKEN`... I'd like
to find the time to look into alternatives to log the user in.

Update August 2024: I don't want to procrastinate any more. This PR
doesn't make things _worse_. So as long as error messages explicitly
tell people to either log in with `gh` or set `process.env.GH_TOKEN`,
this is fine.
  • Loading branch information
JoshuaKGoldberg authored Aug 15, 2024
1 parent 6f260c8 commit 063346e
Show file tree
Hide file tree
Showing 18 changed files with 406 additions and 323 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ It includes options not just for building and testing but also GitHub repository

First make sure you have the following installed:

- [GitHub CLI](https://cli.github.com) _(you'll need to be logged in)_
- [Node.js](https://nodejs.org)
- [pnpm](https://pnpm.io)
- _(optional, but helpful)_ [GitHub CLI](https://cli.github.com) _(you'll need to be logged in)_

Then in an existing repository or in your directory where you'd like to make a new repository:

Expand Down
5 changes: 4 additions & 1 deletion src/create/createWithOptions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,10 @@ describe("createWithOptions", () => {
};

await createWithOptions({ github, options });
expect(addToolAllContributors).toHaveBeenCalledWith(options);
expect(addToolAllContributors).toHaveBeenCalledWith(
github.octokit,
options,
);
});

it("does not call addToolAllContributors when excludeAllContributors is true", async () => {
Expand Down
2 changes: 1 addition & 1 deletion src/create/createWithOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export async function createWithOptions({ github, options }: GitHubAndOptions) {

if (!options.excludeAllContributors && !options.skipAllContributorsApi) {
await withSpinner("Adding contributors to table", async () => {
await addToolAllContributors(options);
await addToolAllContributors(github?.octokit, options);
});
}

Expand Down
5 changes: 4 additions & 1 deletion src/initialize/initializeWithOptions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,10 @@ describe("initializeWithOptions", () => {
options,
});

expect(mockAddOwnerAsAllContributor).toHaveBeenCalledWith(options);
expect(mockAddOwnerAsAllContributor).toHaveBeenCalledWith(
undefined,
options,
);
});

it("does not run addOwnerAsAllContributor when excludeAllContributors is true", async () => {
Expand Down
2 changes: 1 addition & 1 deletion src/initialize/initializeWithOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export async function initializeWithOptions({

if (!options.excludeAllContributors) {
await withSpinner("Updating existing contributor details", async () => {
await addOwnerAsAllContributor(options);
await addOwnerAsAllContributor(github?.octokit, options);
});
}

Expand Down
41 changes: 23 additions & 18 deletions src/shared/getGitHubUserAsAllContributor.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import chalk from "chalk";
import { Octokit } from "octokit";
import { beforeEach, describe, expect, it, MockInstance, vi } from "vitest";

import { getGitHubUserAsAllContributor } from "./getGitHubUserAsAllContributor.js";
Expand All @@ -13,6 +14,11 @@ vi.mock("execa", () => ({

let mockConsoleWarn: MockInstance;

const createMockOctokit = (getAuthenticated: MockInstance = vi.fn()) =>
({
rest: { users: { getAuthenticated } },
}) as unknown as Octokit;

const owner = "TestOwner";

describe("getGitHubUserAsAllContributor", () => {
Expand All @@ -23,7 +29,8 @@ describe("getGitHubUserAsAllContributor", () => {
});

it("defaults to owner with a log when options.offline is true", async () => {
const actual = await getGitHubUserAsAllContributor({
const octokit = createMockOctokit();
const actual = await getGitHubUserAsAllContributor(octokit, {
offline: true,
owner,
});
Expand All @@ -36,23 +43,24 @@ describe("getGitHubUserAsAllContributor", () => {
);
});

it("defaults to owner without a log when octokit is undefined", async () => {
const actual = await getGitHubUserAsAllContributor(undefined, { owner });

expect(actual).toEqual(owner);
expect(mockConsoleWarn).not.toHaveBeenCalled();
});

it("uses the user from gh api user when it succeeds", async () => {
const login = "gh-api-user";
const octokit = createMockOctokit(
vi.fn().mockResolvedValue({ data: { login } }),
);

mock$.mockResolvedValueOnce({
stdout: JSON.stringify({ login }),
});

await getGitHubUserAsAllContributor({ owner });
await getGitHubUserAsAllContributor(octokit, { owner });

expect(mockConsoleWarn).not.toHaveBeenCalled();
expect(mock$.mock.calls).toMatchInlineSnapshot(`
[
[
[
"gh api user",
],
],
[
[
"npx -y [email protected] add ",
Expand All @@ -67,9 +75,11 @@ describe("getGitHubUserAsAllContributor", () => {
});

it("defaults the user to the owner when gh api user fails", async () => {
mock$.mockRejectedValueOnce({});
const octokit = createMockOctokit(
vi.fn().mockRejectedValue(new Error("Oh no!")),
);

await getGitHubUserAsAllContributor({ owner });
await getGitHubUserAsAllContributor(octokit, { owner });

expect(mockConsoleWarn).toHaveBeenCalledWith(
chalk.gray(
Expand All @@ -78,11 +88,6 @@ describe("getGitHubUserAsAllContributor", () => {
);
expect(mock$.mock.calls).toMatchInlineSnapshot(`
[
[
[
"gh api user",
],
],
[
[
"npx -y [email protected] add ",
Expand Down
26 changes: 14 additions & 12 deletions src/shared/getGitHubUserAsAllContributor.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import chalk from "chalk";
import { $ } from "execa";
import { Octokit } from "octokit";

import { Options } from "./types.js";

interface GhUserOutput {
login: string;
}

export async function getGitHubUserAsAllContributor(
octokit: Octokit | undefined,
options: Pick<Options, "offline" | "owner">,
) {
if (options.offline) {
Expand All @@ -21,14 +19,18 @@ export async function getGitHubUserAsAllContributor(

let user: string;

try {
user = (JSON.parse((await $`gh api user`).stdout) as GhUserOutput).login;
} catch {
console.warn(
chalk.gray(
`Couldn't authenticate GitHub user, falling back to the provided owner name '${options.owner}'.`,
),
);
if (octokit) {
try {
user = (await octokit.rest.users.getAuthenticated()).data.login;
} catch {
console.warn(
chalk.gray(
`Couldn't authenticate GitHub user, falling back to the provided owner name '${options.owner}'.`,
),
);
user = options.owner;
}
} else {
user = options.owner;
}

Expand Down
22 changes: 14 additions & 8 deletions src/shared/options/createRepositoryWithApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,20 @@ export async function createRepositoryWithApi(

const currentUser = await octokit.rest.users.getAuthenticated();

if (currentUser.data.login === options.owner) {
await octokit.rest.repos.createForAuthenticatedUser({
name: options.repository,
});
} else {
await octokit.rest.repos.createInOrg({
name: options.repository,
org: options.owner,
try {
if (currentUser.data.login === options.owner) {
await octokit.rest.repos.createForAuthenticatedUser({
name: options.repository,
});
} else {
await octokit.rest.repos.createInOrg({
name: options.repository,
org: options.owner,
});
}
} catch (error) {
throw new Error("Failed to create new repository on GitHub.", {
cause: error,
});
}
}
2 changes: 1 addition & 1 deletion src/shared/options/getGitHub.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ describe("getOctokit", () => {
});

await expect(getGitHub).rejects.toMatchInlineSnapshot(
"[Error: GitHub authentication failed.]",
`[Error: Couldn't authenticate with GitHub. Either log in with \`gh auth login\` (https://cli.github.com) or set a GH_TOKEN environment variable.]`,
);
});

Expand Down
7 changes: 4 additions & 3 deletions src/shared/options/getGitHub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ export async function getGitHub(): Promise<GitHub | undefined> {
const auth = await getGitHubAuthToken();

if (!auth.succeeded) {
throw new Error("GitHub authentication failed.", {
cause: auth.error,
});
throw new Error(
"Couldn't authenticate with GitHub. Either log in with `gh auth login` (https://cli.github.com) or set a GH_TOKEN environment variable.",
{ cause: auth.error },
);
}

const octokit = new Octokit({ auth: auth.token });
Expand Down
8 changes: 4 additions & 4 deletions src/steps/addOwnerAsAllContributor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ describe("addOwnerAsAllContributor", () => {
mockReadFileAsJson.mockResolvedValue("invalid");

await expect(async () => {
await addOwnerAsAllContributor({ owner: mockOwner });
await addOwnerAsAllContributor(undefined, { owner: mockOwner });
}).rejects.toMatchInlineSnapshot(
'[Error: Invalid .all-contributorsrc: "invalid"]',
);
Expand All @@ -54,7 +54,7 @@ describe("addOwnerAsAllContributor", () => {
mockReadFileAsJson.mockResolvedValue({});

await expect(async () => {
await addOwnerAsAllContributor({ owner: mockOwner });
await addOwnerAsAllContributor(undefined, { owner: mockOwner });
}).rejects.toMatchInlineSnapshot(
"[Error: Invalid .all-contributorsrc: {}]",
);
Expand All @@ -68,7 +68,7 @@ describe("addOwnerAsAllContributor", () => {
contributors: [],
});

await addOwnerAsAllContributor({ owner: mockOwner });
await addOwnerAsAllContributor(undefined, { owner: mockOwner });

expect(mockWriteFile).toHaveBeenCalledWith(
"./.all-contributorsrc",
Expand All @@ -89,7 +89,7 @@ describe("addOwnerAsAllContributor", () => {
],
});

await addOwnerAsAllContributor({ owner: mockOwner });
await addOwnerAsAllContributor(undefined, { owner: mockOwner });

expect(mockWriteFile).toHaveBeenCalledWith(
"./.all-contributorsrc",
Expand Down
4 changes: 3 additions & 1 deletion src/steps/addOwnerAsAllContributor.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
import * as fs from "node:fs/promises";
import { Octokit } from "octokit";

import { getGitHubUserAsAllContributor } from "../shared/getGitHubUserAsAllContributor.js";
import { readFileAsJson } from "../shared/readFileAsJson.js";
import { AllContributorsData, Options } from "../shared/types.js";
import { formatJson } from "./writing/creation/formatters/formatJson.js";

export async function addOwnerAsAllContributor(
octokit: Octokit | undefined,
options: Pick<Options, "offline" | "owner">,
) {
const user = await getGitHubUserAsAllContributor(options);
const user = await getGitHubUserAsAllContributor(octokit, options);

const existingContributors = (await readFileAsJson(
"./.all-contributorsrc",
Expand Down
4 changes: 2 additions & 2 deletions src/steps/addToolAllContributors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@ describe("addToolAllContributors", () => {
it("adds JoshuaKGoldberg when that is not the current github user", async () => {
mockGetGitHubUserAsAllContributor.mockResolvedValue("JoshuaKGoldberg");

await addToolAllContributors({ owner: "owner" });
await addToolAllContributors(undefined, { owner: "owner" });

expect(mock$).not.toHaveBeenCalled();
});

it("does not add JoshuaKGoldberg when that not the current github user", async () => {
mockGetGitHubUserAsAllContributor.mockResolvedValue("other");

await addToolAllContributors({ owner: "owner" });
await addToolAllContributors(undefined, { owner: "owner" });

expect(mock$).toHaveBeenCalledWith([
`npx -y all-contributors-cli add JoshuaKGoldberg tool`,
Expand Down
4 changes: 3 additions & 1 deletion src/steps/addToolAllContributors.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { $ } from "execa";
import { Octokit } from "octokit";

import { getGitHubUserAsAllContributor } from "../shared/getGitHubUserAsAllContributor.js";
import { Options } from "../shared/types.js";

export async function addToolAllContributors(
octokit: Octokit | undefined,
options: Pick<Options, "offline" | "owner">,
) {
const login = await getGitHubUserAsAllContributor(options);
const login = await getGitHubUserAsAllContributor(octokit, options);

if (login !== "JoshuaKGoldberg") {
await $`npx -y all-contributors-cli add JoshuaKGoldberg tool`;
Expand Down
2 changes: 1 addition & 1 deletion src/steps/initializeGitHubRepository/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@ export async function initializeGitHubRepository(
await initializeGitRemote(options);
await initializeRepositorySettings(octokit, options);
await initializeBranchProtectionSettings(octokit, options);
await initializeRepositoryLabels();
await initializeRepositoryLabels(octokit, options);
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const aliases = new Map([

export interface GhLabelData {
color: string;
description: string;
description: null | string;
name: string;
}

Expand Down
Loading

0 comments on commit 063346e

Please sign in to comment.