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
2 changes: 2 additions & 0 deletions packages/react-doctor/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { clearProjectCache, discoverProject } from "./utils/discover-project.js"
import { computeJsxIncludePaths } from "./utils/jsx-include-paths.js";
import { clearConfigCache, loadConfig } from "./utils/load-config.js";
import { mergeAndFilterDiagnostics } from "./utils/merge-and-filter-diagnostics.js";
import { parseReactMajor } from "./utils/parse-react-major.js";
import { clearPackageJsonCache } from "./utils/read-package-json.js";
import { createNodeReadFileLinesSync } from "./utils/read-file-lines-node.js";
import { resolveLintIncludePaths } from "./utils/resolve-lint-include-paths.js";
Expand Down Expand Up @@ -130,6 +131,7 @@ export const diagnose = async (
framework: projectInfo.framework,
hasReactCompiler: projectInfo.hasReactCompiler,
hasTanStackQuery: projectInfo.hasTanStackQuery,
reactMajorVersion: parseReactMajor(projectInfo.reactVersion),
includePaths: lintIncludePaths,
customRulesOnly: userConfig?.customRulesOnly ?? false,
respectInlineDisables: effectiveRespectInlineDisables,
Expand Down
8 changes: 6 additions & 2 deletions packages/react-doctor/src/plugin/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -417,8 +417,6 @@ export const TIMER_AND_SCHEDULER_DIRECT_CALLEE_NAMES = new Set([
"queueMicrotask",
]);

export const SUB_HANDLER_DIRECT_CALLEE_NAMES = TIMER_AND_SCHEDULER_DIRECT_CALLEE_NAMES;

// Timer registrations that ALWAYS need a corresponding cleanup call
// (a stricter subset of the scheduler list above — `requestAnimationFrame`
// and friends already invoke once and self-clean, but `setTimeout` /
Expand Down Expand Up @@ -575,6 +573,12 @@ export const EXTERNAL_SYNC_OBSERVER_CONSTRUCTORS = new Set([
// network/analytics side effects. Detection still works via the
// receiver-bound member-call shape (`analytics.track(...)`,
// `api.del(...)`) in `EVENT_TRIGGERED_SIDE_EFFECT_MEMBER_METHODS`.
//
// `post` / `put` / `patch` are KEPT here — the canonical "You Might
// Not Need an Effect" §6 example is `post(jsonToSubmit)` as a bare
// callee, so removing them would silently miss the textbook case.
// The trade-off (FPs on user helpers named `post(...)`) is acceptable
// at this scope.
export const EVENT_TRIGGERED_SIDE_EFFECT_CALLEES = new Set([
...FETCH_CALLEE_NAMES,
// Network shorthand verbs (article uses `post`)
Expand Down
35 changes: 35 additions & 0 deletions packages/react-doctor/src/plugin/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,41 @@ export const walkAst = (node: EsTreeNode, visitor: (child: EsTreeNode) => boolea
}
};

// HACK: variant of `walkAst` that descends through control-flow blocks
// (IfStatement / TryStatement / SwitchCase / loops / labels) but stops
// at any nested function boundary. Used by rules that ask "what runs
// SYNCHRONOUSLY inside this effect's body?" — counts the
// `if (cond) setX(...)` write but ignores the deferred
// `setTimeout(() => setX(...))` one.
//
// Unlike `walkAst`, this one does not support pruning via `false`
// return — descent is always complete except at function boundaries.
export const walkInsideStatementBlocks = (
node: EsTreeNode,
visitor: (child: EsTreeNode) => void,
): void => {
if (!node || typeof node !== "object") return;
if (
node.type === "FunctionDeclaration" ||
node.type === "FunctionExpression" ||
node.type === "ArrowFunctionExpression"
) {
return;
}
visitor(node);
for (const key of Object.keys(node)) {
if (key === "parent") continue;
const child = node[key];
if (Array.isArray(child)) {
for (const item of child) {
if (item && typeof item === "object" && item.type) walkInsideStatementBlocks(item, visitor);
}
} else if (child && typeof child === "object" && child.type) {
walkInsideStatementBlocks(child, visitor);
}
}
};

export const isSetterIdentifier = (name: string): boolean => SETTER_PATTERN.test(name);

export const isSetterCall = (node: EsTreeNode): boolean =>
Expand Down
37 changes: 6 additions & 31 deletions packages/react-doctor/src/plugin/rules/state-and-effects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ import {
NAVIGATION_RECEIVER_NAMES,
REACT_HANDLER_PROP_PATTERN,
RELATED_USE_STATE_THRESHOLD,
SUB_HANDLER_DIRECT_CALLEE_NAMES,
SUBSCRIPTION_METHOD_NAMES,
TIMER_AND_SCHEDULER_DIRECT_CALLEE_NAMES,
TIMER_CALLEE_NAMES_REQUIRING_CLEANUP,
TIMER_CLEANUP_CALLEE_NAMES,
TRIVIAL_DERIVATION_CALLEE_NAMES,
Expand All @@ -41,6 +41,7 @@ import {
isSetterIdentifier,
isUppercaseName,
walkAst,
walkInsideStatementBlocks,
} from "../helpers.js";
import type { EsTreeNode, Rule, RuleContext } from "../types.js";

Expand Down Expand Up @@ -688,32 +689,6 @@ const collectReturnExpressions = (componentBody: EsTreeNode): EsTreeNode[] => {
return returns;
};

const walkInsideStatementBlocks = (
node: EsTreeNode,
visitor: (child: EsTreeNode) => void,
): void => {
if (!node || typeof node !== "object") return;
if (
node.type === "FunctionDeclaration" ||
node.type === "FunctionExpression" ||
node.type === "ArrowFunctionExpression"
) {
return;
}
visitor(node);
for (const key of Object.keys(node)) {
if (key === "parent") continue;
const child = node[key];
if (Array.isArray(child)) {
for (const item of child) {
if (item && typeof item === "object" && item.type) walkInsideStatementBlocks(item, visitor);
}
} else if (child && typeof child === "object" && child.type) {
walkInsideStatementBlocks(child, visitor);
}
}
};

const collectIdentifierNames = (expression: EsTreeNode): Set<string> => {
const names = new Set<string>();
walkAst(expression, (child: EsTreeNode) => {
Expand Down Expand Up @@ -2353,9 +2328,9 @@ export const noMutableInDeps: Rule = {
// - a destructured prop named `on[A-Z]…`, OR
// - a local declared via `const F = useCallback(...)`
// (3) every read of `F` inside the effect body sits inside a sub-
// handler (SUB_HANDLER_DIRECT_CALLEE_NAMES, OR a MemberExpression
// whose property is in SUBSCRIPTION_METHOD_NAMES — same set the
// prefer-use-sync-external-store family uses)
// handler (TIMER_AND_SCHEDULER_DIRECT_CALLEE_NAMES, OR a
// MemberExpression whose property is in SUBSCRIPTION_METHOD_NAMES
// — same set the prefer-use-sync-external-store family uses)
// (4) `F` is NEVER read at the effect's own top level
const collectFunctionTypedLocalBindings = (componentBody: EsTreeNode): Set<string> => {
const functionTypedLocals = new Set<string>();
Expand Down Expand Up @@ -2393,7 +2368,7 @@ const findEnclosingFunctionInsideEffect = (
const isCallExpressionWithSubHandlerCallee = (callExpression: EsTreeNode): boolean => {
if (callExpression?.type !== "CallExpression") return false;
const callee = callExpression.callee;
if (callee?.type === "Identifier" && SUB_HANDLER_DIRECT_CALLEE_NAMES.has(callee.name)) {
if (callee?.type === "Identifier" && TIMER_AND_SCHEDULER_DIRECT_CALLEE_NAMES.has(callee.name)) {
return true;
}
if (
Expand Down
10 changes: 7 additions & 3 deletions packages/react-doctor/src/utils/run-oxlint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -848,9 +848,13 @@ interface RunOxlintOptions {
hasTanStackQuery: boolean;
/**
* Major version of React detected for the project. Forwarded to
* `createOxlintConfig` so React-19-deprecation rules only fire on
* projects where they actually apply. `null` means "unknown — leave
* those rules enabled".
* `createOxlintConfig`, which gates rules directionally:
* - `"deprecation-warning"` rules (e.g. `no-default-props`) stay
* enabled when this is `null` so mid-migration projects still
* get the warning even if version detection failed.
* - `"prefer-newer-api"` rules (e.g. `prefer-use-effect-event`) are
* skipped when this is `null` to avoid recommending APIs that
* may not exist in the consumer's React version.
*/
reactMajorVersion?: number | null;
includePaths?: string[];
Expand Down
77 changes: 77 additions & 0 deletions packages/react-doctor/tests/diagnose.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import fs from "node:fs";
import os from "node:os";
import path from "node:path";
import { afterAll, describe, expect, it } from "vite-plus/test";

import { diagnose } from "../src/index.js";
import { setupReactProject } from "./regressions/_helpers.js";

const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "rd-diagnose-api-"));

afterAll(() => {
fs.rmSync(tempRoot, { recursive: true, force: true });
});

describe("diagnose() programmatic API", () => {
// Regression: pre-fix the programmatic `diagnose()` entry forgot to
// forward `reactMajorVersion` to `runOxlint`. After the directional
// version-gating change, that meant every "prefer-newer-api" rule
// (today: `prefer-use-effect-event`) was silently skipped for all
// programmatic API consumers, even on React 19+ projects. The CLI
// entry (`scan.ts`) was unaffected because it always passed the
// version explicitly.
it("emits prefer-use-effect-event diagnostics on a React 19 project (the prefer-newer-api version-gated rule fires)", async () => {
const projectDir = setupReactProject(tempRoot, "diagnose-prefer-use-effect-event-fires", {
files: {
"src/Debounced.tsx": `import { useEffect, useState } from "react";

export const Debounced = ({ onChange }: { onChange: (value: string) => void }) => {
const [text, setText] = useState("");
useEffect(() => {
const id = setTimeout(() => onChange(text), 300);
return () => clearTimeout(id);
}, [text, onChange]);
return <input value={text} onChange={(event) => setText(event.target.value)} />;
};
`,
},
});

const result = await diagnose(projectDir, { lint: true, deadCode: false });
const preferUseEffectEventHits = result.diagnostics.filter(
(diagnostic) => diagnostic.rule === "prefer-use-effect-event",
);
expect(preferUseEffectEventHits.length).toBeGreaterThanOrEqual(1);
});

it("skips prefer-use-effect-event when the project's React version cannot be resolved (no react dep)", async () => {
// Symmetric guard: when the project has no React dependency the
// function throws before lint runs, so we synthesize a project
// with an unresolvable React version range. Its major can't be
// parsed, so `parseReactMajor` returns null and the prefer-newer-
// api rule should be skipped pessimistically — confirming the
// forward really is honoring the version-gate boundary.
const projectDir = setupReactProject(tempRoot, "diagnose-prefer-use-effect-event-skipped", {
reactVersion: "github:facebook/react",
files: {
"src/Debounced.tsx": `import { useEffect, useState } from "react";

export const Debounced = ({ onChange }: { onChange: (value: string) => void }) => {
const [text, setText] = useState("");
useEffect(() => {
const id = setTimeout(() => onChange(text), 300);
return () => clearTimeout(id);
}, [text, onChange]);
return <input value={text} onChange={(event) => setText(event.target.value)} />;
};
`,
},
});

const result = await diagnose(projectDir, { lint: true, deadCode: false });
const preferUseEffectEventHits = result.diagnostics.filter(
(diagnostic) => diagnostic.rule === "prefer-use-effect-event",
);
expect(preferUseEffectEventHits).toHaveLength(0);
});
});
7 changes: 2 additions & 5 deletions scripts/benchmark-scores.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ interface BenchmarkResult {
errorMessage: string | null;
}

const gh = (args: string): string =>
execSync(`gh ${args}`, { encoding: "utf-8" }).trim();
const gh = (args: string): string => execSync(`gh ${args}`, { encoding: "utf-8" }).trim();

const getLatestRunId = (): number => {
const output = gh(
Expand Down Expand Up @@ -157,9 +156,7 @@ const main = async (): Promise<void> => {
const results = await Promise.all(
artifacts.map(({ id }) => downloadAndParseArtifact(id, tempDirectory)),
);
const validResults = results.filter(
(result): result is BenchmarkResult => result !== null,
);
const validResults = results.filter((result): result is BenchmarkResult => result !== null);

if (validResults.length === 0) {
console.error("No valid benchmark results found");
Expand Down
Loading