Skip to content

Commit 0053faa

Browse files
authored
fix(react-doctor): skip empty patterns before knip runs (#149) (#150)
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
1 parent 40b3f7d commit 0053faa

6 files changed

Lines changed: 171 additions & 4 deletions

File tree

packages/react-doctor/src/commands/browser/playwright.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,7 @@ export const playwright = new Command()
4242
.description(
4343
"Evaluate a Playwright snippet against the active session. The snippet runs as an async function with `page`, `browser`, and `context` in scope. Pass code via --eval or pipe it via stdin. JavaScript only — TypeScript and JSX are not transpiled.",
4444
)
45-
.argument(
46-
"[url]",
47-
"navigate to this URL before evaluating (optional if a page is already open)",
48-
)
45+
.argument("[url]", "navigate to this URL before evaluating (optional if a page is already open)")
4946
.option("-e, --eval <code>", "inline JS code to execute")
5047
.option(
5148
"--wait-until <state>",

packages/react-doctor/src/utils/run-knip.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { findMonorepoRoot } from "./find-monorepo-root.js";
1010
import { hasKnipConfig } from "./has-knip-config.js";
1111
import { isFile } from "./is-file.js";
1212
import { readPackageJson } from "./read-package-json.js";
13+
import { sanitizeKnipConfigPatterns } from "./sanitize-knip-config-patterns.js";
1314

1415
interface KnipIssueDescriptor {
1516
category: string;
@@ -117,6 +118,7 @@ const runKnipWithOptions = async (
117118
);
118119

119120
const parsedConfig = options.parsedConfig as Record<string, unknown>;
121+
sanitizeKnipConfigPatterns(parsedConfig);
120122
const disabledPlugins = new Set<string>();
121123
let lastKnipError: unknown;
122124

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import { isPlainObject } from "./is-plain-object.js";
2+
3+
const isMeaningfulPattern = (value: unknown): boolean =>
4+
typeof value !== "string" || value.trim().length > 0;
5+
6+
const sanitizeStringArray = (values: unknown[]): unknown[] =>
7+
values.filter((entry) => (typeof entry === "string" ? entry.trim().length > 0 : true));
8+
9+
// HACK: knip funnels every pattern through picomatch which throws
10+
// `Expected pattern to be a non-empty string` if any entry is `""`.
11+
// Empty strings can sneak in via tsconfig/package.json fields, knip
12+
// configs, or plugin shorthand resolution (issue #149). Walk the
13+
// parsed config and strip empty/whitespace-only patterns so the bad
14+
// entry doesn't take down the whole dead-code step.
15+
export const sanitizeKnipConfigPatterns = (parsedConfig: Record<string, unknown>): void => {
16+
for (const [key, value] of Object.entries(parsedConfig)) {
17+
if (typeof value === "string") {
18+
if (!isMeaningfulPattern(value)) delete parsedConfig[key];
19+
continue;
20+
}
21+
if (Array.isArray(value)) {
22+
if (value.length === 0) continue;
23+
const sanitized = sanitizeStringArray(value);
24+
if (sanitized.length === value.length) continue;
25+
if (sanitized.length === 0) {
26+
delete parsedConfig[key];
27+
} else {
28+
parsedConfig[key] = sanitized;
29+
}
30+
continue;
31+
}
32+
if (isPlainObject(value)) {
33+
sanitizeKnipConfigPatterns(value);
34+
}
35+
}
36+
};

packages/react-doctor/tests/regressions/scan-resilience.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@
1414
* #89 — `--offline` calculates the score locally (no network round trip)
1515
* #115 — `--staged` snapshots git INDEX content (not working tree) so
1616
* partially-staged hunks behave correctly
17+
* #149 — empty / whitespace-only pattern strings reaching knip cause
18+
* `picomatch` to throw `Expected pattern to be a non-empty
19+
* string`, killing the whole dead-code step. The sanitizer
20+
* strips them before `main()` runs.
1721
* #141 — REACT_COMPILER_RULES must not be enabled in the oxlint config
1822
* unless the `react-hooks-js` plugin (eslint-plugin-react-hooks,
1923
* an optional peer) actually resolved — otherwise oxlint errors
@@ -38,6 +42,7 @@ import { batchIncludePaths } from "../../src/utils/batch-include-paths.js";
3842
import { discoverProject } from "../../src/utils/discover-project.js";
3943
import { extractFailedPluginName } from "../../src/utils/extract-failed-plugin-name.js";
4044
import { getStagedSourceFiles, materializeStagedFiles } from "../../src/utils/get-staged-files.js";
45+
import { sanitizeKnipConfigPatterns } from "../../src/utils/sanitize-knip-config-patterns.js";
4146
import { buildDiagnostic, initGitRepo, writeFile, writeJson } from "./_helpers.js";
4247

4348
const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "rd-scan-resilience-"));
@@ -220,6 +225,31 @@ describe("issue #115: --staged uses git INDEX content, not working tree", () =>
220225
});
221226
});
222227

228+
describe("issue #149: empty pattern strings cannot reach knip's picomatch matchers", () => {
229+
it("strips empty/whitespace-only patterns from arrays, scalars, and nested plugin configs", () => {
230+
const parsedConfig: Record<string, unknown> = {
231+
entry: ["src/index.ts", "", " "],
232+
project: "",
233+
ignore: ["", "node_modules/**"],
234+
vite: { config: ["", "vite.config.ts"], entry: " " },
235+
workspaces: {
236+
"packages/foo": { entry: ["", "src/index.ts"], ignore: "" },
237+
},
238+
};
239+
240+
sanitizeKnipConfigPatterns(parsedConfig);
241+
242+
expect(parsedConfig).toEqual({
243+
entry: ["src/index.ts"],
244+
ignore: ["node_modules/**"],
245+
vite: { config: ["vite.config.ts"] },
246+
workspaces: {
247+
"packages/foo": { entry: ["src/index.ts"] },
248+
},
249+
});
250+
});
251+
});
252+
223253
describe("issue #141: oxlint config must not reference unloaded plugins", () => {
224254
// HACK: the bug only fires when eslint-plugin-react-hooks is missing
225255
// AND React Compiler is detected — so REACT_COMPILER_RULES (under the

packages/react-doctor/tests/run-knip.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,22 @@ describe("runKnip", () => {
220220
expect(mockKnipState.mainCallCount).toBe(1);
221221
});
222222

223+
it("strips empty pattern strings from parsedConfig before calling knip (issue #149)", async () => {
224+
mockKnipState.parsedConfig = {
225+
entry: ["src/index.ts", "", "src/main.ts"],
226+
ignore: "",
227+
vite: { config: ["", "vite.config.ts"] },
228+
};
229+
230+
await runKnip(standaloneRoot);
231+
232+
expect(mockKnipState.parsedConfig).toEqual({
233+
entry: ["src/index.ts", "src/main.ts"],
234+
vite: { config: ["vite.config.ts"] },
235+
});
236+
expect(mockKnipState.mainCallCount).toBe(1);
237+
});
238+
223239
it("rethrows the most recent error after exhausting retries instead of `Unreachable`", async () => {
224240
const sequencedErrors = [
225241
new Error("Error loading /repo/vite.config.ts"),
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
import { describe, expect, it } from "vite-plus/test";
2+
import { sanitizeKnipConfigPatterns } from "../src/utils/sanitize-knip-config-patterns.js";
3+
4+
describe("sanitizeKnipConfigPatterns", () => {
5+
it("removes empty string values at the top level", () => {
6+
const parsedConfig: Record<string, unknown> = {
7+
entry: "",
8+
project: "src/**/*.ts",
9+
};
10+
sanitizeKnipConfigPatterns(parsedConfig);
11+
expect(parsedConfig).toEqual({ project: "src/**/*.ts" });
12+
});
13+
14+
it("removes whitespace-only string values", () => {
15+
const parsedConfig: Record<string, unknown> = {
16+
entry: " ",
17+
ignore: "\n\t",
18+
project: "src/**/*.ts",
19+
};
20+
sanitizeKnipConfigPatterns(parsedConfig);
21+
expect(parsedConfig).toEqual({ project: "src/**/*.ts" });
22+
});
23+
24+
it("filters empty strings out of arrays", () => {
25+
const parsedConfig: Record<string, unknown> = {
26+
entry: ["src/index.ts", "", "src/main.ts"],
27+
};
28+
sanitizeKnipConfigPatterns(parsedConfig);
29+
expect(parsedConfig).toEqual({ entry: ["src/index.ts", "src/main.ts"] });
30+
});
31+
32+
it("removes arrays that become empty after filtering", () => {
33+
const parsedConfig: Record<string, unknown> = {
34+
entry: ["", " "],
35+
project: ["src/**/*.ts"],
36+
};
37+
sanitizeKnipConfigPatterns(parsedConfig);
38+
expect(parsedConfig).toEqual({ project: ["src/**/*.ts"] });
39+
});
40+
41+
it("recurses into nested plugin configs and workspaces", () => {
42+
const parsedConfig: Record<string, unknown> = {
43+
vite: {
44+
config: ["", "vite.config.ts"],
45+
entry: "",
46+
},
47+
workspaces: {
48+
"packages/foo": {
49+
entry: ["", "src/index.ts"],
50+
ignore: "",
51+
},
52+
},
53+
};
54+
sanitizeKnipConfigPatterns(parsedConfig);
55+
expect(parsedConfig).toEqual({
56+
vite: { config: ["vite.config.ts"] },
57+
workspaces: {
58+
"packages/foo": { entry: ["src/index.ts"] },
59+
},
60+
});
61+
});
62+
63+
it("preserves non-string entries inside arrays", () => {
64+
const parsedConfig: Record<string, unknown> = {
65+
ignoreDependencies: [/regex/, "valid", ""],
66+
};
67+
sanitizeKnipConfigPatterns(parsedConfig);
68+
expect(parsedConfig).toEqual({ ignoreDependencies: [/regex/, "valid"] });
69+
});
70+
71+
it("leaves boolean and falsy non-string values untouched", () => {
72+
const parsedConfig: Record<string, unknown> = {
73+
vite: false,
74+
eslint: true,
75+
tags: [],
76+
includeEntryExports: false,
77+
};
78+
sanitizeKnipConfigPatterns(parsedConfig);
79+
expect(parsedConfig).toEqual({
80+
vite: false,
81+
eslint: true,
82+
tags: [],
83+
includeEntryExports: false,
84+
});
85+
});
86+
});

0 commit comments

Comments
 (0)