Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
256 changes: 256 additions & 0 deletions packages/cli-core/src/commands/init/format.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
import { test, expect, describe, beforeEach, afterEach } from "bun:test";
import { join } from "node:path";
import { mkdtemp, rm } from "node:fs/promises";
import { tmpdir } from "node:os";
import { runFormatters } from "./format.ts";
import type { ProjectContext } from "./frameworks/types.js";

type SpawnArgs = readonly string[];

const origSpawn = Bun.spawn;
const origWhich = Bun.which;
const origSpawnSync = Bun.spawnSync;

type SpawnImpl = (
cmd: SpawnArgs,
opts?: { cwd?: string; stdout?: unknown; stderr?: unknown },
) => { exited: Promise<number> };

function setSpawn(impl: SpawnImpl) {
try {
(Bun as unknown as { spawn: SpawnImpl }).spawn = impl;
} catch {
// Bun.spawn may not be writable on some runtimes
}
}
function restoreSpawn() {
try {
(Bun as unknown as { spawn: typeof Bun.spawn }).spawn = origSpawn;
} catch {
// ignore
}
}

function mockWhich(present: ReadonlySet<string>) {
Comment on lines +22 to +34

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Silent mock failures make tests untrustworthy

The try/catch blocks silently swallow mock-assignment failures. If Bun makes these properties non-writable in a future version, every test runs against real globals with confusing results rather than failing fast.

Same issue in mockWhich (line 42) and mockSpawnSync (line 55) and all three restore* functions.

Suggestion — drop all try/catch blocks, verify the assignment took effect:

functionsetSpawn(impl: SpawnImpl){(Bunasunknownas{spawn: SpawnImpl}).spawn=impl;if(Bun.spawn!==(implasunknown)){thrownewError("Failed to mock Bun.spawn — property may be non-writable");}}functionrestoreSpawn(){(Bunasunknownas{spawn: typeofBun.spawn}).spawn=origSpawn;}

Bonus — extract the cast once to reduce boilerplate (repeated 6× as-is):

constbunOverrides=Bunasunknownas{spawn: SpawnImpl;which: (bin: string)=>string|null;spawnSync: (cmd: string[])=>{exitCode: number};};functionsetSpawn(impl: SpawnImpl){bunOverrides.spawn=impl;}functionrestoreSpawn(){bunOverrides.spawn=origSpawnasunknownasSpawnImpl;}// ... etc

try {
(Bun as unknown as { which: (bin: string) => string | null }).which = (bin) =>
present.has(bin) ? `/usr/local/bin/${bin}` : null;
} catch {
// ignore
}
}
function restoreWhich() {
try {
(Bun as unknown as { which: typeof Bun.which }).which = origWhich;
} catch {
// ignore
}
}

function mockSpawnSync(yarnDlxExitCode: number) {
try {
(Bun as unknown as { spawnSync: (cmd: string[]) => { exitCode: number } }).spawnSync = (
cmd,
) => {
if (cmd[0] === "yarn" && cmd[1] === "dlx") return { exitCode: yarnDlxExitCode };
return { exitCode: 0 };
};
} catch {
// ignore
}
}
function restoreSpawnSync() {
try {
(Bun as unknown as { spawnSync: typeof Bun.spawnSync }).spawnSync = origSpawnSync;
} catch {
// ignore
}
}

/** Minimal ProjectContext suitable for driving runFormatters. */
function makeCtx(overrides: Partial<ProjectContext> & { cwd: string }): ProjectContext {
return {
framework: {
name: "next",
sdk: "@clerk/nextjs",
dep: "next",
envFile: ".env.local",
} as ProjectContext["framework"],
typescript: true,
srcDir: false,
packageManager: "bun",
existingClerk: false,
deps: {},
envFile: ".env.local",
...overrides,
};
}

describe("runFormatters", () => {
let tempDir: string;
let spawnCalls: SpawnArgs[];

beforeEach(async () => {
tempDir = await mkdtemp(join(tmpdir(), "clerk-format-"));
spawnCalls = [];
setSpawn((cmd) => {
spawnCalls.push(cmd);
return { exited: Promise.resolve(0) };
});
mockWhich(new Set(["bunx", "npx", "pnpm", "yarn"]));
mockSpawnSync(0);
});

afterEach(async () => {
restoreSpawn();
restoreWhich();
restoreSpawnSync();
await rm(tempDir, { recursive: true, force: true });
});

test("no-op when files is empty", async () => {
const ctx = makeCtx({ cwd: tempDir, deps: { prettier: "3.0.0" } });
await runFormatters(ctx, []);
expect(spawnCalls).toHaveLength(0);
});

test("no-op when no supported formatter is in deps", async () => {
const ctx = makeCtx({ cwd: tempDir, deps: { next: "15.0.0" } });
await runFormatters(ctx, ["a.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("runs prettier via the package-manager's preferred runner (bun → bunx)", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["src/a.ts", "src/b.ts"]);
expect(spawnCalls).toEqual([
["bunx", "prettier", "--ignore-unknown", "--write", "src/a.ts", "src/b.ts"],
]);
});

test("runs biome via pnpm dlx when packageManager is pnpm", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "pnpm",
deps: { "@biomejs/biome": "1.9.0" },
});
await runFormatters(ctx, ["src/a.ts"]);
expect(spawnCalls).toEqual([
["pnpm", "dlx", "@biomejs/biome", "format", "--write", "src/a.ts"],
]);
});

test("runs both prettier and biome when both are in deps, in FORMATTERS order", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "npm",
deps: { prettier: "3.0.0", "@biomejs/biome": "1.9.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([
["npx", "prettier", "--ignore-unknown", "--write", "x.ts"],
["npx", "@biomejs/biome", "format", "--write", "x.ts"],
]);
});

test("falls back to the first available runner when the pm's runner is missing", async () => {
// Project says bun, but only npx/pnpm are on PATH.
mockWhich(new Set(["npx", "pnpm"]));
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([["npx", "prettier", "--ignore-unknown", "--write", "x.ts"]]);
});

test("skips silently when no runners are on PATH", async () => {
mockWhich(new Set());
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("excludes yarn when yarn dlx probe fails (Yarn Classic)", async () => {
mockWhich(new Set(["yarn"]));
mockSpawnSync(1); // yarn dlx --help exits non-zero
const ctx = makeCtx({
cwd: tempDir,
packageManager: "yarn",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("swallows spawn errors (best-effort) and continues to later formatters", async () => {
const attempted: SpawnArgs[] = [];
setSpawn((cmd) => {
attempted.push(cmd);
if (cmd[1] === "prettier") {
throw new Error("spawn failed");
}
return { exited: Promise.resolve(0) };
});
const ctx = makeCtx({
cwd: tempDir,
packageManager: "npm",
deps: { prettier: "3.0.0", "@biomejs/biome": "1.9.0" },
});
// Should not throw even though prettier spawn blows up.
await runFormatters(ctx, ["x.ts"]);
expect(attempted).toEqual([
["npx", "prettier", "--ignore-unknown", "--write", "x.ts"],
["npx", "@biomejs/biome", "format", "--write", "x.ts"],
]);
});

test("reads deps from disk when ctx.deps is empty", async () => {
await Bun.write(
join(tempDir, "package.json"),
JSON.stringify({ dependencies: { prettier: "3.0.0" } }),
);
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: {}, // empty → triggers disk fallback
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([["bunx", "prettier", "--ignore-unknown", "--write", "x.ts"]]);
});

test("no-op when ctx.deps is empty and package.json is missing", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: {},
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("spawns in the project cwd", async () => {
let seenCwd: string | undefined;
setSpawn((cmd, opts?: { cwd?: string }) => {
seenCwd = opts?.cwd;
spawnCalls.push(cmd);
return { exited: Promise.resolve(0) };
});
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(seenCwd).toBe(tempDir);
});
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing test: non-zero formatter exit code

Tests cover spawn throwing (swallowed correctly) and spawn succeeding (exit 0), but there's no test for a formatter exiting non-zero. The current code silently ignores exit codes (await proc.exited with no check), which is the right best-effort behavior — worth locking in with a test:

test("ignores non-zero exit code from formatter (best-effort)",async()=>{setSpawn((cmd)=>{spawnCalls.push(cmd);return{exited: Promise.resolve(1)};});constctx=makeCtx({cwd: tempDir,packageManager: "bun",deps: {prettier: "3.0.0","@biomejs/biome": "1.9.0"},});awaitrunFormatters(ctx,["x.ts"]);// Both formatters attempted despite prettier exiting non-zeroexpect(spawnCalls).toHaveLength(2);});

49 changes: 36 additions & 13 deletions packages/cli-core/src/commands/init/format.ts
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,58 @@
import { readDeps } from "./context.js";
// Pulls in the same runner detection skills.ts uses, so a bun project with
// no `npx` on PATH (entirely possible if the user installed Bun via Homebrew
// but never installed Node) will fall back to bunx instead of silently failing.
import { detectAvailableRunners, preferredRunner, runnerCommand } from "../../lib/runners.js";
Comment on lines +2 to +5

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This reads like commit-message rationale rather than a code comment. The function-level docstring already covers the best-effort / silent-failure contract, and the import names (detectAvailableRunners, preferredRunner) are self-documenting.

Suggestion: delete this comment block entirely — the PR description is the right place for the "why we're switching" context.

import type { ProjectContext } from "./frameworks/types.js";

type FormatterConfig = {
pkg: string;
args: (files: string[]) => string[];
/** Args after the runner: binary + flags + files. The runner is prepended at spawn time. */
binArgs: (files: string[]) => string[];
};

const FORMATTERS: FormatterConfig[] = [
{
pkg: "prettier",
args: (files) => ["npx", "prettier", "--ignore-unknown", "--write", ...files],
binArgs: (files) => ["prettier", "--ignore-unknown", "--write", ...files],
},
{
pkg: "@biomejs/biome",
args: (files) => ["npx", "@biomejs/biome", "format", "--write", ...files],
binArgs: (files) => ["@biomejs/biome", "format", "--write", ...files],
},
];

export async function runFormatters(cwd: string, files: string[]): Promise<void> {
/**
* Format scaffolded files with prettier or biome (whichever the project uses).
*
* Best-effort: failures are silent (stdio ignored, spawn errors swallowed)
* because formatting is purely cosmetic and shouldn't break init.
*/
export async function runFormatters(ctx: ProjectContext, files: string[]): Promise<void> {
if (files.length === 0) return;

const deps = await readDeps(cwd);
const deps = ctx.deps && Object.keys(ctx.deps).length > 0 ? ctx.deps : await readDeps(ctx.cwd);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two things on this line:

1. ctx.deps && is dead codedeps is typed Record<string, string> (not optional), and gatherContext always sets it to deps ?? {}. An empty object is truthy, so the && never short-circuits. Simplify to:

constdeps=Object.keys(ctx.deps).length>0 ? ctx.deps : awaitreadDeps(ctx.cwd);

2. The disk fallback itself is redundant in practicegatherContext() already calls readDeps(cwd) and stores the result. If it returned null, ctx.deps becomes {}. Re-calling readDeps here will return the same null. The only scenario this fallback helps is when someone constructs a ProjectContext manually with deps: {} while a real package.json exists — which only happens in the test for this very code path.

Consider simplifying to just trust ctx.deps:

exportasyncfunctionrunFormatters(ctx: ProjectContext,files: string[]): Promise<void>{if(files.length===0)return;if(Object.keys(ctx.deps).length===0)return;constmatchingFormatters=FORMATTERS.filter((f)=>f.pkginctx.deps);
...

Then update the "reads deps from disk when ctx.deps is empty" test to pass deps: { prettier: "3.0.0" } directly, and drop the "no-op when ctx.deps is empty and package.json is missing" test (already covered by "no-op when no supported formatter is in deps").

if (!deps) return;

for (const formatter of FORMATTERS) {
if (!(formatter.pkg in deps)) continue;
const matchingFormatters = FORMATTERS.filter((f) => f.pkg in deps);
if (matchingFormatters.length === 0) return;

const proc = Bun.spawn(formatter.args(files), {
cwd,
stdout: "ignore",
stderr: "ignore",
});
await proc.exited;
const available = detectAvailableRunners();
if (available.length === 0) return;
const runner = preferredRunner(ctx.packageManager, available);
if (!runner) return;

for (const formatter of matchingFormatters) {
const command = runnerCommand(runner, formatter.binArgs(files));
try {
const proc = Bun.spawn(command, {
cwd: ctx.cwd,
stdout: "ignore",
stderr: "ignore",
});
await proc.exited;
} catch {
// Best-effort, see function doc comment.
}
}
}
2 changes: 1 addition & 1 deletion packages/cli-core/src/commands/init/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -321,7 +321,7 @@ async function scaffoldAndWrite(
}

const writtenFiles = await writePlan(cwd, plan);
await runFormatters(cwd, writtenFiles);
await runFormatters(ctx, writtenFiles);

const findings = await withSpinner("Scanning for issues...", () =>
scanForIssues(cwd, ctx.framework.dep),
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
256 changes: 256 additions & 0 deletions packages/cli-core/src/commands/init/format.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
import { test, expect, describe, beforeEach, afterEach } from "bun:test";
import { join } from "node:path";
import { mkdtemp, rm } from "node:fs/promises";
import { tmpdir } from "node:os";
import { runFormatters } from "./format.ts";
import type { ProjectContext } from "./frameworks/types.js";

type SpawnArgs = readonly string[];

const origSpawn = Bun.spawn;
const origWhich = Bun.which;
const origSpawnSync = Bun.spawnSync;

type SpawnImpl = (
cmd: SpawnArgs,
opts?: { cwd?: string; stdout?: unknown; stderr?: unknown },
) => { exited: Promise<number> };

function setSpawn(impl: SpawnImpl) {
try {
(Bun as unknown as { spawn: SpawnImpl }).spawn = impl;
} catch {
// Bun.spawn may not be writable on some runtimes
}
}
function restoreSpawn() {
try {
(Bun as unknown as { spawn: typeof Bun.spawn }).spawn = origSpawn;
} catch {
// ignore
}
}

function mockWhich(present: ReadonlySet<string>) {
Comment on lines +22 to +34

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Silent mock failures make tests untrustworthy

The try/catch blocks silently swallow mock-assignment failures. If Bun makes these properties non-writable in a future version, every test runs against real globals with confusing results rather than failing fast.

Same issue in mockWhich (line 42) and mockSpawnSync (line 55) and all three restore* functions.

Suggestion — drop all try/catch blocks, verify the assignment took effect:

functionsetSpawn(impl: SpawnImpl){(Bunasunknownas{spawn: SpawnImpl}).spawn=impl;if(Bun.spawn!==(implasunknown)){thrownewError("Failed to mock Bun.spawn — property may be non-writable");}}functionrestoreSpawn(){(Bunasunknownas{spawn: typeofBun.spawn}).spawn=origSpawn;}

Bonus — extract the cast once to reduce boilerplate (repeated 6× as-is):

constbunOverrides=Bunasunknownas{spawn: SpawnImpl;which: (bin: string)=>string|null;spawnSync: (cmd: string[])=>{exitCode: number};};functionsetSpawn(impl: SpawnImpl){bunOverrides.spawn=impl;}functionrestoreSpawn(){bunOverrides.spawn=origSpawnasunknownasSpawnImpl;}// ... etc

try {
(Bun as unknown as { which: (bin: string) => string | null }).which = (bin) =>
present.has(bin) ? `/usr/local/bin/${bin}` : null;
} catch {
// ignore
}
}
function restoreWhich() {
try {
(Bun as unknown as { which: typeof Bun.which }).which = origWhich;
} catch {
// ignore
}
}

function mockSpawnSync(yarnDlxExitCode: number) {
try {
(Bun as unknown as { spawnSync: (cmd: string[]) => { exitCode: number } }).spawnSync = (
cmd,
) => {
if (cmd[0] === "yarn" && cmd[1] === "dlx") return { exitCode: yarnDlxExitCode };
return { exitCode: 0 };
};
} catch {
// ignore
}
}
function restoreSpawnSync() {
try {
(Bun as unknown as { spawnSync: typeof Bun.spawnSync }).spawnSync = origSpawnSync;
} catch {
// ignore
}
}

/** Minimal ProjectContext suitable for driving runFormatters. */
function makeCtx(overrides: Partial<ProjectContext> & { cwd: string }): ProjectContext {
return {
framework: {
name: "next",
sdk: "@clerk/nextjs",
dep: "next",
envFile: ".env.local",
} as ProjectContext["framework"],
typescript: true,
srcDir: false,
packageManager: "bun",
existingClerk: false,
deps: {},
envFile: ".env.local",
...overrides,
};
}

describe("runFormatters", () => {
let tempDir: string;
let spawnCalls: SpawnArgs[];

beforeEach(async () => {
tempDir = await mkdtemp(join(tmpdir(), "clerk-format-"));
spawnCalls = [];
setSpawn((cmd) => {
spawnCalls.push(cmd);
return { exited: Promise.resolve(0) };
});
mockWhich(new Set(["bunx", "npx", "pnpm", "yarn"]));
mockSpawnSync(0);
});

afterEach(async () => {
restoreSpawn();
restoreWhich();
restoreSpawnSync();
await rm(tempDir, { recursive: true, force: true });
});

test("no-op when files is empty", async () => {
const ctx = makeCtx({ cwd: tempDir, deps: { prettier: "3.0.0" } });
await runFormatters(ctx, []);
expect(spawnCalls).toHaveLength(0);
});

test("no-op when no supported formatter is in deps", async () => {
const ctx = makeCtx({ cwd: tempDir, deps: { next: "15.0.0" } });
await runFormatters(ctx, ["a.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("runs prettier via the package-manager's preferred runner (bun → bunx)", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["src/a.ts", "src/b.ts"]);
expect(spawnCalls).toEqual([
["bunx", "prettier", "--ignore-unknown", "--write", "src/a.ts", "src/b.ts"],
]);
});

test("runs biome via pnpm dlx when packageManager is pnpm", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "pnpm",
deps: { "@biomejs/biome": "1.9.0" },
});
await runFormatters(ctx, ["src/a.ts"]);
expect(spawnCalls).toEqual([
["pnpm", "dlx", "@biomejs/biome", "format", "--write", "src/a.ts"],
]);
});

test("runs both prettier and biome when both are in deps, in FORMATTERS order", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "npm",
deps: { prettier: "3.0.0", "@biomejs/biome": "1.9.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([
["npx", "prettier", "--ignore-unknown", "--write", "x.ts"],
["npx", "@biomejs/biome", "format", "--write", "x.ts"],
]);
});

test("falls back to the first available runner when the pm's runner is missing", async () => {
// Project says bun, but only npx/pnpm are on PATH.
mockWhich(new Set(["npx", "pnpm"]));
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([["npx", "prettier", "--ignore-unknown", "--write", "x.ts"]]);
});

test("skips silently when no runners are on PATH", async () => {
mockWhich(new Set());
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("excludes yarn when yarn dlx probe fails (Yarn Classic)", async () => {
mockWhich(new Set(["yarn"]));
mockSpawnSync(1); // yarn dlx --help exits non-zero
const ctx = makeCtx({
cwd: tempDir,
packageManager: "yarn",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("swallows spawn errors (best-effort) and continues to later formatters", async () => {
const attempted: SpawnArgs[] = [];
setSpawn((cmd) => {
attempted.push(cmd);
if (cmd[1] === "prettier") {
throw new Error("spawn failed");
}
return { exited: Promise.resolve(0) };
});
const ctx = makeCtx({
cwd: tempDir,
packageManager: "npm",
deps: { prettier: "3.0.0", "@biomejs/biome": "1.9.0" },
});
// Should not throw even though prettier spawn blows up.
await runFormatters(ctx, ["x.ts"]);
expect(attempted).toEqual([
["npx", "prettier", "--ignore-unknown", "--write", "x.ts"],
["npx", "@biomejs/biome", "format", "--write", "x.ts"],
]);
});

test("reads deps from disk when ctx.deps is empty", async () => {
await Bun.write(
join(tempDir, "package.json"),
JSON.stringify({ dependencies: { prettier: "3.0.0" } }),
);
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: {}, // empty → triggers disk fallback
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([["bunx", "prettier", "--ignore-unknown", "--write", "x.ts"]]);
});

test("no-op when ctx.deps is empty and package.json is missing", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: {},
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("spawns in the project cwd", async () => {
let seenCwd: string | undefined;
setSpawn((cmd, opts?: { cwd?: string }) => {
seenCwd = opts?.cwd;
spawnCalls.push(cmd);
return { exited: Promise.resolve(0) };
});
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(seenCwd).toBe(tempDir);
});
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing test: non-zero formatter exit code

Tests cover spawn throwing (swallowed correctly) and spawn succeeding (exit 0), but there's no test for a formatter exiting non-zero. The current code silently ignores exit codes (await proc.exited with no check), which is the right best-effort behavior — worth locking in with a test:

test("ignores non-zero exit code from formatter (best-effort)",async()=>{setSpawn((cmd)=>{spawnCalls.push(cmd);return{exited: Promise.resolve(1)};});constctx=makeCtx({cwd: tempDir,packageManager: "bun",deps: {prettier: "3.0.0","@biomejs/biome": "1.9.0"},});awaitrunFormatters(ctx,["x.ts"]);// Both formatters attempted despite prettier exiting non-zeroexpect(spawnCalls).toHaveLength(2);});

49 changes: 36 additions & 13 deletions packages/cli-core/src/commands/init/format.ts
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,58 @@
import { readDeps } from "./context.js";
// Pulls in the same runner detection skills.ts uses, so a bun project with
// no `npx` on PATH (entirely possible if the user installed Bun via Homebrew
// but never installed Node) will fall back to bunx instead of silently failing.
import { detectAvailableRunners, preferredRunner, runnerCommand } from "../../lib/runners.js";
Comment on lines +2 to +5

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This reads like commit-message rationale rather than a code comment. The function-level docstring already covers the best-effort / silent-failure contract, and the import names (detectAvailableRunners, preferredRunner) are self-documenting.

Suggestion: delete this comment block entirely — the PR description is the right place for the "why we're switching" context.

import type { ProjectContext } from "./frameworks/types.js";

type FormatterConfig = {
pkg: string;
args: (files: string[]) => string[];
/** Args after the runner: binary + flags + files. The runner is prepended at spawn time. */
binArgs: (files: string[]) => string[];
};

const FORMATTERS: FormatterConfig[] = [
{
pkg: "prettier",
args: (files) => ["npx", "prettier", "--ignore-unknown", "--write", ...files],
binArgs: (files) => ["prettier", "--ignore-unknown", "--write", ...files],
},
{
pkg: "@biomejs/biome",
args: (files) => ["npx", "@biomejs/biome", "format", "--write", ...files],
binArgs: (files) => ["@biomejs/biome", "format", "--write", ...files],
},
];

export async function runFormatters(cwd: string, files: string[]): Promise<void> {
/**
* Format scaffolded files with prettier or biome (whichever the project uses).
*
* Best-effort: failures are silent (stdio ignored, spawn errors swallowed)
* because formatting is purely cosmetic and shouldn't break init.
*/
export async function runFormatters(ctx: ProjectContext, files: string[]): Promise<void> {
if (files.length === 0) return;

const deps = await readDeps(cwd);
const deps = ctx.deps && Object.keys(ctx.deps).length > 0 ? ctx.deps : await readDeps(ctx.cwd);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two things on this line:

1. ctx.deps && is dead codedeps is typed Record<string, string> (not optional), and gatherContext always sets it to deps ?? {}. An empty object is truthy, so the && never short-circuits. Simplify to:

constdeps=Object.keys(ctx.deps).length>0 ? ctx.deps : awaitreadDeps(ctx.cwd);

2. The disk fallback itself is redundant in practicegatherContext() already calls readDeps(cwd) and stores the result. If it returned null, ctx.deps becomes {}. Re-calling readDeps here will return the same null. The only scenario this fallback helps is when someone constructs a ProjectContext manually with deps: {} while a real package.json exists — which only happens in the test for this very code path.

Consider simplifying to just trust ctx.deps:

exportasyncfunctionrunFormatters(ctx: ProjectContext,files: string[]): Promise<void>{if(files.length===0)return;if(Object.keys(ctx.deps).length===0)return;constmatchingFormatters=FORMATTERS.filter((f)=>f.pkginctx.deps);
...

Then update the "reads deps from disk when ctx.deps is empty" test to pass deps: { prettier: "3.0.0" } directly, and drop the "no-op when ctx.deps is empty and package.json is missing" test (already covered by "no-op when no supported formatter is in deps").

if (!deps) return;

for (const formatter of FORMATTERS) {
if (!(formatter.pkg in deps)) continue;
const matchingFormatters = FORMATTERS.filter((f) => f.pkg in deps);
if (matchingFormatters.length === 0) return;

const proc = Bun.spawn(formatter.args(files), {
cwd,
stdout: "ignore",
stderr: "ignore",
});
await proc.exited;
const available = detectAvailableRunners();
if (available.length === 0) return;
const runner = preferredRunner(ctx.packageManager, available);
if (!runner) return;

for (const formatter of matchingFormatters) {
const command = runnerCommand(runner, formatter.binArgs(files));
try {
const proc = Bun.spawn(command, {
cwd: ctx.cwd,
stdout: "ignore",
stderr: "ignore",
});
await proc.exited;
} catch {
// Best-effort, see function doc comment.
}
}
}
2 changes: 1 addition & 1 deletion packages/cli-core/src/commands/init/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -321,7 +321,7 @@ async function scaffoldAndWrite(
}

const writtenFiles = await writePlan(cwd, plan);
await runFormatters(cwd, writtenFiles);
await runFormatters(ctx, writtenFiles);

const findings = await withSpinner("Scanning for issues...", () =>
scanForIssues(cwd, ctx.framework.dep),
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
256 changes: 256 additions & 0 deletions packages/cli-core/src/commands/init/format.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
import { test, expect, describe, beforeEach, afterEach } from "bun:test";
import { join } from "node:path";
import { mkdtemp, rm } from "node:fs/promises";
import { tmpdir } from "node:os";
import { runFormatters } from "./format.ts";
import type { ProjectContext } from "./frameworks/types.js";

type SpawnArgs = readonly string[];

const origSpawn = Bun.spawn;
const origWhich = Bun.which;
const origSpawnSync = Bun.spawnSync;

type SpawnImpl = (
cmd: SpawnArgs,
opts?: { cwd?: string; stdout?: unknown; stderr?: unknown },
) => { exited: Promise<number> };

function setSpawn(impl: SpawnImpl) {
try {
(Bun as unknown as { spawn: SpawnImpl }).spawn = impl;
} catch {
// Bun.spawn may not be writable on some runtimes
}
}
function restoreSpawn() {
try {
(Bun as unknown as { spawn: typeof Bun.spawn }).spawn = origSpawn;
} catch {
// ignore
}
}

function mockWhich(present: ReadonlySet<string>) {
Comment on lines +22 to +34

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Silent mock failures make tests untrustworthy

The try/catch blocks silently swallow mock-assignment failures. If Bun makes these properties non-writable in a future version, every test runs against real globals with confusing results rather than failing fast.

Same issue in mockWhich (line 42) and mockSpawnSync (line 55) and all three restore* functions.

Suggestion — drop all try/catch blocks, verify the assignment took effect:

functionsetSpawn(impl: SpawnImpl){(Bunasunknownas{spawn: SpawnImpl}).spawn=impl;if(Bun.spawn!==(implasunknown)){thrownewError("Failed to mock Bun.spawn — property may be non-writable");}}functionrestoreSpawn(){(Bunasunknownas{spawn: typeofBun.spawn}).spawn=origSpawn;}

Bonus — extract the cast once to reduce boilerplate (repeated 6× as-is):

constbunOverrides=Bunasunknownas{spawn: SpawnImpl;which: (bin: string)=>string|null;spawnSync: (cmd: string[])=>{exitCode: number};};functionsetSpawn(impl: SpawnImpl){bunOverrides.spawn=impl;}functionrestoreSpawn(){bunOverrides.spawn=origSpawnasunknownasSpawnImpl;}// ... etc

try {
(Bun as unknown as { which: (bin: string) => string | null }).which = (bin) =>
present.has(bin) ? `/usr/local/bin/${bin}` : null;
} catch {
// ignore
}
}
function restoreWhich() {
try {
(Bun as unknown as { which: typeof Bun.which }).which = origWhich;
} catch {
// ignore
}
}

function mockSpawnSync(yarnDlxExitCode: number) {
try {
(Bun as unknown as { spawnSync: (cmd: string[]) => { exitCode: number } }).spawnSync = (
cmd,
) => {
if (cmd[0] === "yarn" && cmd[1] === "dlx") return { exitCode: yarnDlxExitCode };
return { exitCode: 0 };
};
} catch {
// ignore
}
}
function restoreSpawnSync() {
try {
(Bun as unknown as { spawnSync: typeof Bun.spawnSync }).spawnSync = origSpawnSync;
} catch {
// ignore
}
}

/** Minimal ProjectContext suitable for driving runFormatters. */
function makeCtx(overrides: Partial<ProjectContext> & { cwd: string }): ProjectContext {
return {
framework: {
name: "next",
sdk: "@clerk/nextjs",
dep: "next",
envFile: ".env.local",
} as ProjectContext["framework"],
typescript: true,
srcDir: false,
packageManager: "bun",
existingClerk: false,
deps: {},
envFile: ".env.local",
...overrides,
};
}

describe("runFormatters", () => {
let tempDir: string;
let spawnCalls: SpawnArgs[];

beforeEach(async () => {
tempDir = await mkdtemp(join(tmpdir(), "clerk-format-"));
spawnCalls = [];
setSpawn((cmd) => {
spawnCalls.push(cmd);
return { exited: Promise.resolve(0) };
});
mockWhich(new Set(["bunx", "npx", "pnpm", "yarn"]));
mockSpawnSync(0);
});

afterEach(async () => {
restoreSpawn();
restoreWhich();
restoreSpawnSync();
await rm(tempDir, { recursive: true, force: true });
});

test("no-op when files is empty", async () => {
const ctx = makeCtx({ cwd: tempDir, deps: { prettier: "3.0.0" } });
await runFormatters(ctx, []);
expect(spawnCalls).toHaveLength(0);
});

test("no-op when no supported formatter is in deps", async () => {
const ctx = makeCtx({ cwd: tempDir, deps: { next: "15.0.0" } });
await runFormatters(ctx, ["a.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("runs prettier via the package-manager's preferred runner (bun → bunx)", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["src/a.ts", "src/b.ts"]);
expect(spawnCalls).toEqual([
["bunx", "prettier", "--ignore-unknown", "--write", "src/a.ts", "src/b.ts"],
]);
});

test("runs biome via pnpm dlx when packageManager is pnpm", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "pnpm",
deps: { "@biomejs/biome": "1.9.0" },
});
await runFormatters(ctx, ["src/a.ts"]);
expect(spawnCalls).toEqual([
["pnpm", "dlx", "@biomejs/biome", "format", "--write", "src/a.ts"],
]);
});

test("runs both prettier and biome when both are in deps, in FORMATTERS order", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "npm",
deps: { prettier: "3.0.0", "@biomejs/biome": "1.9.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([
["npx", "prettier", "--ignore-unknown", "--write", "x.ts"],
["npx", "@biomejs/biome", "format", "--write", "x.ts"],
]);
});

test("falls back to the first available runner when the pm's runner is missing", async () => {
// Project says bun, but only npx/pnpm are on PATH.
mockWhich(new Set(["npx", "pnpm"]));
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([["npx", "prettier", "--ignore-unknown", "--write", "x.ts"]]);
});

test("skips silently when no runners are on PATH", async () => {
mockWhich(new Set());
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("excludes yarn when yarn dlx probe fails (Yarn Classic)", async () => {
mockWhich(new Set(["yarn"]));
mockSpawnSync(1); // yarn dlx --help exits non-zero
const ctx = makeCtx({
cwd: tempDir,
packageManager: "yarn",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("swallows spawn errors (best-effort) and continues to later formatters", async () => {
const attempted: SpawnArgs[] = [];
setSpawn((cmd) => {
attempted.push(cmd);
if (cmd[1] === "prettier") {
throw new Error("spawn failed");
}
return { exited: Promise.resolve(0) };
});
const ctx = makeCtx({
cwd: tempDir,
packageManager: "npm",
deps: { prettier: "3.0.0", "@biomejs/biome": "1.9.0" },
});
// Should not throw even though prettier spawn blows up.
await runFormatters(ctx, ["x.ts"]);
expect(attempted).toEqual([
["npx", "prettier", "--ignore-unknown", "--write", "x.ts"],
["npx", "@biomejs/biome", "format", "--write", "x.ts"],
]);
});

test("reads deps from disk when ctx.deps is empty", async () => {
await Bun.write(
join(tempDir, "package.json"),
JSON.stringify({ dependencies: { prettier: "3.0.0" } }),
);
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: {}, // empty → triggers disk fallback
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([["bunx", "prettier", "--ignore-unknown", "--write", "x.ts"]]);
});

test("no-op when ctx.deps is empty and package.json is missing", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: {},
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("spawns in the project cwd", async () => {
let seenCwd: string | undefined;
setSpawn((cmd, opts?: { cwd?: string }) => {
seenCwd = opts?.cwd;
spawnCalls.push(cmd);
return { exited: Promise.resolve(0) };
});
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(seenCwd).toBe(tempDir);
});
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing test: non-zero formatter exit code

Tests cover spawn throwing (swallowed correctly) and spawn succeeding (exit 0), but there's no test for a formatter exiting non-zero. The current code silently ignores exit codes (await proc.exited with no check), which is the right best-effort behavior — worth locking in with a test:

test("ignores non-zero exit code from formatter (best-effort)",async()=>{setSpawn((cmd)=>{spawnCalls.push(cmd);return{exited: Promise.resolve(1)};});constctx=makeCtx({cwd: tempDir,packageManager: "bun",deps: {prettier: "3.0.0","@biomejs/biome": "1.9.0"},});awaitrunFormatters(ctx,["x.ts"]);// Both formatters attempted despite prettier exiting non-zeroexpect(spawnCalls).toHaveLength(2);});

49 changes: 36 additions & 13 deletions packages/cli-core/src/commands/init/format.ts
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,58 @@
import { readDeps } from "./context.js";
// Pulls in the same runner detection skills.ts uses, so a bun project with
// no `npx` on PATH (entirely possible if the user installed Bun via Homebrew
// but never installed Node) will fall back to bunx instead of silently failing.
import { detectAvailableRunners, preferredRunner, runnerCommand } from "../../lib/runners.js";
Comment on lines +2 to +5

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This reads like commit-message rationale rather than a code comment. The function-level docstring already covers the best-effort / silent-failure contract, and the import names (detectAvailableRunners, preferredRunner) are self-documenting.

Suggestion: delete this comment block entirely — the PR description is the right place for the "why we're switching" context.

import type { ProjectContext } from "./frameworks/types.js";

type FormatterConfig = {
pkg: string;
args: (files: string[]) => string[];
/** Args after the runner: binary + flags + files. The runner is prepended at spawn time. */
binArgs: (files: string[]) => string[];
};

const FORMATTERS: FormatterConfig[] = [
{
pkg: "prettier",
args: (files) => ["npx", "prettier", "--ignore-unknown", "--write", ...files],
binArgs: (files) => ["prettier", "--ignore-unknown", "--write", ...files],
},
{
pkg: "@biomejs/biome",
args: (files) => ["npx", "@biomejs/biome", "format", "--write", ...files],
binArgs: (files) => ["@biomejs/biome", "format", "--write", ...files],
},
];

export async function runFormatters(cwd: string, files: string[]): Promise<void> {
/**
* Format scaffolded files with prettier or biome (whichever the project uses).
*
* Best-effort: failures are silent (stdio ignored, spawn errors swallowed)
* because formatting is purely cosmetic and shouldn't break init.
*/
export async function runFormatters(ctx: ProjectContext, files: string[]): Promise<void> {
if (files.length === 0) return;

const deps = await readDeps(cwd);
const deps = ctx.deps && Object.keys(ctx.deps).length > 0 ? ctx.deps : await readDeps(ctx.cwd);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two things on this line:

1. ctx.deps && is dead codedeps is typed Record<string, string> (not optional), and gatherContext always sets it to deps ?? {}. An empty object is truthy, so the && never short-circuits. Simplify to:

constdeps=Object.keys(ctx.deps).length>0 ? ctx.deps : awaitreadDeps(ctx.cwd);

2. The disk fallback itself is redundant in practicegatherContext() already calls readDeps(cwd) and stores the result. If it returned null, ctx.deps becomes {}. Re-calling readDeps here will return the same null. The only scenario this fallback helps is when someone constructs a ProjectContext manually with deps: {} while a real package.json exists — which only happens in the test for this very code path.

Consider simplifying to just trust ctx.deps:

exportasyncfunctionrunFormatters(ctx: ProjectContext,files: string[]): Promise<void>{if(files.length===0)return;if(Object.keys(ctx.deps).length===0)return;constmatchingFormatters=FORMATTERS.filter((f)=>f.pkginctx.deps);
...

Then update the "reads deps from disk when ctx.deps is empty" test to pass deps: { prettier: "3.0.0" } directly, and drop the "no-op when ctx.deps is empty and package.json is missing" test (already covered by "no-op when no supported formatter is in deps").

if (!deps) return;

for (const formatter of FORMATTERS) {
if (!(formatter.pkg in deps)) continue;
const matchingFormatters = FORMATTERS.filter((f) => f.pkg in deps);
if (matchingFormatters.length === 0) return;

const proc = Bun.spawn(formatter.args(files), {
cwd,
stdout: "ignore",
stderr: "ignore",
});
await proc.exited;
const available = detectAvailableRunners();
if (available.length === 0) return;
const runner = preferredRunner(ctx.packageManager, available);
if (!runner) return;

for (const formatter of matchingFormatters) {
const command = runnerCommand(runner, formatter.binArgs(files));
try {
const proc = Bun.spawn(command, {
cwd: ctx.cwd,
stdout: "ignore",
stderr: "ignore",
});
await proc.exited;
} catch {
// Best-effort, see function doc comment.
}
}
}
2 changes: 1 addition & 1 deletion packages/cli-core/src/commands/init/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -321,7 +321,7 @@ async function scaffoldAndWrite(
}

const writtenFiles = await writePlan(cwd, plan);
await runFormatters(cwd, writtenFiles);
await runFormatters(ctx, writtenFiles);

const findings = await withSpinner("Scanning for issues...", () =>
scanForIssues(cwd, ctx.framework.dep),
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
256 changes: 256 additions & 0 deletions packages/cli-core/src/commands/init/format.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
import { test, expect, describe, beforeEach, afterEach } from "bun:test";
import { join } from "node:path";
import { mkdtemp, rm } from "node:fs/promises";
import { tmpdir } from "node:os";
import { runFormatters } from "./format.ts";
import type { ProjectContext } from "./frameworks/types.js";

type SpawnArgs = readonly string[];

const origSpawn = Bun.spawn;
const origWhich = Bun.which;
const origSpawnSync = Bun.spawnSync;

type SpawnImpl = (
cmd: SpawnArgs,
opts?: { cwd?: string; stdout?: unknown; stderr?: unknown },
) => { exited: Promise<number> };

function setSpawn(impl: SpawnImpl) {
try {
(Bun as unknown as { spawn: SpawnImpl }).spawn = impl;
} catch {
// Bun.spawn may not be writable on some runtimes
}
}
function restoreSpawn() {
try {
(Bun as unknown as { spawn: typeof Bun.spawn }).spawn = origSpawn;
} catch {
// ignore
}
}

function mockWhich(present: ReadonlySet<string>) {
Comment on lines +22 to +34

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Silent mock failures make tests untrustworthy

The try/catch blocks silently swallow mock-assignment failures. If Bun makes these properties non-writable in a future version, every test runs against real globals with confusing results rather than failing fast.

Same issue in mockWhich (line 42) and mockSpawnSync (line 55) and all three restore* functions.

Suggestion — drop all try/catch blocks, verify the assignment took effect:

functionsetSpawn(impl: SpawnImpl){(Bunasunknownas{spawn: SpawnImpl}).spawn=impl;if(Bun.spawn!==(implasunknown)){thrownewError("Failed to mock Bun.spawn — property may be non-writable");}}functionrestoreSpawn(){(Bunasunknownas{spawn: typeofBun.spawn}).spawn=origSpawn;}

Bonus — extract the cast once to reduce boilerplate (repeated 6× as-is):

constbunOverrides=Bunasunknownas{spawn: SpawnImpl;which: (bin: string)=>string|null;spawnSync: (cmd: string[])=>{exitCode: number};};functionsetSpawn(impl: SpawnImpl){bunOverrides.spawn=impl;}functionrestoreSpawn(){bunOverrides.spawn=origSpawnasunknownasSpawnImpl;}// ... etc

try {
(Bun as unknown as { which: (bin: string) => string | null }).which = (bin) =>
present.has(bin) ? `/usr/local/bin/${bin}` : null;
} catch {
// ignore
}
}
function restoreWhich() {
try {
(Bun as unknown as { which: typeof Bun.which }).which = origWhich;
} catch {
// ignore
}
}

function mockSpawnSync(yarnDlxExitCode: number) {
try {
(Bun as unknown as { spawnSync: (cmd: string[]) => { exitCode: number } }).spawnSync = (
cmd,
) => {
if (cmd[0] === "yarn" && cmd[1] === "dlx") return { exitCode: yarnDlxExitCode };
return { exitCode: 0 };
};
} catch {
// ignore
}
}
function restoreSpawnSync() {
try {
(Bun as unknown as { spawnSync: typeof Bun.spawnSync }).spawnSync = origSpawnSync;
} catch {
// ignore
}
}

/** Minimal ProjectContext suitable for driving runFormatters. */
function makeCtx(overrides: Partial<ProjectContext> & { cwd: string }): ProjectContext {
return {
framework: {
name: "next",
sdk: "@clerk/nextjs",
dep: "next",
envFile: ".env.local",
} as ProjectContext["framework"],
typescript: true,
srcDir: false,
packageManager: "bun",
existingClerk: false,
deps: {},
envFile: ".env.local",
...overrides,
};
}

describe("runFormatters", () => {
let tempDir: string;
let spawnCalls: SpawnArgs[];

beforeEach(async () => {
tempDir = await mkdtemp(join(tmpdir(), "clerk-format-"));
spawnCalls = [];
setSpawn((cmd) => {
spawnCalls.push(cmd);
return { exited: Promise.resolve(0) };
});
mockWhich(new Set(["bunx", "npx", "pnpm", "yarn"]));
mockSpawnSync(0);
});

afterEach(async () => {
restoreSpawn();
restoreWhich();
restoreSpawnSync();
await rm(tempDir, { recursive: true, force: true });
});

test("no-op when files is empty", async () => {
const ctx = makeCtx({ cwd: tempDir, deps: { prettier: "3.0.0" } });
await runFormatters(ctx, []);
expect(spawnCalls).toHaveLength(0);
});

test("no-op when no supported formatter is in deps", async () => {
const ctx = makeCtx({ cwd: tempDir, deps: { next: "15.0.0" } });
await runFormatters(ctx, ["a.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("runs prettier via the package-manager's preferred runner (bun → bunx)", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["src/a.ts", "src/b.ts"]);
expect(spawnCalls).toEqual([
["bunx", "prettier", "--ignore-unknown", "--write", "src/a.ts", "src/b.ts"],
]);
});

test("runs biome via pnpm dlx when packageManager is pnpm", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "pnpm",
deps: { "@biomejs/biome": "1.9.0" },
});
await runFormatters(ctx, ["src/a.ts"]);
expect(spawnCalls).toEqual([
["pnpm", "dlx", "@biomejs/biome", "format", "--write", "src/a.ts"],
]);
});

test("runs both prettier and biome when both are in deps, in FORMATTERS order", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "npm",
deps: { prettier: "3.0.0", "@biomejs/biome": "1.9.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([
["npx", "prettier", "--ignore-unknown", "--write", "x.ts"],
["npx", "@biomejs/biome", "format", "--write", "x.ts"],
]);
});

test("falls back to the first available runner when the pm's runner is missing", async () => {
// Project says bun, but only npx/pnpm are on PATH.
mockWhich(new Set(["npx", "pnpm"]));
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([["npx", "prettier", "--ignore-unknown", "--write", "x.ts"]]);
});

test("skips silently when no runners are on PATH", async () => {
mockWhich(new Set());
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("excludes yarn when yarn dlx probe fails (Yarn Classic)", async () => {
mockWhich(new Set(["yarn"]));
mockSpawnSync(1); // yarn dlx --help exits non-zero
const ctx = makeCtx({
cwd: tempDir,
packageManager: "yarn",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("swallows spawn errors (best-effort) and continues to later formatters", async () => {
const attempted: SpawnArgs[] = [];
setSpawn((cmd) => {
attempted.push(cmd);
if (cmd[1] === "prettier") {
throw new Error("spawn failed");
}
return { exited: Promise.resolve(0) };
});
const ctx = makeCtx({
cwd: tempDir,
packageManager: "npm",
deps: { prettier: "3.0.0", "@biomejs/biome": "1.9.0" },
});
// Should not throw even though prettier spawn blows up.
await runFormatters(ctx, ["x.ts"]);
expect(attempted).toEqual([
["npx", "prettier", "--ignore-unknown", "--write", "x.ts"],
["npx", "@biomejs/biome", "format", "--write", "x.ts"],
]);
});

test("reads deps from disk when ctx.deps is empty", async () => {
await Bun.write(
join(tempDir, "package.json"),
JSON.stringify({ dependencies: { prettier: "3.0.0" } }),
);
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: {}, // empty → triggers disk fallback
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([["bunx", "prettier", "--ignore-unknown", "--write", "x.ts"]]);
});

test("no-op when ctx.deps is empty and package.json is missing", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: {},
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("spawns in the project cwd", async () => {
let seenCwd: string | undefined;
setSpawn((cmd, opts?: { cwd?: string }) => {
seenCwd = opts?.cwd;
spawnCalls.push(cmd);
return { exited: Promise.resolve(0) };
});
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(seenCwd).toBe(tempDir);
});
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing test: non-zero formatter exit code

Tests cover spawn throwing (swallowed correctly) and spawn succeeding (exit 0), but there's no test for a formatter exiting non-zero. The current code silently ignores exit codes (await proc.exited with no check), which is the right best-effort behavior — worth locking in with a test:

test("ignores non-zero exit code from formatter (best-effort)",async()=>{setSpawn((cmd)=>{spawnCalls.push(cmd);return{exited: Promise.resolve(1)};});constctx=makeCtx({cwd: tempDir,packageManager: "bun",deps: {prettier: "3.0.0","@biomejs/biome": "1.9.0"},});awaitrunFormatters(ctx,["x.ts"]);// Both formatters attempted despite prettier exiting non-zeroexpect(spawnCalls).toHaveLength(2);});

49 changes: 36 additions & 13 deletions packages/cli-core/src/commands/init/format.ts
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,58 @@
import { readDeps } from "./context.js";
// Pulls in the same runner detection skills.ts uses, so a bun project with
// no `npx` on PATH (entirely possible if the user installed Bun via Homebrew
// but never installed Node) will fall back to bunx instead of silently failing.
import { detectAvailableRunners, preferredRunner, runnerCommand } from "../../lib/runners.js";
Comment on lines +2 to +5

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This reads like commit-message rationale rather than a code comment. The function-level docstring already covers the best-effort / silent-failure contract, and the import names (detectAvailableRunners, preferredRunner) are self-documenting.

Suggestion: delete this comment block entirely — the PR description is the right place for the "why we're switching" context.

import type { ProjectContext } from "./frameworks/types.js";

type FormatterConfig = {
pkg: string;
args: (files: string[]) => string[];
/** Args after the runner: binary + flags + files. The runner is prepended at spawn time. */
binArgs: (files: string[]) => string[];
};

const FORMATTERS: FormatterConfig[] = [
{
pkg: "prettier",
args: (files) => ["npx", "prettier", "--ignore-unknown", "--write", ...files],
binArgs: (files) => ["prettier", "--ignore-unknown", "--write", ...files],
},
{
pkg: "@biomejs/biome",
args: (files) => ["npx", "@biomejs/biome", "format", "--write", ...files],
binArgs: (files) => ["@biomejs/biome", "format", "--write", ...files],
},
];

export async function runFormatters(cwd: string, files: string[]): Promise<void> {
/**
* Format scaffolded files with prettier or biome (whichever the project uses).
*
* Best-effort: failures are silent (stdio ignored, spawn errors swallowed)
* because formatting is purely cosmetic and shouldn't break init.
*/
export async function runFormatters(ctx: ProjectContext, files: string[]): Promise<void> {
if (files.length === 0) return;

const deps = await readDeps(cwd);
const deps = ctx.deps && Object.keys(ctx.deps).length > 0 ? ctx.deps : await readDeps(ctx.cwd);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two things on this line:

1. ctx.deps && is dead codedeps is typed Record<string, string> (not optional), and gatherContext always sets it to deps ?? {}. An empty object is truthy, so the && never short-circuits. Simplify to:

constdeps=Object.keys(ctx.deps).length>0 ? ctx.deps : awaitreadDeps(ctx.cwd);

2. The disk fallback itself is redundant in practicegatherContext() already calls readDeps(cwd) and stores the result. If it returned null, ctx.deps becomes {}. Re-calling readDeps here will return the same null. The only scenario this fallback helps is when someone constructs a ProjectContext manually with deps: {} while a real package.json exists — which only happens in the test for this very code path.

Consider simplifying to just trust ctx.deps:

exportasyncfunctionrunFormatters(ctx: ProjectContext,files: string[]): Promise<void>{if(files.length===0)return;if(Object.keys(ctx.deps).length===0)return;constmatchingFormatters=FORMATTERS.filter((f)=>f.pkginctx.deps);
...

Then update the "reads deps from disk when ctx.deps is empty" test to pass deps: { prettier: "3.0.0" } directly, and drop the "no-op when ctx.deps is empty and package.json is missing" test (already covered by "no-op when no supported formatter is in deps").

if (!deps) return;

for (const formatter of FORMATTERS) {
if (!(formatter.pkg in deps)) continue;
const matchingFormatters = FORMATTERS.filter((f) => f.pkg in deps);
if (matchingFormatters.length === 0) return;

const proc = Bun.spawn(formatter.args(files), {
cwd,
stdout: "ignore",
stderr: "ignore",
});
await proc.exited;
const available = detectAvailableRunners();
if (available.length === 0) return;
const runner = preferredRunner(ctx.packageManager, available);
if (!runner) return;

for (const formatter of matchingFormatters) {
const command = runnerCommand(runner, formatter.binArgs(files));
try {
const proc = Bun.spawn(command, {
cwd: ctx.cwd,
stdout: "ignore",
stderr: "ignore",
});
await proc.exited;
} catch {
// Best-effort, see function doc comment.
}
}
}
2 changes: 1 addition & 1 deletion packages/cli-core/src/commands/init/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -321,7 +321,7 @@ async function scaffoldAndWrite(
}

const writtenFiles = await writePlan(cwd, plan);
await runFormatters(cwd, writtenFiles);
await runFormatters(ctx, writtenFiles);

const findings = await withSpinner("Scanning for issues...", () =>
scanForIssues(cwd, ctx.framework.dep),
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
256 changes: 256 additions & 0 deletions packages/cli-core/src/commands/init/format.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
import { test, expect, describe, beforeEach, afterEach } from "bun:test";
import { join } from "node:path";
import { mkdtemp, rm } from "node:fs/promises";
import { tmpdir } from "node:os";
import { runFormatters } from "./format.ts";
import type { ProjectContext } from "./frameworks/types.js";

type SpawnArgs = readonly string[];

const origSpawn = Bun.spawn;
const origWhich = Bun.which;
const origSpawnSync = Bun.spawnSync;

type SpawnImpl = (
cmd: SpawnArgs,
opts?: { cwd?: string; stdout?: unknown; stderr?: unknown },
) => { exited: Promise<number> };

function setSpawn(impl: SpawnImpl) {
try {
(Bun as unknown as { spawn: SpawnImpl }).spawn = impl;
} catch {
// Bun.spawn may not be writable on some runtimes
}
}
function restoreSpawn() {
try {
(Bun as unknown as { spawn: typeof Bun.spawn }).spawn = origSpawn;
} catch {
// ignore
}
}

function mockWhich(present: ReadonlySet<string>) {
Comment on lines +22 to +34

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Silent mock failures make tests untrustworthy

The try/catch blocks silently swallow mock-assignment failures. If Bun makes these properties non-writable in a future version, every test runs against real globals with confusing results rather than failing fast.

Same issue in mockWhich (line 42) and mockSpawnSync (line 55) and all three restore* functions.

Suggestion — drop all try/catch blocks, verify the assignment took effect:

functionsetSpawn(impl: SpawnImpl){(Bunasunknownas{spawn: SpawnImpl}).spawn=impl;if(Bun.spawn!==(implasunknown)){thrownewError("Failed to mock Bun.spawn — property may be non-writable");}}functionrestoreSpawn(){(Bunasunknownas{spawn: typeofBun.spawn}).spawn=origSpawn;}

Bonus — extract the cast once to reduce boilerplate (repeated 6× as-is):

constbunOverrides=Bunasunknownas{spawn: SpawnImpl;which: (bin: string)=>string|null;spawnSync: (cmd: string[])=>{exitCode: number};};functionsetSpawn(impl: SpawnImpl){bunOverrides.spawn=impl;}functionrestoreSpawn(){bunOverrides.spawn=origSpawnasunknownasSpawnImpl;}// ... etc

try {
(Bun as unknown as { which: (bin: string) => string | null }).which = (bin) =>
present.has(bin) ? `/usr/local/bin/${bin}` : null;
} catch {
// ignore
}
}
function restoreWhich() {
try {
(Bun as unknown as { which: typeof Bun.which }).which = origWhich;
} catch {
// ignore
}
}

function mockSpawnSync(yarnDlxExitCode: number) {
try {
(Bun as unknown as { spawnSync: (cmd: string[]) => { exitCode: number } }).spawnSync = (
cmd,
) => {
if (cmd[0] === "yarn" && cmd[1] === "dlx") return { exitCode: yarnDlxExitCode };
return { exitCode: 0 };
};
} catch {
// ignore
}
}
function restoreSpawnSync() {
try {
(Bun as unknown as { spawnSync: typeof Bun.spawnSync }).spawnSync = origSpawnSync;
} catch {
// ignore
}
}

/** Minimal ProjectContext suitable for driving runFormatters. */
function makeCtx(overrides: Partial<ProjectContext> & { cwd: string }): ProjectContext {
return {
framework: {
name: "next",
sdk: "@clerk/nextjs",
dep: "next",
envFile: ".env.local",
} as ProjectContext["framework"],
typescript: true,
srcDir: false,
packageManager: "bun",
existingClerk: false,
deps: {},
envFile: ".env.local",
...overrides,
};
}

describe("runFormatters", () => {
let tempDir: string;
let spawnCalls: SpawnArgs[];

beforeEach(async () => {
tempDir = await mkdtemp(join(tmpdir(), "clerk-format-"));
spawnCalls = [];
setSpawn((cmd) => {
spawnCalls.push(cmd);
return { exited: Promise.resolve(0) };
});
mockWhich(new Set(["bunx", "npx", "pnpm", "yarn"]));
mockSpawnSync(0);
});

afterEach(async () => {
restoreSpawn();
restoreWhich();
restoreSpawnSync();
await rm(tempDir, { recursive: true, force: true });
});

test("no-op when files is empty", async () => {
const ctx = makeCtx({ cwd: tempDir, deps: { prettier: "3.0.0" } });
await runFormatters(ctx, []);
expect(spawnCalls).toHaveLength(0);
});

test("no-op when no supported formatter is in deps", async () => {
const ctx = makeCtx({ cwd: tempDir, deps: { next: "15.0.0" } });
await runFormatters(ctx, ["a.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("runs prettier via the package-manager's preferred runner (bun → bunx)", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["src/a.ts", "src/b.ts"]);
expect(spawnCalls).toEqual([
["bunx", "prettier", "--ignore-unknown", "--write", "src/a.ts", "src/b.ts"],
]);
});

test("runs biome via pnpm dlx when packageManager is pnpm", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "pnpm",
deps: { "@biomejs/biome": "1.9.0" },
});
await runFormatters(ctx, ["src/a.ts"]);
expect(spawnCalls).toEqual([
["pnpm", "dlx", "@biomejs/biome", "format", "--write", "src/a.ts"],
]);
});

test("runs both prettier and biome when both are in deps, in FORMATTERS order", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "npm",
deps: { prettier: "3.0.0", "@biomejs/biome": "1.9.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([
["npx", "prettier", "--ignore-unknown", "--write", "x.ts"],
["npx", "@biomejs/biome", "format", "--write", "x.ts"],
]);
});

test("falls back to the first available runner when the pm's runner is missing", async () => {
// Project says bun, but only npx/pnpm are on PATH.
mockWhich(new Set(["npx", "pnpm"]));
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([["npx", "prettier", "--ignore-unknown", "--write", "x.ts"]]);
});

test("skips silently when no runners are on PATH", async () => {
mockWhich(new Set());
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("excludes yarn when yarn dlx probe fails (Yarn Classic)", async () => {
mockWhich(new Set(["yarn"]));
mockSpawnSync(1); // yarn dlx --help exits non-zero
const ctx = makeCtx({
cwd: tempDir,
packageManager: "yarn",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("swallows spawn errors (best-effort) and continues to later formatters", async () => {
const attempted: SpawnArgs[] = [];
setSpawn((cmd) => {
attempted.push(cmd);
if (cmd[1] === "prettier") {
throw new Error("spawn failed");
}
return { exited: Promise.resolve(0) };
});
const ctx = makeCtx({
cwd: tempDir,
packageManager: "npm",
deps: { prettier: "3.0.0", "@biomejs/biome": "1.9.0" },
});
// Should not throw even though prettier spawn blows up.
await runFormatters(ctx, ["x.ts"]);
expect(attempted).toEqual([
["npx", "prettier", "--ignore-unknown", "--write", "x.ts"],
["npx", "@biomejs/biome", "format", "--write", "x.ts"],
]);
});

test("reads deps from disk when ctx.deps is empty", async () => {
await Bun.write(
join(tempDir, "package.json"),
JSON.stringify({ dependencies: { prettier: "3.0.0" } }),
);
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: {}, // empty → triggers disk fallback
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([["bunx", "prettier", "--ignore-unknown", "--write", "x.ts"]]);
});

test("no-op when ctx.deps is empty and package.json is missing", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: {},
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("spawns in the project cwd", async () => {
let seenCwd: string | undefined;
setSpawn((cmd, opts?: { cwd?: string }) => {
seenCwd = opts?.cwd;
spawnCalls.push(cmd);
return { exited: Promise.resolve(0) };
});
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(seenCwd).toBe(tempDir);
});
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing test: non-zero formatter exit code

Tests cover spawn throwing (swallowed correctly) and spawn succeeding (exit 0), but there's no test for a formatter exiting non-zero. The current code silently ignores exit codes (await proc.exited with no check), which is the right best-effort behavior — worth locking in with a test:

test("ignores non-zero exit code from formatter (best-effort)",async()=>{setSpawn((cmd)=>{spawnCalls.push(cmd);return{exited: Promise.resolve(1)};});constctx=makeCtx({cwd: tempDir,packageManager: "bun",deps: {prettier: "3.0.0","@biomejs/biome": "1.9.0"},});awaitrunFormatters(ctx,["x.ts"]);// Both formatters attempted despite prettier exiting non-zeroexpect(spawnCalls).toHaveLength(2);});

49 changes: 36 additions & 13 deletions packages/cli-core/src/commands/init/format.ts
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,58 @@
import { readDeps } from "./context.js";
// Pulls in the same runner detection skills.ts uses, so a bun project with
// no `npx` on PATH (entirely possible if the user installed Bun via Homebrew
// but never installed Node) will fall back to bunx instead of silently failing.
import { detectAvailableRunners, preferredRunner, runnerCommand } from "../../lib/runners.js";
Comment on lines +2 to +5

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This reads like commit-message rationale rather than a code comment. The function-level docstring already covers the best-effort / silent-failure contract, and the import names (detectAvailableRunners, preferredRunner) are self-documenting.

Suggestion: delete this comment block entirely — the PR description is the right place for the "why we're switching" context.

import type { ProjectContext } from "./frameworks/types.js";

type FormatterConfig = {
pkg: string;
args: (files: string[]) => string[];
/** Args after the runner: binary + flags + files. The runner is prepended at spawn time. */
binArgs: (files: string[]) => string[];
};

const FORMATTERS: FormatterConfig[] = [
{
pkg: "prettier",
args: (files) => ["npx", "prettier", "--ignore-unknown", "--write", ...files],
binArgs: (files) => ["prettier", "--ignore-unknown", "--write", ...files],
},
{
pkg: "@biomejs/biome",
args: (files) => ["npx", "@biomejs/biome", "format", "--write", ...files],
binArgs: (files) => ["@biomejs/biome", "format", "--write", ...files],
},
];

export async function runFormatters(cwd: string, files: string[]): Promise<void> {
/**
* Format scaffolded files with prettier or biome (whichever the project uses).
*
* Best-effort: failures are silent (stdio ignored, spawn errors swallowed)
* because formatting is purely cosmetic and shouldn't break init.
*/
export async function runFormatters(ctx: ProjectContext, files: string[]): Promise<void> {
if (files.length === 0) return;

const deps = await readDeps(cwd);
const deps = ctx.deps && Object.keys(ctx.deps).length > 0 ? ctx.deps : await readDeps(ctx.cwd);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two things on this line:

1. ctx.deps && is dead codedeps is typed Record<string, string> (not optional), and gatherContext always sets it to deps ?? {}. An empty object is truthy, so the && never short-circuits. Simplify to:

constdeps=Object.keys(ctx.deps).length>0 ? ctx.deps : awaitreadDeps(ctx.cwd);

2. The disk fallback itself is redundant in practicegatherContext() already calls readDeps(cwd) and stores the result. If it returned null, ctx.deps becomes {}. Re-calling readDeps here will return the same null. The only scenario this fallback helps is when someone constructs a ProjectContext manually with deps: {} while a real package.json exists — which only happens in the test for this very code path.

Consider simplifying to just trust ctx.deps:

exportasyncfunctionrunFormatters(ctx: ProjectContext,files: string[]): Promise<void>{if(files.length===0)return;if(Object.keys(ctx.deps).length===0)return;constmatchingFormatters=FORMATTERS.filter((f)=>f.pkginctx.deps);
...

Then update the "reads deps from disk when ctx.deps is empty" test to pass deps: { prettier: "3.0.0" } directly, and drop the "no-op when ctx.deps is empty and package.json is missing" test (already covered by "no-op when no supported formatter is in deps").

if (!deps) return;

for (const formatter of FORMATTERS) {
if (!(formatter.pkg in deps)) continue;
const matchingFormatters = FORMATTERS.filter((f) => f.pkg in deps);
if (matchingFormatters.length === 0) return;

const proc = Bun.spawn(formatter.args(files), {
cwd,
stdout: "ignore",
stderr: "ignore",
});
await proc.exited;
const available = detectAvailableRunners();
if (available.length === 0) return;
const runner = preferredRunner(ctx.packageManager, available);
if (!runner) return;

for (const formatter of matchingFormatters) {
const command = runnerCommand(runner, formatter.binArgs(files));
try {
const proc = Bun.spawn(command, {
cwd: ctx.cwd,
stdout: "ignore",
stderr: "ignore",
});
await proc.exited;
} catch {
// Best-effort, see function doc comment.
}
}
}
2 changes: 1 addition & 1 deletion packages/cli-core/src/commands/init/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -321,7 +321,7 @@ async function scaffoldAndWrite(
}

const writtenFiles = await writePlan(cwd, plan);
await runFormatters(cwd, writtenFiles);
await runFormatters(ctx, writtenFiles);

const findings = await withSpinner("Scanning for issues...", () =>
scanForIssues(cwd, ctx.framework.dep),
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
256 changes: 256 additions & 0 deletions packages/cli-core/src/commands/init/format.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
import { test, expect, describe, beforeEach, afterEach } from "bun:test";
import { join } from "node:path";
import { mkdtemp, rm } from "node:fs/promises";
import { tmpdir } from "node:os";
import { runFormatters } from "./format.ts";
import type { ProjectContext } from "./frameworks/types.js";

type SpawnArgs = readonly string[];

const origSpawn = Bun.spawn;
const origWhich = Bun.which;
const origSpawnSync = Bun.spawnSync;

type SpawnImpl = (
cmd: SpawnArgs,
opts?: { cwd?: string; stdout?: unknown; stderr?: unknown },
) => { exited: Promise<number> };

function setSpawn(impl: SpawnImpl) {
try {
(Bun as unknown as { spawn: SpawnImpl }).spawn = impl;
} catch {
// Bun.spawn may not be writable on some runtimes
}
}
function restoreSpawn() {
try {
(Bun as unknown as { spawn: typeof Bun.spawn }).spawn = origSpawn;
} catch {
// ignore
}
}

function mockWhich(present: ReadonlySet<string>) {
Comment on lines +22 to +34

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Silent mock failures make tests untrustworthy

The try/catch blocks silently swallow mock-assignment failures. If Bun makes these properties non-writable in a future version, every test runs against real globals with confusing results rather than failing fast.

Same issue in mockWhich (line 42) and mockSpawnSync (line 55) and all three restore* functions.

Suggestion — drop all try/catch blocks, verify the assignment took effect:

functionsetSpawn(impl: SpawnImpl){(Bunasunknownas{spawn: SpawnImpl}).spawn=impl;if(Bun.spawn!==(implasunknown)){thrownewError("Failed to mock Bun.spawn — property may be non-writable");}}functionrestoreSpawn(){(Bunasunknownas{spawn: typeofBun.spawn}).spawn=origSpawn;}

Bonus — extract the cast once to reduce boilerplate (repeated 6× as-is):

constbunOverrides=Bunasunknownas{spawn: SpawnImpl;which: (bin: string)=>string|null;spawnSync: (cmd: string[])=>{exitCode: number};};functionsetSpawn(impl: SpawnImpl){bunOverrides.spawn=impl;}functionrestoreSpawn(){bunOverrides.spawn=origSpawnasunknownasSpawnImpl;}// ... etc

try {
(Bun as unknown as { which: (bin: string) => string | null }).which = (bin) =>
present.has(bin) ? `/usr/local/bin/${bin}` : null;
} catch {
// ignore
}
}
function restoreWhich() {
try {
(Bun as unknown as { which: typeof Bun.which }).which = origWhich;
} catch {
// ignore
}
}

function mockSpawnSync(yarnDlxExitCode: number) {
try {
(Bun as unknown as { spawnSync: (cmd: string[]) => { exitCode: number } }).spawnSync = (
cmd,
) => {
if (cmd[0] === "yarn" && cmd[1] === "dlx") return { exitCode: yarnDlxExitCode };
return { exitCode: 0 };
};
} catch {
// ignore
}
}
function restoreSpawnSync() {
try {
(Bun as unknown as { spawnSync: typeof Bun.spawnSync }).spawnSync = origSpawnSync;
} catch {
// ignore
}
}

/** Minimal ProjectContext suitable for driving runFormatters. */
function makeCtx(overrides: Partial<ProjectContext> & { cwd: string }): ProjectContext {
return {
framework: {
name: "next",
sdk: "@clerk/nextjs",
dep: "next",
envFile: ".env.local",
} as ProjectContext["framework"],
typescript: true,
srcDir: false,
packageManager: "bun",
existingClerk: false,
deps: {},
envFile: ".env.local",
...overrides,
};
}

describe("runFormatters", () => {
let tempDir: string;
let spawnCalls: SpawnArgs[];

beforeEach(async () => {
tempDir = await mkdtemp(join(tmpdir(), "clerk-format-"));
spawnCalls = [];
setSpawn((cmd) => {
spawnCalls.push(cmd);
return { exited: Promise.resolve(0) };
});
mockWhich(new Set(["bunx", "npx", "pnpm", "yarn"]));
mockSpawnSync(0);
});

afterEach(async () => {
restoreSpawn();
restoreWhich();
restoreSpawnSync();
await rm(tempDir, { recursive: true, force: true });
});

test("no-op when files is empty", async () => {
const ctx = makeCtx({ cwd: tempDir, deps: { prettier: "3.0.0" } });
await runFormatters(ctx, []);
expect(spawnCalls).toHaveLength(0);
});

test("no-op when no supported formatter is in deps", async () => {
const ctx = makeCtx({ cwd: tempDir, deps: { next: "15.0.0" } });
await runFormatters(ctx, ["a.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("runs prettier via the package-manager's preferred runner (bun → bunx)", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["src/a.ts", "src/b.ts"]);
expect(spawnCalls).toEqual([
["bunx", "prettier", "--ignore-unknown", "--write", "src/a.ts", "src/b.ts"],
]);
});

test("runs biome via pnpm dlx when packageManager is pnpm", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "pnpm",
deps: { "@biomejs/biome": "1.9.0" },
});
await runFormatters(ctx, ["src/a.ts"]);
expect(spawnCalls).toEqual([
["pnpm", "dlx", "@biomejs/biome", "format", "--write", "src/a.ts"],
]);
});

test("runs both prettier and biome when both are in deps, in FORMATTERS order", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "npm",
deps: { prettier: "3.0.0", "@biomejs/biome": "1.9.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([
["npx", "prettier", "--ignore-unknown", "--write", "x.ts"],
["npx", "@biomejs/biome", "format", "--write", "x.ts"],
]);
});

test("falls back to the first available runner when the pm's runner is missing", async () => {
// Project says bun, but only npx/pnpm are on PATH.
mockWhich(new Set(["npx", "pnpm"]));
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([["npx", "prettier", "--ignore-unknown", "--write", "x.ts"]]);
});

test("skips silently when no runners are on PATH", async () => {
mockWhich(new Set());
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("excludes yarn when yarn dlx probe fails (Yarn Classic)", async () => {
mockWhich(new Set(["yarn"]));
mockSpawnSync(1); // yarn dlx --help exits non-zero
const ctx = makeCtx({
cwd: tempDir,
packageManager: "yarn",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("swallows spawn errors (best-effort) and continues to later formatters", async () => {
const attempted: SpawnArgs[] = [];
setSpawn((cmd) => {
attempted.push(cmd);
if (cmd[1] === "prettier") {
throw new Error("spawn failed");
}
return { exited: Promise.resolve(0) };
});
const ctx = makeCtx({
cwd: tempDir,
packageManager: "npm",
deps: { prettier: "3.0.0", "@biomejs/biome": "1.9.0" },
});
// Should not throw even though prettier spawn blows up.
await runFormatters(ctx, ["x.ts"]);
expect(attempted).toEqual([
["npx", "prettier", "--ignore-unknown", "--write", "x.ts"],
["npx", "@biomejs/biome", "format", "--write", "x.ts"],
]);
});

test("reads deps from disk when ctx.deps is empty", async () => {
await Bun.write(
join(tempDir, "package.json"),
JSON.stringify({ dependencies: { prettier: "3.0.0" } }),
);
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: {}, // empty → triggers disk fallback
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([["bunx", "prettier", "--ignore-unknown", "--write", "x.ts"]]);
});

test("no-op when ctx.deps is empty and package.json is missing", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: {},
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("spawns in the project cwd", async () => {
let seenCwd: string | undefined;
setSpawn((cmd, opts?: { cwd?: string }) => {
seenCwd = opts?.cwd;
spawnCalls.push(cmd);
return { exited: Promise.resolve(0) };
});
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(seenCwd).toBe(tempDir);
});
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing test: non-zero formatter exit code

Tests cover spawn throwing (swallowed correctly) and spawn succeeding (exit 0), but there's no test for a formatter exiting non-zero. The current code silently ignores exit codes (await proc.exited with no check), which is the right best-effort behavior — worth locking in with a test:

test("ignores non-zero exit code from formatter (best-effort)",async()=>{setSpawn((cmd)=>{spawnCalls.push(cmd);return{exited: Promise.resolve(1)};});constctx=makeCtx({cwd: tempDir,packageManager: "bun",deps: {prettier: "3.0.0","@biomejs/biome": "1.9.0"},});awaitrunFormatters(ctx,["x.ts"]);// Both formatters attempted despite prettier exiting non-zeroexpect(spawnCalls).toHaveLength(2);});

49 changes: 36 additions & 13 deletions packages/cli-core/src/commands/init/format.ts
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,58 @@
import { readDeps } from "./context.js";
// Pulls in the same runner detection skills.ts uses, so a bun project with
// no `npx` on PATH (entirely possible if the user installed Bun via Homebrew
// but never installed Node) will fall back to bunx instead of silently failing.
import { detectAvailableRunners, preferredRunner, runnerCommand } from "../../lib/runners.js";
Comment on lines +2 to +5

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This reads like commit-message rationale rather than a code comment. The function-level docstring already covers the best-effort / silent-failure contract, and the import names (detectAvailableRunners, preferredRunner) are self-documenting.

Suggestion: delete this comment block entirely — the PR description is the right place for the "why we're switching" context.

import type { ProjectContext } from "./frameworks/types.js";

type FormatterConfig = {
pkg: string;
args: (files: string[]) => string[];
/** Args after the runner: binary + flags + files. The runner is prepended at spawn time. */
binArgs: (files: string[]) => string[];
};

const FORMATTERS: FormatterConfig[] = [
{
pkg: "prettier",
args: (files) => ["npx", "prettier", "--ignore-unknown", "--write", ...files],
binArgs: (files) => ["prettier", "--ignore-unknown", "--write", ...files],
},
{
pkg: "@biomejs/biome",
args: (files) => ["npx", "@biomejs/biome", "format", "--write", ...files],
binArgs: (files) => ["@biomejs/biome", "format", "--write", ...files],
},
];

export async function runFormatters(cwd: string, files: string[]): Promise<void> {
/**
* Format scaffolded files with prettier or biome (whichever the project uses).
*
* Best-effort: failures are silent (stdio ignored, spawn errors swallowed)
* because formatting is purely cosmetic and shouldn't break init.
*/
export async function runFormatters(ctx: ProjectContext, files: string[]): Promise<void> {
if (files.length === 0) return;

const deps = await readDeps(cwd);
const deps = ctx.deps && Object.keys(ctx.deps).length > 0 ? ctx.deps : await readDeps(ctx.cwd);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two things on this line:

1. ctx.deps && is dead codedeps is typed Record<string, string> (not optional), and gatherContext always sets it to deps ?? {}. An empty object is truthy, so the && never short-circuits. Simplify to:

constdeps=Object.keys(ctx.deps).length>0 ? ctx.deps : awaitreadDeps(ctx.cwd);

2. The disk fallback itself is redundant in practicegatherContext() already calls readDeps(cwd) and stores the result. If it returned null, ctx.deps becomes {}. Re-calling readDeps here will return the same null. The only scenario this fallback helps is when someone constructs a ProjectContext manually with deps: {} while a real package.json exists — which only happens in the test for this very code path.

Consider simplifying to just trust ctx.deps:

exportasyncfunctionrunFormatters(ctx: ProjectContext,files: string[]): Promise<void>{if(files.length===0)return;if(Object.keys(ctx.deps).length===0)return;constmatchingFormatters=FORMATTERS.filter((f)=>f.pkginctx.deps);
...

Then update the "reads deps from disk when ctx.deps is empty" test to pass deps: { prettier: "3.0.0" } directly, and drop the "no-op when ctx.deps is empty and package.json is missing" test (already covered by "no-op when no supported formatter is in deps").

if (!deps) return;

for (const formatter of FORMATTERS) {
if (!(formatter.pkg in deps)) continue;
const matchingFormatters = FORMATTERS.filter((f) => f.pkg in deps);
if (matchingFormatters.length === 0) return;

const proc = Bun.spawn(formatter.args(files), {
cwd,
stdout: "ignore",
stderr: "ignore",
});
await proc.exited;
const available = detectAvailableRunners();
if (available.length === 0) return;
const runner = preferredRunner(ctx.packageManager, available);
if (!runner) return;

for (const formatter of matchingFormatters) {
const command = runnerCommand(runner, formatter.binArgs(files));
try {
const proc = Bun.spawn(command, {
cwd: ctx.cwd,
stdout: "ignore",
stderr: "ignore",
});
await proc.exited;
} catch {
// Best-effort, see function doc comment.
}
}
}
2 changes: 1 addition & 1 deletion packages/cli-core/src/commands/init/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -321,7 +321,7 @@ async function scaffoldAndWrite(
}

const writtenFiles = await writePlan(cwd, plan);
await runFormatters(cwd, writtenFiles);
await runFormatters(ctx, writtenFiles);

const findings = await withSpinner("Scanning for issues...", () =>
scanForIssues(cwd, ctx.framework.dep),
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
256 changes: 256 additions & 0 deletions packages/cli-core/src/commands/init/format.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
import { test, expect, describe, beforeEach, afterEach } from "bun:test";
import { join } from "node:path";
import { mkdtemp, rm } from "node:fs/promises";
import { tmpdir } from "node:os";
import { runFormatters } from "./format.ts";
import type { ProjectContext } from "./frameworks/types.js";

type SpawnArgs = readonly string[];

const origSpawn = Bun.spawn;
const origWhich = Bun.which;
const origSpawnSync = Bun.spawnSync;

type SpawnImpl = (
cmd: SpawnArgs,
opts?: { cwd?: string; stdout?: unknown; stderr?: unknown },
) => { exited: Promise<number> };

function setSpawn(impl: SpawnImpl) {
try {
(Bun as unknown as { spawn: SpawnImpl }).spawn = impl;
} catch {
// Bun.spawn may not be writable on some runtimes
}
}
function restoreSpawn() {
try {
(Bun as unknown as { spawn: typeof Bun.spawn }).spawn = origSpawn;
} catch {
// ignore
}
}

function mockWhich(present: ReadonlySet<string>) {
Comment on lines +22 to +34

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Silent mock failures make tests untrustworthy

The try/catch blocks silently swallow mock-assignment failures. If Bun makes these properties non-writable in a future version, every test runs against real globals with confusing results rather than failing fast.

Same issue in mockWhich (line 42) and mockSpawnSync (line 55) and all three restore* functions.

Suggestion — drop all try/catch blocks, verify the assignment took effect:

functionsetSpawn(impl: SpawnImpl){(Bunasunknownas{spawn: SpawnImpl}).spawn=impl;if(Bun.spawn!==(implasunknown)){thrownewError("Failed to mock Bun.spawn — property may be non-writable");}}functionrestoreSpawn(){(Bunasunknownas{spawn: typeofBun.spawn}).spawn=origSpawn;}

Bonus — extract the cast once to reduce boilerplate (repeated 6× as-is):

constbunOverrides=Bunasunknownas{spawn: SpawnImpl;which: (bin: string)=>string|null;spawnSync: (cmd: string[])=>{exitCode: number};};functionsetSpawn(impl: SpawnImpl){bunOverrides.spawn=impl;}functionrestoreSpawn(){bunOverrides.spawn=origSpawnasunknownasSpawnImpl;}// ... etc

try {
(Bun as unknown as { which: (bin: string) => string | null }).which = (bin) =>
present.has(bin) ? `/usr/local/bin/${bin}` : null;
} catch {
// ignore
}
}
function restoreWhich() {
try {
(Bun as unknown as { which: typeof Bun.which }).which = origWhich;
} catch {
// ignore
}
}

function mockSpawnSync(yarnDlxExitCode: number) {
try {
(Bun as unknown as { spawnSync: (cmd: string[]) => { exitCode: number } }).spawnSync = (
cmd,
) => {
if (cmd[0] === "yarn" && cmd[1] === "dlx") return { exitCode: yarnDlxExitCode };
return { exitCode: 0 };
};
} catch {
// ignore
}
}
function restoreSpawnSync() {
try {
(Bun as unknown as { spawnSync: typeof Bun.spawnSync }).spawnSync = origSpawnSync;
} catch {
// ignore
}
}

/** Minimal ProjectContext suitable for driving runFormatters. */
function makeCtx(overrides: Partial<ProjectContext> & { cwd: string }): ProjectContext {
return {
framework: {
name: "next",
sdk: "@clerk/nextjs",
dep: "next",
envFile: ".env.local",
} as ProjectContext["framework"],
typescript: true,
srcDir: false,
packageManager: "bun",
existingClerk: false,
deps: {},
envFile: ".env.local",
...overrides,
};
}

describe("runFormatters", () => {
let tempDir: string;
let spawnCalls: SpawnArgs[];

beforeEach(async () => {
tempDir = await mkdtemp(join(tmpdir(), "clerk-format-"));
spawnCalls = [];
setSpawn((cmd) => {
spawnCalls.push(cmd);
return { exited: Promise.resolve(0) };
});
mockWhich(new Set(["bunx", "npx", "pnpm", "yarn"]));
mockSpawnSync(0);
});

afterEach(async () => {
restoreSpawn();
restoreWhich();
restoreSpawnSync();
await rm(tempDir, { recursive: true, force: true });
});

test("no-op when files is empty", async () => {
const ctx = makeCtx({ cwd: tempDir, deps: { prettier: "3.0.0" } });
await runFormatters(ctx, []);
expect(spawnCalls).toHaveLength(0);
});

test("no-op when no supported formatter is in deps", async () => {
const ctx = makeCtx({ cwd: tempDir, deps: { next: "15.0.0" } });
await runFormatters(ctx, ["a.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("runs prettier via the package-manager's preferred runner (bun → bunx)", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["src/a.ts", "src/b.ts"]);
expect(spawnCalls).toEqual([
["bunx", "prettier", "--ignore-unknown", "--write", "src/a.ts", "src/b.ts"],
]);
});

test("runs biome via pnpm dlx when packageManager is pnpm", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "pnpm",
deps: { "@biomejs/biome": "1.9.0" },
});
await runFormatters(ctx, ["src/a.ts"]);
expect(spawnCalls).toEqual([
["pnpm", "dlx", "@biomejs/biome", "format", "--write", "src/a.ts"],
]);
});

test("runs both prettier and biome when both are in deps, in FORMATTERS order", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "npm",
deps: { prettier: "3.0.0", "@biomejs/biome": "1.9.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([
["npx", "prettier", "--ignore-unknown", "--write", "x.ts"],
["npx", "@biomejs/biome", "format", "--write", "x.ts"],
]);
});

test("falls back to the first available runner when the pm's runner is missing", async () => {
// Project says bun, but only npx/pnpm are on PATH.
mockWhich(new Set(["npx", "pnpm"]));
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([["npx", "prettier", "--ignore-unknown", "--write", "x.ts"]]);
});

test("skips silently when no runners are on PATH", async () => {
mockWhich(new Set());
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("excludes yarn when yarn dlx probe fails (Yarn Classic)", async () => {
mockWhich(new Set(["yarn"]));
mockSpawnSync(1); // yarn dlx --help exits non-zero
const ctx = makeCtx({
cwd: tempDir,
packageManager: "yarn",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("swallows spawn errors (best-effort) and continues to later formatters", async () => {
const attempted: SpawnArgs[] = [];
setSpawn((cmd) => {
attempted.push(cmd);
if (cmd[1] === "prettier") {
throw new Error("spawn failed");
}
return { exited: Promise.resolve(0) };
});
const ctx = makeCtx({
cwd: tempDir,
packageManager: "npm",
deps: { prettier: "3.0.0", "@biomejs/biome": "1.9.0" },
});
// Should not throw even though prettier spawn blows up.
await runFormatters(ctx, ["x.ts"]);
expect(attempted).toEqual([
["npx", "prettier", "--ignore-unknown", "--write", "x.ts"],
["npx", "@biomejs/biome", "format", "--write", "x.ts"],
]);
});

test("reads deps from disk when ctx.deps is empty", async () => {
await Bun.write(
join(tempDir, "package.json"),
JSON.stringify({ dependencies: { prettier: "3.0.0" } }),
);
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: {}, // empty → triggers disk fallback
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([["bunx", "prettier", "--ignore-unknown", "--write", "x.ts"]]);
});

test("no-op when ctx.deps is empty and package.json is missing", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: {},
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("spawns in the project cwd", async () => {
let seenCwd: string | undefined;
setSpawn((cmd, opts?: { cwd?: string }) => {
seenCwd = opts?.cwd;
spawnCalls.push(cmd);
return { exited: Promise.resolve(0) };
});
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(seenCwd).toBe(tempDir);
});
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing test: non-zero formatter exit code

Tests cover spawn throwing (swallowed correctly) and spawn succeeding (exit 0), but there's no test for a formatter exiting non-zero. The current code silently ignores exit codes (await proc.exited with no check), which is the right best-effort behavior — worth locking in with a test:

test("ignores non-zero exit code from formatter (best-effort)",async()=>{setSpawn((cmd)=>{spawnCalls.push(cmd);return{exited: Promise.resolve(1)};});constctx=makeCtx({cwd: tempDir,packageManager: "bun",deps: {prettier: "3.0.0","@biomejs/biome": "1.9.0"},});awaitrunFormatters(ctx,["x.ts"]);// Both formatters attempted despite prettier exiting non-zeroexpect(spawnCalls).toHaveLength(2);});

49 changes: 36 additions & 13 deletions packages/cli-core/src/commands/init/format.ts
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,58 @@
import { readDeps } from "./context.js";
// Pulls in the same runner detection skills.ts uses, so a bun project with
// no `npx` on PATH (entirely possible if the user installed Bun via Homebrew
// but never installed Node) will fall back to bunx instead of silently failing.
import { detectAvailableRunners, preferredRunner, runnerCommand } from "../../lib/runners.js";
Comment on lines +2 to +5

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This reads like commit-message rationale rather than a code comment. The function-level docstring already covers the best-effort / silent-failure contract, and the import names (detectAvailableRunners, preferredRunner) are self-documenting.

Suggestion: delete this comment block entirely — the PR description is the right place for the "why we're switching" context.

import type { ProjectContext } from "./frameworks/types.js";

type FormatterConfig = {
pkg: string;
args: (files: string[]) => string[];
/** Args after the runner: binary + flags + files. The runner is prepended at spawn time. */
binArgs: (files: string[]) => string[];
};

const FORMATTERS: FormatterConfig[] = [
{
pkg: "prettier",
args: (files) => ["npx", "prettier", "--ignore-unknown", "--write", ...files],
binArgs: (files) => ["prettier", "--ignore-unknown", "--write", ...files],
},
{
pkg: "@biomejs/biome",
args: (files) => ["npx", "@biomejs/biome", "format", "--write", ...files],
binArgs: (files) => ["@biomejs/biome", "format", "--write", ...files],
},
];

export async function runFormatters(cwd: string, files: string[]): Promise<void> {
/**
* Format scaffolded files with prettier or biome (whichever the project uses).
*
* Best-effort: failures are silent (stdio ignored, spawn errors swallowed)
* because formatting is purely cosmetic and shouldn't break init.
*/
export async function runFormatters(ctx: ProjectContext, files: string[]): Promise<void> {
if (files.length === 0) return;

const deps = await readDeps(cwd);
const deps = ctx.deps && Object.keys(ctx.deps).length > 0 ? ctx.deps : await readDeps(ctx.cwd);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two things on this line:

1. ctx.deps && is dead codedeps is typed Record<string, string> (not optional), and gatherContext always sets it to deps ?? {}. An empty object is truthy, so the && never short-circuits. Simplify to:

constdeps=Object.keys(ctx.deps).length>0 ? ctx.deps : awaitreadDeps(ctx.cwd);

2. The disk fallback itself is redundant in practicegatherContext() already calls readDeps(cwd) and stores the result. If it returned null, ctx.deps becomes {}. Re-calling readDeps here will return the same null. The only scenario this fallback helps is when someone constructs a ProjectContext manually with deps: {} while a real package.json exists — which only happens in the test for this very code path.

Consider simplifying to just trust ctx.deps:

exportasyncfunctionrunFormatters(ctx: ProjectContext,files: string[]): Promise<void>{if(files.length===0)return;if(Object.keys(ctx.deps).length===0)return;constmatchingFormatters=FORMATTERS.filter((f)=>f.pkginctx.deps);
...

Then update the "reads deps from disk when ctx.deps is empty" test to pass deps: { prettier: "3.0.0" } directly, and drop the "no-op when ctx.deps is empty and package.json is missing" test (already covered by "no-op when no supported formatter is in deps").

if (!deps) return;

for (const formatter of FORMATTERS) {
if (!(formatter.pkg in deps)) continue;
const matchingFormatters = FORMATTERS.filter((f) => f.pkg in deps);
if (matchingFormatters.length === 0) return;

const proc = Bun.spawn(formatter.args(files), {
cwd,
stdout: "ignore",
stderr: "ignore",
});
await proc.exited;
const available = detectAvailableRunners();
if (available.length === 0) return;
const runner = preferredRunner(ctx.packageManager, available);
if (!runner) return;

for (const formatter of matchingFormatters) {
const command = runnerCommand(runner, formatter.binArgs(files));
try {
const proc = Bun.spawn(command, {
cwd: ctx.cwd,
stdout: "ignore",
stderr: "ignore",
});
await proc.exited;
} catch {
// Best-effort, see function doc comment.
}
}
}
2 changes: 1 addition & 1 deletion packages/cli-core/src/commands/init/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -321,7 +321,7 @@ async function scaffoldAndWrite(
}

const writtenFiles = await writePlan(cwd, plan);
await runFormatters(cwd, writtenFiles);
await runFormatters(ctx, writtenFiles);

const findings = await withSpinner("Scanning for issues...", () =>
scanForIssues(cwd, ctx.framework.dep),
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
256 changes: 256 additions & 0 deletions packages/cli-core/src/commands/init/format.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
import { test, expect, describe, beforeEach, afterEach } from "bun:test";
import { join } from "node:path";
import { mkdtemp, rm } from "node:fs/promises";
import { tmpdir } from "node:os";
import { runFormatters } from "./format.ts";
import type { ProjectContext } from "./frameworks/types.js";

type SpawnArgs = readonly string[];

const origSpawn = Bun.spawn;
const origWhich = Bun.which;
const origSpawnSync = Bun.spawnSync;

type SpawnImpl = (
cmd: SpawnArgs,
opts?: { cwd?: string; stdout?: unknown; stderr?: unknown },
) => { exited: Promise<number> };

function setSpawn(impl: SpawnImpl) {
try {
(Bun as unknown as { spawn: SpawnImpl }).spawn = impl;
} catch {
// Bun.spawn may not be writable on some runtimes
}
}
function restoreSpawn() {
try {
(Bun as unknown as { spawn: typeof Bun.spawn }).spawn = origSpawn;
} catch {
// ignore
}
}

function mockWhich(present: ReadonlySet<string>) {
Comment on lines +22 to +34

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Silent mock failures make tests untrustworthy

The try/catch blocks silently swallow mock-assignment failures. If Bun makes these properties non-writable in a future version, every test runs against real globals with confusing results rather than failing fast.

Same issue in mockWhich (line 42) and mockSpawnSync (line 55) and all three restore* functions.

Suggestion — drop all try/catch blocks, verify the assignment took effect:

functionsetSpawn(impl: SpawnImpl){(Bunasunknownas{spawn: SpawnImpl}).spawn=impl;if(Bun.spawn!==(implasunknown)){thrownewError("Failed to mock Bun.spawn — property may be non-writable");}}functionrestoreSpawn(){(Bunasunknownas{spawn: typeofBun.spawn}).spawn=origSpawn;}

Bonus — extract the cast once to reduce boilerplate (repeated 6× as-is):

constbunOverrides=Bunasunknownas{spawn: SpawnImpl;which: (bin: string)=>string|null;spawnSync: (cmd: string[])=>{exitCode: number};};functionsetSpawn(impl: SpawnImpl){bunOverrides.spawn=impl;}functionrestoreSpawn(){bunOverrides.spawn=origSpawnasunknownasSpawnImpl;}// ... etc

try {
(Bun as unknown as { which: (bin: string) => string | null }).which = (bin) =>
present.has(bin) ? `/usr/local/bin/${bin}` : null;
} catch {
// ignore
}
}
function restoreWhich() {
try {
(Bun as unknown as { which: typeof Bun.which }).which = origWhich;
} catch {
// ignore
}
}

function mockSpawnSync(yarnDlxExitCode: number) {
try {
(Bun as unknown as { spawnSync: (cmd: string[]) => { exitCode: number } }).spawnSync = (
cmd,
) => {
if (cmd[0] === "yarn" && cmd[1] === "dlx") return { exitCode: yarnDlxExitCode };
return { exitCode: 0 };
};
} catch {
// ignore
}
}
function restoreSpawnSync() {
try {
(Bun as unknown as { spawnSync: typeof Bun.spawnSync }).spawnSync = origSpawnSync;
} catch {
// ignore
}
}

/** Minimal ProjectContext suitable for driving runFormatters. */
function makeCtx(overrides: Partial<ProjectContext> & { cwd: string }): ProjectContext {
return {
framework: {
name: "next",
sdk: "@clerk/nextjs",
dep: "next",
envFile: ".env.local",
} as ProjectContext["framework"],
typescript: true,
srcDir: false,
packageManager: "bun",
existingClerk: false,
deps: {},
envFile: ".env.local",
...overrides,
};
}

describe("runFormatters", () => {
let tempDir: string;
let spawnCalls: SpawnArgs[];

beforeEach(async () => {
tempDir = await mkdtemp(join(tmpdir(), "clerk-format-"));
spawnCalls = [];
setSpawn((cmd) => {
spawnCalls.push(cmd);
return { exited: Promise.resolve(0) };
});
mockWhich(new Set(["bunx", "npx", "pnpm", "yarn"]));
mockSpawnSync(0);
});

afterEach(async () => {
restoreSpawn();
restoreWhich();
restoreSpawnSync();
await rm(tempDir, { recursive: true, force: true });
});

test("no-op when files is empty", async () => {
const ctx = makeCtx({ cwd: tempDir, deps: { prettier: "3.0.0" } });
await runFormatters(ctx, []);
expect(spawnCalls).toHaveLength(0);
});

test("no-op when no supported formatter is in deps", async () => {
const ctx = makeCtx({ cwd: tempDir, deps: { next: "15.0.0" } });
await runFormatters(ctx, ["a.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("runs prettier via the package-manager's preferred runner (bun → bunx)", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["src/a.ts", "src/b.ts"]);
expect(spawnCalls).toEqual([
["bunx", "prettier", "--ignore-unknown", "--write", "src/a.ts", "src/b.ts"],
]);
});

test("runs biome via pnpm dlx when packageManager is pnpm", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "pnpm",
deps: { "@biomejs/biome": "1.9.0" },
});
await runFormatters(ctx, ["src/a.ts"]);
expect(spawnCalls).toEqual([
["pnpm", "dlx", "@biomejs/biome", "format", "--write", "src/a.ts"],
]);
});

test("runs both prettier and biome when both are in deps, in FORMATTERS order", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "npm",
deps: { prettier: "3.0.0", "@biomejs/biome": "1.9.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([
["npx", "prettier", "--ignore-unknown", "--write", "x.ts"],
["npx", "@biomejs/biome", "format", "--write", "x.ts"],
]);
});

test("falls back to the first available runner when the pm's runner is missing", async () => {
// Project says bun, but only npx/pnpm are on PATH.
mockWhich(new Set(["npx", "pnpm"]));
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([["npx", "prettier", "--ignore-unknown", "--write", "x.ts"]]);
});

test("skips silently when no runners are on PATH", async () => {
mockWhich(new Set());
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("excludes yarn when yarn dlx probe fails (Yarn Classic)", async () => {
mockWhich(new Set(["yarn"]));
mockSpawnSync(1); // yarn dlx --help exits non-zero
const ctx = makeCtx({
cwd: tempDir,
packageManager: "yarn",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("swallows spawn errors (best-effort) and continues to later formatters", async () => {
const attempted: SpawnArgs[] = [];
setSpawn((cmd) => {
attempted.push(cmd);
if (cmd[1] === "prettier") {
throw new Error("spawn failed");
}
return { exited: Promise.resolve(0) };
});
const ctx = makeCtx({
cwd: tempDir,
packageManager: "npm",
deps: { prettier: "3.0.0", "@biomejs/biome": "1.9.0" },
});
// Should not throw even though prettier spawn blows up.
await runFormatters(ctx, ["x.ts"]);
expect(attempted).toEqual([
["npx", "prettier", "--ignore-unknown", "--write", "x.ts"],
["npx", "@biomejs/biome", "format", "--write", "x.ts"],
]);
});

test("reads deps from disk when ctx.deps is empty", async () => {
await Bun.write(
join(tempDir, "package.json"),
JSON.stringify({ dependencies: { prettier: "3.0.0" } }),
);
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: {}, // empty → triggers disk fallback
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toEqual([["bunx", "prettier", "--ignore-unknown", "--write", "x.ts"]]);
});

test("no-op when ctx.deps is empty and package.json is missing", async () => {
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: {},
});
await runFormatters(ctx, ["x.ts"]);
expect(spawnCalls).toHaveLength(0);
});

test("spawns in the project cwd", async () => {
let seenCwd: string | undefined;
setSpawn((cmd, opts?: { cwd?: string }) => {
seenCwd = opts?.cwd;
spawnCalls.push(cmd);
return { exited: Promise.resolve(0) };
});
const ctx = makeCtx({
cwd: tempDir,
packageManager: "bun",
deps: { prettier: "3.0.0" },
});
await runFormatters(ctx, ["x.ts"]);
expect(seenCwd).toBe(tempDir);
});
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing test: non-zero formatter exit code

Tests cover spawn throwing (swallowed correctly) and spawn succeeding (exit 0), but there's no test for a formatter exiting non-zero. The current code silently ignores exit codes (await proc.exited with no check), which is the right best-effort behavior — worth locking in with a test:

test("ignores non-zero exit code from formatter (best-effort)",async()=>{setSpawn((cmd)=>{spawnCalls.push(cmd);return{exited: Promise.resolve(1)};});constctx=makeCtx({cwd: tempDir,packageManager: "bun",deps: {prettier: "3.0.0","@biomejs/biome": "1.9.0"},});awaitrunFormatters(ctx,["x.ts"]);// Both formatters attempted despite prettier exiting non-zeroexpect(spawnCalls).toHaveLength(2);});

49 changes: 36 additions & 13 deletions packages/cli-core/src/commands/init/format.ts
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,58 @@
import { readDeps } from "./context.js";
// Pulls in the same runner detection skills.ts uses, so a bun project with
// no `npx` on PATH (entirely possible if the user installed Bun via Homebrew
// but never installed Node) will fall back to bunx instead of silently failing.
import { detectAvailableRunners, preferredRunner, runnerCommand } from "../../lib/runners.js";
Comment on lines +2 to +5

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This reads like commit-message rationale rather than a code comment. The function-level docstring already covers the best-effort / silent-failure contract, and the import names (detectAvailableRunners, preferredRunner) are self-documenting.

Suggestion: delete this comment block entirely — the PR description is the right place for the "why we're switching" context.

import type { ProjectContext } from "./frameworks/types.js";

type FormatterConfig = {
pkg: string;
args: (files: string[]) => string[];
/** Args after the runner: binary + flags + files. The runner is prepended at spawn time. */
binArgs: (files: string[]) => string[];
};

const FORMATTERS: FormatterConfig[] = [
{
pkg: "prettier",
args: (files) => ["npx", "prettier", "--ignore-unknown", "--write", ...files],
binArgs: (files) => ["prettier", "--ignore-unknown", "--write", ...files],
},
{
pkg: "@biomejs/biome",
args: (files) => ["npx", "@biomejs/biome", "format", "--write", ...files],
binArgs: (files) => ["@biomejs/biome", "format", "--write", ...files],
},
];

export async function runFormatters(cwd: string, files: string[]): Promise<void> {
/**
* Format scaffolded files with prettier or biome (whichever the project uses).
*
* Best-effort: failures are silent (stdio ignored, spawn errors swallowed)
* because formatting is purely cosmetic and shouldn't break init.
*/
export async function runFormatters(ctx: ProjectContext, files: string[]): Promise<void> {
if (files.length === 0) return;

const deps = await readDeps(cwd);
const deps = ctx.deps && Object.keys(ctx.deps).length > 0 ? ctx.deps : await readDeps(ctx.cwd);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two things on this line:

1. ctx.deps && is dead codedeps is typed Record<string, string> (not optional), and gatherContext always sets it to deps ?? {}. An empty object is truthy, so the && never short-circuits. Simplify to:

constdeps=Object.keys(ctx.deps).length>0 ? ctx.deps : awaitreadDeps(ctx.cwd);

2. The disk fallback itself is redundant in practicegatherContext() already calls readDeps(cwd) and stores the result. If it returned null, ctx.deps becomes {}. Re-calling readDeps here will return the same null. The only scenario this fallback helps is when someone constructs a ProjectContext manually with deps: {} while a real package.json exists — which only happens in the test for this very code path.

Consider simplifying to just trust ctx.deps:

exportasyncfunctionrunFormatters(ctx: ProjectContext,files: string[]): Promise<void>{if(files.length===0)return;if(Object.keys(ctx.deps).length===0)return;constmatchingFormatters=FORMATTERS.filter((f)=>f.pkginctx.deps);
...

Then update the "reads deps from disk when ctx.deps is empty" test to pass deps: { prettier: "3.0.0" } directly, and drop the "no-op when ctx.deps is empty and package.json is missing" test (already covered by "no-op when no supported formatter is in deps").

if (!deps) return;

for (const formatter of FORMATTERS) {
if (!(formatter.pkg in deps)) continue;
const matchingFormatters = FORMATTERS.filter((f) => f.pkg in deps);
if (matchingFormatters.length === 0) return;

const proc = Bun.spawn(formatter.args(files), {
cwd,
stdout: "ignore",
stderr: "ignore",
});
await proc.exited;
const available = detectAvailableRunners();
if (available.length === 0) return;
const runner = preferredRunner(ctx.packageManager, available);
if (!runner) return;

for (const formatter of matchingFormatters) {
const command = runnerCommand(runner, formatter.binArgs(files));
try {
const proc = Bun.spawn(command, {
cwd: ctx.cwd,
stdout: "ignore",
stderr: "ignore",
});
await proc.exited;
} catch {
// Best-effort, see function doc comment.
}
}
}
2 changes: 1 addition & 1 deletion packages/cli-core/src/commands/init/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -321,7 +321,7 @@ async function scaffoldAndWrite(
}

const writtenFiles = await writePlan(cwd, plan);
await runFormatters(cwd, writtenFiles);
await runFormatters(ctx, writtenFiles);

const findings = await withSpinner("Scanning for issues...", () =>
scanForIssues(cwd, ctx.framework.dep),
Expand Down