Skip to content

Commit

Permalink
fix: improve migration owner replacements (#1078)
Browse files Browse the repository at this point in the history
## PR Checklist

- [x] Addresses an existing open issue: fixes #1043
- [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

Uses a [`to`
callback](https://github.com/adamreisnz/replace-in-file/#using-callbacks-for-to)
to determine whether to swap out owner to `options.owner` in
`"JoshuaKGoldberg"` _(always)_ or `"JoshuaKGoldberg/..."` (if the `...`
is `options.repository`).

Also fixes some existing edge cases around owner replacements:

* Syncs up the end-of-readme attribution notice so there's only one
place it's written in the templates
* Uses `options.owner` and `options.repository` in
`createDotGitHubFiles` instead of hardcoding
`JoshuaKGoldberg/create-typescript-app`.
  • Loading branch information
JoshuaKGoldberg authored Dec 7, 2023
1 parent df82c12 commit 4e25327
Show file tree
Hide file tree
Showing 10 changed files with 146 additions and 33 deletions.
2 changes: 1 addition & 1 deletion script/initialize-test-e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ for (const search of [`/JoshuaKGoldberg/`, "create-typescript-app"]) {
const { stdout } = await $`grep -i ${search} ${files}`;
assert.equal(
stdout,
`README.md:> 💙 This package was templated with [create-typescript-app](https://github.com/JoshuaKGoldberg/create-typescript-app).`,
`README.md:> 💙 This package was templated with [\`create-typescript-app\`](https://github.com/JoshuaKGoldberg/create-typescript-app).`,
);
}

Expand Down
7 changes: 6 additions & 1 deletion src/initialize/initializeWithOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,12 @@ export async function initializeWithOptions({
await updateLocalFiles(options);
},
],
["Updating README.md", updateReadme],
[
"Updating README.md",
async () => {
await updateReadme(options);
},
],
["Clearing changelog", clearChangelog],
[
"Updating all-contributors table",
Expand Down
25 changes: 25 additions & 0 deletions src/steps/createJoshuaKGoldbergReplacement.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { describe, expect, test } from "vitest";

import { createJoshuaKGoldbergReplacement } from "./createJoshuaKGoldbergReplacement.js";

const options = {
owner: "NewOwner",
repository: "new-repository",
};

describe("createJoshuaKGoldbergReplacement", () => {
test.each([
[`JoshuaKGoldberg`, options.owner],
[
`JoshuaKGoldberg/${options.repository}`,
`${options.owner}/${options.repository}`,
],
[`JoshuaKGoldberg/other-repository`, `JoshuaKGoldberg/other-repository`],
])("%s", (before, expected) => {
const [matcher, replacer] = createJoshuaKGoldbergReplacement(options);

const actual = replacer(before, matcher.exec(before)?.[1]);

expect(actual).toBe(expected);
});
});
22 changes: 22 additions & 0 deletions src/steps/createJoshuaKGoldbergReplacement.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { Options } from "../shared/types.js";

/**
* Creates a replace-in-file replacement for JoshuaKGoldberg/... matches,
* keeping repository names not being migrated (e.g. for GitHub actions).
*/
export const createJoshuaKGoldbergReplacement = (
options: Pick<Options, "owner" | "repository">,
) =>
[
/JoshuaKGoldberg(?:\/(.+))?/g,
(full: string, capture: string | undefined) =>
capture
? // If this was a "JoshuaKGoldberg/..." repository link,
// swap the owner if it's the repository being migrated.
capture.startsWith(options.repository)
? `${options.owner}/${capture}`
: full
: // Otherwise it's just "JoshuaKGoldberg" standalone,
// so swap to the new owner.
options.owner,
] as const;
28 changes: 22 additions & 6 deletions src/steps/updateLocalFiles.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,15 @@ describe("updateLocalFiles", () => {
"./.github/**/*",
"./*.*",
],
"from": /JoshuaKGoldberg\\(\\?!\\\\/console-fail-test\\)/g,
"from": /JoshuaKGoldberg\\(\\?:\\\\/\\(\\.\\+\\)\\)\\?/g,
"to": [Function],
},
],
[
{
"allowEmptyPaths": true,
"files": "package.json",
"from": /JoshuaKGoldberg/g,
"to": "StubOwner",
},
],
Expand Down Expand Up @@ -200,8 +208,8 @@ describe("updateLocalFiles", () => {
{
"allowEmptyPaths": true,
"files": "./README.md",
"from": "> 💙 This package is based on [@StubOwner](https://github.com/StubOwner)'s [stub-repository](https://github.com/JoshuaKGoldberg/stub-repository).",
"to": "> 💙 This package is based on [@JoshuaKGoldberg](https://github.com/JoshuaKGoldberg)'s [create-typescript-app](https://github.com/JoshuaKGoldberg/create-typescript-app).",
"from": /> 💙 This package was templated with \\.\\+\\\\\\./g,
"to": "> 💙 This package was templated with [\`create-typescript-app\`](https://github.com/JoshuaKGoldberg/create-typescript-app).",
},
],
]
Expand Down Expand Up @@ -234,7 +242,15 @@ describe("updateLocalFiles", () => {
"./.github/**/*",
"./*.*",
],
"from": /JoshuaKGoldberg\\(\\?!\\\\/console-fail-test\\)/g,
"from": /JoshuaKGoldberg\\(\\?:\\\\/\\(\\.\\+\\)\\)\\?/g,
"to": [Function],
},
],
[
{
"allowEmptyPaths": true,
"files": "package.json",
"from": /JoshuaKGoldberg/g,
"to": "StubOwner",
},
],
Expand Down Expand Up @@ -359,8 +375,8 @@ describe("updateLocalFiles", () => {
{
"allowEmptyPaths": true,
"files": "./README.md",
"from": "> 💙 This package is based on [@StubOwner](https://github.com/StubOwner)'s [stub-repository](https://github.com/JoshuaKGoldberg/stub-repository).",
"to": "> 💙 This package is based on [@JoshuaKGoldberg](https://github.com/JoshuaKGoldberg)'s [create-typescript-app](https://github.com/JoshuaKGoldberg/create-typescript-app).",
"from": /> 💙 This package was templated with \\.\\+\\\\\\./g,
"to": "> 💙 This package was templated with [\`create-typescript-app\`](https://github.com/JoshuaKGoldberg/create-typescript-app).",
},
],
]
Expand Down
12 changes: 8 additions & 4 deletions src/steps/updateLocalFiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import replaceInFile from "replace-in-file";

import { readFileSafeAsJson } from "../shared/readFileSafeAsJson.js";
import { Options } from "../shared/types.js";
import { createJoshuaKGoldbergReplacement } from "./createJoshuaKGoldbergReplacement.js";
import { endOfReadmeTemplateLine } from "./updateReadme.js";

interface ExistingPackageData {
description?: string;
Expand All @@ -14,7 +16,8 @@ export async function updateLocalFiles(options: Options) {

const replacements = [
[/Create TypeScript App/g, options.title],
[/JoshuaKGoldberg(?!\/console-fail-test)/g, options.owner],
createJoshuaKGoldbergReplacement(options),
[/JoshuaKGoldberg/g, options.owner, "package.json"],
[/create-typescript-app/g, options.repository],
[/\/\*\n.+\*\/\n\n/gs, ``, ".eslintrc.cjs"],
[/"author": ".+"/g, `"author": "${options.author}"`, "./package.json"],
Expand All @@ -37,8 +40,8 @@ export async function updateLocalFiles(options: Options) {
[`["src/**/*.ts!", "script/**/*.js"]`, `"src/**/*.ts!"`, "./knip.jsonc"],
// Edge case: migration scripts will rewrite README.md attribution
[
`> 💙 This package is based on [@${options.owner}](https://github.com/${options.owner})'s [${options.repository}](https://github.com/JoshuaKGoldberg/${options.repository}).`,
`> 💙 This package is based on [@JoshuaKGoldberg](https://github.com/JoshuaKGoldberg)'s [create-typescript-app](https://github.com/JoshuaKGoldberg/create-typescript-app).`,
/> 💙 This package was templated with .+\./g,
endOfReadmeTemplateLine,
"./README.md",
],
];
Expand All @@ -65,8 +68,9 @@ export async function updateLocalFiles(options: Options) {
to,
});
} catch (error) {
const toString = typeof to === "function" ? "(function)" : to;
throw new Error(
`Failed to replace ${from.toString()} with ${to} in ${files.toString()}`,
`Failed to replace ${from.toString()} with ${toString} in ${files.toString()}`,
{
cause: error,
},
Expand Down
58 changes: 46 additions & 12 deletions src/steps/updateReadme.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import { describe, expect, it, vi } from "vitest";

import { updateReadme } from "./updateReadme.js";

const mockAppendFile = vi.fn();
const mockWriteFile = vi.fn();

vi.mock("node:fs/promises", () => ({
default: {
get appendFile() {
return mockAppendFile;
get writeFile() {
return mockWriteFile;
},
},
}));
Expand All @@ -20,20 +20,26 @@ vi.mock("../shared/readFileSafe.js", () => ({
},
}));

const options = {
owner: "NewOwner",
};

describe("updateReadme", () => {
it("adds a notice when the file does not contain it already", async () => {
mockReadFileSafe.mockResolvedValue("");
mockReadFileSafe.mockResolvedValue(
"Existing JoshuaKGoldberg/create-typescript-app content.",
);

await updateReadme();
await updateReadme(options);

expect(mockAppendFile.mock.calls).toMatchInlineSnapshot(`
expect(mockWriteFile.mock.calls).toMatchInlineSnapshot(`
[
[
"./README.md",
"
"Existing NewOwner/create-typescript-app content.
<!-- You can remove this notice if you don't want it 🙂 no worries! -->
> 💙 This package was templated with [create-typescript-app](https://github.com/JoshuaKGoldberg/create-typescript-app).
> 💙 This package was templated with [\`create-typescript-app\`](https://github.com/JoshuaKGoldberg/create-typescript-app).
",
],
]
Expand All @@ -42,23 +48,51 @@ describe("updateReadme", () => {

it("doesn't add a notice when the file contains it already", async () => {
mockReadFileSafe.mockResolvedValue(`
Existing JoshuaKGoldberg/create-typescript-app content.
<!-- You can remove this notice if you don't want it 🙂 no worries! -->
> 💙 This package was templated using [create-typescript-app](https://github.com/JoshuaKGoldberg/create-typescript-app).
`);

await updateReadme();
await updateReadme(options);

expect(mockAppendFile.mock.calls).toMatchInlineSnapshot("[]");
expect(mockWriteFile.mock.calls).toMatchInlineSnapshot(`
[
[
"./README.md",
"
Existing NewOwner/create-typescript-app content.
<!-- You can remove this notice if you don't want it 🙂 no worries! -->
> 💙 This package was templated using [create-typescript-app](https://github.com/NewOwner/create-typescript-app).
",
],
]
`);
});

it("doesn't add a notice when the file contains an older version of it already", async () => {
mockReadFileSafe.mockResolvedValue(`
Existing JoshuaKGoldberg/create-typescript-app content.
💙 This package is based on [@JoshuaKGoldberg](https://github.com/JoshuaKGoldberg)'s [create-typescript-app](https://github.com/JoshuaKGoldberg/create-typescript-app).
`);

await updateReadme();
await updateReadme(options);

expect(mockAppendFile.mock.calls).toMatchInlineSnapshot("[]");
expect(mockWriteFile.mock.calls).toMatchInlineSnapshot(`
[
[
"./README.md",
"
Existing NewOwner/create-typescript-app content.
💙 This package is based on [@NewOwner](https://github.com/NewOwner)'s [create-typescript-app](https://github.com/NewOwner/create-typescript-app).
",
],
]
`);
});
});
15 changes: 11 additions & 4 deletions src/steps/updateReadme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,29 @@ import fs from "node:fs/promises";
import { EOL } from "node:os";

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

export const endOfReadmeTemplateLine = `> 💙 This package was templated with [\`create-typescript-app\`](https://github.com/JoshuaKGoldberg/create-typescript-app).`;

export const endOfReadmeNotice = [
``,
`<!-- You can remove this notice if you don't want it 🙂 no worries! -->`,
``,
`> 💙 This package was templated with [create-typescript-app](https://github.com/JoshuaKGoldberg/create-typescript-app).`,
endOfReadmeTemplateLine,
``,
].join(EOL);

export const endOfReadmeMatcher =
/💙.+(?:based|built|templated).+(?:from|using|on|with).+create-typescript-app/;

export async function updateReadme() {
const contents = await readFileSafe("./README.md", "");
export async function updateReadme(options: Pick<Options, "owner">) {
let contents = await readFileSafe("./README.md", "");

contents = contents.replaceAll("JoshuaKGoldberg", options.owner);

if (!endOfReadmeMatcher.test(contents)) {
await fs.appendFile("./README.md", endOfReadmeNotice);
contents += endOfReadmeNotice;
}

await fs.writeFile("./README.md", contents);
}
6 changes: 3 additions & 3 deletions src/steps/writeReadme/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ describe("writeReadme", () => {
<!-- You can remove this notice if you don't want it 🙂 no worries! -->
> 💙 This package was templated with [create-typescript-app](https://github.com/JoshuaKGoldberg/create-typescript-app).
> 💙 This package was templated with [\`create-typescript-app\`](https://github.com/JoshuaKGoldberg/create-typescript-app).
",
],
]
Expand Down Expand Up @@ -156,7 +156,7 @@ describe("writeReadme", () => {
<!-- You can remove this notice if you don't want it 🙂 no worries! -->
> 💙 This package was templated with [create-typescript-app](https://github.com/JoshuaKGoldberg/create-typescript-app).
> 💙 This package was templated with [\`create-typescript-app\`](https://github.com/JoshuaKGoldberg/create-typescript-app).
",
],
]
Expand Down Expand Up @@ -220,7 +220,7 @@ describe("writeReadme", () => {
<!-- You can remove this notice if you don't want it 🙂 no worries! -->
> 💙 This package was templated with [create-typescript-app](https://github.com/JoshuaKGoldberg/create-typescript-app).
> 💙 This package was templated with [\`create-typescript-app\`](https://github.com/JoshuaKGoldberg/create-typescript-app).
",
],
]
Expand Down
4 changes: 2 additions & 2 deletions src/steps/writing/creation/dotGitHub/createDotGitHubFiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,8 @@ There are two steps involved:
### Finding an Issue
With the exception of very small typos, all changes to this repository generally need to correspond to an [unassigned open issue marked as \`status: accepting prs\` on the issue tracker](https://github.com/JoshuaKGoldberg/create-typescript-app/issues?q=is%3Aissue+is%3Aopen+label%3A%22status%3A+accepting+prs%22+no%3Aassignee+).
If this is your first time contributing, consider searching for [unassigned issues that also have the \`good first issue\` label](https://github.com/JoshuaKGoldberg/create-typescript-app/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22+label%3A%22status%3A+accepting+prs%22+no%3Aassignee+).
With the exception of very small typos, all changes to this repository generally need to correspond to an [unassigned open issue marked as \`status: accepting prs\` on the issue tracker](https://github.com/${options.owner}/${options.repository}/issues?q=is%3Aissue+is%3Aopen+label%3A%22status%3A+accepting+prs%22+no%3Aassignee+).
If this is your first time contributing, consider searching for [unassigned issues that also have the \`good first issue\` label](https://github.com/${options.owner}/${options.repository}/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22+label%3A%22status%3A+accepting+prs%22+no%3Aassignee+).
If the issue you'd like to fix isn't found on the issue, see [Reporting Issues](#reporting-issues) for filing your own (please do!).
#### Issue Claiming
Expand Down

0 comments on commit 4e25327

Please sign in to comment.