Skip to content

Commit 01704e9

Browse files
fix: normalize whether build happens before lint/knip (#1152)
## PR Checklist - [x] Addresses an existing open issue: fixes #1087 - [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 Finally makes it so a `pnpm build || exit 0` only happens if `bin` is specified. This is because: * `n/no-missing-import` will complain if the referenced `.js` file doesn't exist * The only way for that file to exist is to `build` * But, building failing -especially in migrating an existing repo- shouldn't prevent the rest of the commands from running
1 parent 620b6b6 commit 01704e9

10 files changed

+68
-30
lines changed

.github/workflows/lint-knip.yml

-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ jobs:
44
steps:
55
- uses: actions/checkout@v4
66
- uses: ./.github/actions/prepare
7-
- run: pnpm build || true
87
- run: pnpm lint:knip
98

109
name: Lint Knip

script/__snapshots__/migrate-test-e2e.js.snap

-13
Original file line numberDiff line numberDiff line change
@@ -50,19 +50,6 @@ exports[`expected file changes > .eslintrc.cjs 1`] = `
5050
{"
5151
`;
5252
53-
exports[`expected file changes > .github/workflows/lint-knip.yml 1`] = `
54-
"--- a/.github/workflows/lint-knip.yml
55-
+++ b/.github/workflows/lint-knip.yml
56-
@@ ... @@ jobs:
57-
steps:
58-
- uses: actions/checkout@v4
59-
- uses: ./.github/actions/prepare
60-
- - run: pnpm build || true
61-
- run: pnpm lint:knip
62-
63-
name: Lint Knip"
64-
`;
65-
6653
exports[`expected file changes > .github/workflows/test.yml 1`] = `
6754
"--- a/.github/workflows/test.yml
6855
+++ b/.github/workflows/test.yml

script/migrate-test-e2e.js

-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ const filesExpectedToBeChanged = [
1111
"package.json",
1212
".eslintignore",
1313
".eslintrc.cjs",
14-
".github/workflows/lint-knip.yml",
1514
".github/workflows/test.yml",
1615
".gitignore",
1716
".prettierignore",

src/create/createWithOptions.ts

+8-5
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { $ } from "execa";
22

33
import { withSpinner, withSpinners } from "../shared/cli/spinners.js";
4+
import { createCleanUpFilesCommands } from "../shared/createCleanUpFilesCommands.js";
45
import { doesRepositoryExist } from "../shared/doesRepositoryExist.js";
56
import { GitHubAndOptions } from "../shared/options/readOptions.js";
67
import { addToolAllContributors } from "../steps/addToolAllContributors.js";
@@ -37,11 +38,13 @@ export async function createWithOptions({ github, options }: GitHubAndOptions) {
3738
finalizeDependencies(options),
3839
);
3940

40-
await runCommands("Cleaning up files", [
41-
"pnpm dedupe",
42-
"pnpm lint --fix",
43-
"pnpm format --write",
44-
]);
41+
await runCommands(
42+
"Cleaning up files",
43+
createCleanUpFilesCommands({
44+
bin: !!options.bin,
45+
dedupe: true,
46+
}),
47+
);
4548
}
4649

4750
const sendToGitHub =

src/initialize/initializeWithOptions.ts

+7-4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { withSpinner, withSpinners } from "../shared/cli/spinners.js";
2+
import { createCleanUpFilesCommands } from "../shared/createCleanUpFilesCommands.js";
23
import { GitHubAndOptions } from "../shared/options/readOptions.js";
34
import { addOwnerAsAllContributor } from "../steps/addOwnerAsAllContributor.js";
45
import { clearChangelog } from "../steps/clearChangelog.js";
@@ -60,8 +61,10 @@ export async function initializeWithOptions({
6061
);
6162
}
6263

63-
await runCommands("Cleaning up files", [
64-
"pnpm lint --fix",
65-
"pnpm format --write",
66-
]);
64+
await runCommands(
65+
"Cleaning up files",
66+
createCleanUpFilesCommands({
67+
bin: !!options.bin,
68+
}),
69+
);
6770
}

src/migrate/migrateWithOptions.ts

+8-4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { withSpinner, withSpinners } from "../shared/cli/spinners.js";
2+
import { createCleanUpFilesCommands } from "../shared/createCleanUpFilesCommands.js";
23
import { GitHubAndOptions } from "../shared/options/readOptions.js";
34
import { clearUnnecessaryFiles } from "../steps/clearUnnecessaryFiles.js";
45
import { detectExistingContributors } from "../steps/detectExistingContributors.js";
@@ -60,8 +61,11 @@ export async function migrateWithOptions({
6061
);
6162
}
6263

63-
await runCommands("Cleaning up files", [
64-
"pnpm lint --fix",
65-
"pnpm format --write",
66-
]);
64+
await runCommands(
65+
"Cleaning up files",
66+
createCleanUpFilesCommands({
67+
bin: !!options.bin,
68+
dedupe: true,
69+
}),
70+
);
6771
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { describe, expect, it } from "vitest";
2+
3+
import { createCleanUpFilesCommands } from "./createCleanUpFilesCommands.js";
4+
5+
describe("createCleanUpFilesCommands", () => {
6+
it("only lints and formats when no options are specified", () => {
7+
const actual = createCleanUpFilesCommands({});
8+
9+
expect(actual).toEqual(["pnpm lint --fix", "pnpm format --write"]);
10+
});
11+
12+
it("runs dedupe and build before it lints and formats when both options are specified", () => {
13+
const actual = createCleanUpFilesCommands({
14+
bin: true,
15+
dedupe: true,
16+
});
17+
18+
expect(actual).toEqual([
19+
"pnpm dedupe",
20+
"pnpm build || exit 0",
21+
"pnpm lint --fix",
22+
"pnpm format --write",
23+
]);
24+
});
25+
});
+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
export interface CleanUpFilesOptions {
2+
bin?: boolean;
3+
dedupe?: boolean;
4+
}
5+
6+
export function createCleanUpFilesCommands({
7+
bin,
8+
dedupe,
9+
}: CleanUpFilesOptions) {
10+
return [
11+
// There's no need to dedupe when initializing from the fixed template
12+
...(dedupe ? ["pnpm dedupe"] : []),
13+
// n/no-missing-import rightfully reports on a missing the bin .js file
14+
...(bin ? ["pnpm build || exit 0"] : []),
15+
"pnpm lint --fix",
16+
"pnpm format --write",
17+
];
18+
}

src/steps/writing/creation/dotGitHub/createWorkflows.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const createOptions = (exclude: boolean) =>
77
({
88
access: "public",
99
base: "everything",
10+
bin: exclude ? undefined : "./bin/index.js",
1011
description: "Test description.",
1112
directory: ".",
1213
email: {
@@ -413,7 +414,6 @@ describe("createWorkflows", () => {
413414
steps:
414415
- uses: actions/checkout@v4
415416
- uses: ./.github/actions/prepare
416-
- run: pnpm build || true
417417
- run: pnpm lint
418418
419419
name: Lint

src/steps/writing/creation/dotGitHub/createWorkflows.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ export function createWorkflows(options: Options) {
114114
}),
115115
"lint.yml": createWorkflowFile({
116116
name: "Lint",
117-
runs: ["pnpm build || true", "pnpm lint"],
117+
runs: [...(options.bin ? ["pnpm build || true"] : []), "pnpm lint"],
118118
}),
119119
...(!options.excludeLintKnip && {
120120
"lint-knip.yml": createWorkflowFile({

0 commit comments

Comments
 (0)