diff --git a/.changeset/compiler-bailout-reason-in-message.md b/.changeset/compiler-bailout-reason-in-message.md new file mode 100644 index 000000000..e292efdbd --- /dev/null +++ b/.changeset/compiler-bailout-reason-in-message.md @@ -0,0 +1,5 @@ +--- +"react-doctor": patch +--- + +Carry the React Compiler bail-out reason in the primary diagnostic message. `react-hooks-js/*` diagnostics previously all rendered the same generic "This component misses React Compiler's automatic memoization…" message, with the specific reason relegated to `help`. The message now includes the first line of the compiler's reason (e.g. `useMemo() callbacks may not be async or generator functions`) so contexts that only show the message explain _why_ the compiler bailed; the reason's remaining lines stay in `help`, so the rendered message + suggestion never repeat the same sentence. `todo` diagnostics keep the generic message — their reasons are compiler-internal work notes, not user-facing copy. Because diagnostics dedupe on their full message, two _different_ bail-out reasons anchored at the same source location now survive as two diagnostics instead of collapsing into one, so counts can rise slightly on affected projects. diff --git a/packages/core/src/runners/oxlint/parse-output.ts b/packages/core/src/runners/oxlint/parse-output.ts index fef837646..a1285d78c 100644 --- a/packages/core/src/runners/oxlint/parse-output.ts +++ b/packages/core/src/runners/oxlint/parse-output.ts @@ -17,18 +17,30 @@ import { redactSensitiveText } from "../../utils/redact-sensitive-text.js"; import { shouldSuppressLocalUseHookDiagnostic } from "./should-suppress-local-use-hook-diagnostic.js"; const FILEPATH_WITH_LOCATION_PATTERN = /\S+\.\w+:\d+:\d+[\s\S]*$/; +const LEADING_SEVERITY_LABEL_PATTERN = /^(?:Error|Warning):\s*/; +const TRAILING_PERIOD_PATTERN = /\.$/; // Adopted `react-hooks-js` (React Compiler) diagnostics have no // react-doctor `title`, so they'd otherwise render their bare // `react-hooks-js/todo` id. Give them a human headline & an impact-first -// message; the specific bail-out reason stays in `help`. +// message that carries the first line of the compiler's bail-out reason; +// the reason's remaining lines stay in `help`, so renderers that print +// message + help never repeat the same sentence back-to-back. const REACT_COMPILER_TITLE = "React Compiler can't optimize this"; // The compiler's `todo` rule fires on syntax it doesn't handle yet — // an unsupported-syntax bail-out, not an optimization miss in the // user's code, so it gets its own headline. const REACT_COMPILER_TODO_TITLE = "React Compiler doesn't support this syntax"; -const REACT_COMPILER_MESSAGE = - "This component misses React Compiler's automatic memoization & re-renders more than it should. Rewrite the flagged code so the compiler can optimize it."; +const REACT_COMPILER_IMPACT = + "This component misses React Compiler's automatic memoization & re-renders more than it should"; +const REACT_COMPILER_ACTION = "Rewrite the flagged code so the compiler can optimize it."; +const REACT_COMPILER_GENERIC_MESSAGE = `${REACT_COMPILER_IMPACT}. ${REACT_COMPILER_ACTION}`; + +const buildReactCompilerMessage = (reasonSummary: string): string => { + const normalizedSummary = reasonSummary.replace(TRAILING_PERIOD_PATTERN, ""); + if (!normalizedSummary) return REACT_COMPILER_GENERIC_MESSAGE; + return `${REACT_COMPILER_IMPACT}: ${normalizedSummary}. ${REACT_COMPILER_ACTION}`; +}; // Adopted third-party plugins (not in the react-doctor registry) → the // clear user-facing bucket their diagnostics roll up under. Mirrors the @@ -134,10 +146,29 @@ const resolveCleanedDiagnostic = ( project: ProjectInfo, ): CleanedDiagnostic => { if (plugin === "react-hooks-js") { - const rawMessage = message.replace(FILEPATH_WITH_LOCATION_PATTERN, "").trim(); + const bailoutReason = message + .replace(FILEPATH_WITH_LOCATION_PATTERN, "") + .trim() + .replace(LEADING_SEVERITY_LABEL_PATTERN, "") + .trim(); + // `todo` bail-out reasons are compiler-internal work notes (e.g. + // "(BuildHIR::lowerExpression) Handle TaggedTemplateExpression + // expressions") — not user-facing impact copy — so they stay in + // `help` and the message keeps its generic wording. + if (rule === "todo") { + return { + message: REACT_COMPILER_GENERIC_MESSAGE, + help: appendReanimatedSharedValueHint(bailoutReason || help, rule, project), + }; + } + // The reason's first line is its summary; any remaining lines are the + // compiler's elaboration. The summary moves into the primary message + // and only the elaboration stays in `help`. + const [reasonSummary = "", ...reasonDetailLines] = bailoutReason.split("\n"); + const reasonDetail = reasonDetailLines.join("\n").trim(); return { - message: REACT_COMPILER_MESSAGE, - help: appendReanimatedSharedValueHint(rawMessage || help, rule, project), + message: buildReactCompilerMessage(reasonSummary.trim()), + help: appendReanimatedSharedValueHint(reasonDetail || help, rule, project), }; } const cleaned = message.replace(FILEPATH_WITH_LOCATION_PATTERN, "").trim(); diff --git a/packages/core/tests/append-reanimated-shared-value-hint.test.ts b/packages/core/tests/append-reanimated-shared-value-hint.test.ts index 386e2b17c..96352815d 100644 --- a/packages/core/tests/append-reanimated-shared-value-hint.test.ts +++ b/packages/core/tests/append-reanimated-shared-value-hint.test.ts @@ -1,9 +1,11 @@ import { describe, expect, it } from "vite-plus/test"; -import type { ProjectInfo } from "@react-doctor/core"; import { appendReanimatedSharedValueHint } from "../src/utils/append-reanimated-shared-value-hint.js"; import { parseOxlintOutput } from "../src/runners/oxlint/parse-output.js"; - -const ROOT_DIRECTORY = "/home/user/app"; +import { + buildOxlintStdout, + buildProject, + TEST_ROOT_DIRECTORY, +} from "./helpers/oxlint-parse-harness.js"; const REACT_COMPILER_IMMUTABILITY_HELP = "This value cannot be modified\n\nModifying a value returned from a hook is not allowed. Consider moving the modification into the hook where the value is constructed."; @@ -11,57 +13,12 @@ const REACT_COMPILER_IMMUTABILITY_HELP = const REANIMATED_DOCS_ANCHOR = "https://docs.swmansion.com/react-native-reanimated/docs/core/useSharedValue/#react-compiler-support"; -const buildProject = (overrides: Partial = {}): ProjectInfo => ({ - rootDirectory: ROOT_DIRECTORY, - projectName: "app", - reactVersion: "19.2.0", - reactMajorVersion: 19, - tailwindVersion: null, - zodVersion: null, - zodMajorVersion: null, - framework: "expo", - hasTypeScript: true, - hasReactCompiler: true, - hasTanStackQuery: false, - nextjsVersion: null, - nextjsMajorVersion: null, - hasReactNativeWorkspace: true, - expoVersion: "~51.0.0", - shopifyFlashListVersion: null, - shopifyFlashListMajorVersion: null, - hasReanimated: true, - isPreES2023Target: false, - preactVersion: null, - preactMajorVersion: null, - sourceFileCount: 10, - ...overrides, -}); - -const buildOxlintStdout = (code: string, message: string): string => - JSON.stringify({ - diagnostics: [ - { - message, - code, - severity: "error", - causes: [], - url: "", - help: "", - filename: "src/components/SpinningIcon.tsx", - labels: [{ label: "", span: { offset: 0, length: 1, line: 23, column: 5 } }], - related: [], - }, - ], - number_of_files: 1, - number_of_rules: 1, - }); - describe("appendReanimatedSharedValueHint", () => { it("appends the .get()/.set() hint for immutability findings when reanimated is installed", () => { const help = appendReanimatedSharedValueHint( REACT_COMPILER_IMMUTABILITY_HELP, "immutability", - buildProject(), + buildProject({ hasReanimated: true }), ); expect(help).toContain(REACT_COMPILER_IMMUTABILITY_HELP); expect(help).toContain("`.get()` / `.set()`"); @@ -69,7 +26,11 @@ describe("appendReanimatedSharedValueHint", () => { }); it("returns just the hint when the upstream help is empty", () => { - const help = appendReanimatedSharedValueHint("", "immutability", buildProject()); + const help = appendReanimatedSharedValueHint( + "", + "immutability", + buildProject({ hasReanimated: true }), + ); expect(help).toContain("`.get()` / `.set()`"); expect(help.startsWith("\n")).toBe(false); }); @@ -87,7 +48,7 @@ describe("appendReanimatedSharedValueHint", () => { const help = appendReanimatedSharedValueHint( REACT_COMPILER_IMMUTABILITY_HELP, "refs", - buildProject(), + buildProject({ hasReanimated: true }), ); expect(help).toBe(REACT_COMPILER_IMMUTABILITY_HELP); }); @@ -99,11 +60,17 @@ describe("parseOxlintOutput react-hooks-js immutability messaging", () => { "react-hooks-js(immutability)", REACT_COMPILER_IMMUTABILITY_HELP, ); - const [diagnostic] = parseOxlintOutput(stdout, buildProject(), ROOT_DIRECTORY); + const [diagnostic] = parseOxlintOutput( + stdout, + buildProject({ hasReanimated: true }), + TEST_ROOT_DIRECTORY, + ); expect(diagnostic.title).toBe("React Compiler can't optimize this"); expect(diagnostic.message).toContain("misses React Compiler's automatic memoization"); + expect(diagnostic.message).toContain("This value cannot be modified"); expect(diagnostic.category).toBe("Performance"); + expect(diagnostic.help).toContain("Modifying a value returned from a hook is not allowed"); expect(diagnostic.help).toContain("`.get()` / `.set()`"); expect(diagnostic.help).toContain(REANIMATED_DOCS_ANCHOR); }); @@ -116,7 +83,7 @@ describe("parseOxlintOutput react-hooks-js immutability messaging", () => { const [diagnostic] = parseOxlintOutput( stdout, buildProject({ hasReanimated: false }), - ROOT_DIRECTORY, + TEST_ROOT_DIRECTORY, ); expect(diagnostic.help).not.toContain("`.get()` / `.set()`"); @@ -124,7 +91,11 @@ describe("parseOxlintOutput react-hooks-js immutability messaging", () => { it("does not surface the hint for other React Compiler rules", () => { const stdout = buildOxlintStdout("react-hooks-js(refs)", "Cannot access ref during render"); - const [diagnostic] = parseOxlintOutput(stdout, buildProject(), ROOT_DIRECTORY); + const [diagnostic] = parseOxlintOutput( + stdout, + buildProject({ hasReanimated: true }), + TEST_ROOT_DIRECTORY, + ); expect(diagnostic.help).not.toContain("`.get()` / `.set()`"); }); diff --git a/packages/core/tests/helpers/oxlint-parse-harness.ts b/packages/core/tests/helpers/oxlint-parse-harness.ts new file mode 100644 index 000000000..688ba2510 --- /dev/null +++ b/packages/core/tests/helpers/oxlint-parse-harness.ts @@ -0,0 +1,48 @@ +import type { ProjectInfo } from "@react-doctor/core"; + +export const TEST_ROOT_DIRECTORY = "/home/user/app"; + +export const buildProject = (overrides: Partial = {}): ProjectInfo => ({ + rootDirectory: TEST_ROOT_DIRECTORY, + projectName: "app", + reactVersion: "19.2.0", + reactMajorVersion: 19, + tailwindVersion: null, + zodVersion: null, + zodMajorVersion: null, + framework: "nextjs", + hasTypeScript: true, + hasReactCompiler: true, + hasTanStackQuery: false, + nextjsVersion: "15.0.0", + nextjsMajorVersion: 15, + hasReactNativeWorkspace: false, + expoVersion: null, + shopifyFlashListVersion: null, + shopifyFlashListMajorVersion: null, + hasReanimated: false, + isPreES2023Target: false, + preactVersion: null, + preactMajorVersion: null, + sourceFileCount: 10, + ...overrides, +}); + +export const buildOxlintStdout = (code: string, message: string): string => + JSON.stringify({ + diagnostics: [ + { + message, + code, + severity: "error", + causes: [], + url: "", + help: "", + filename: "src/components/widget.tsx", + labels: [{ label: "", span: { offset: 0, length: 1, line: 12, column: 3 } }], + related: [], + }, + ], + number_of_files: 1, + number_of_rules: 1, + }); diff --git a/packages/core/tests/react-compiler-bailout-message.test.ts b/packages/core/tests/react-compiler-bailout-message.test.ts new file mode 100644 index 000000000..af8681d9a --- /dev/null +++ b/packages/core/tests/react-compiler-bailout-message.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, it } from "vite-plus/test"; +import { parseOxlintOutput } from "../src/runners/oxlint/parse-output.js"; +import { + buildOxlintStdout, + buildProject, + TEST_ROOT_DIRECTORY, +} from "./helpers/oxlint-parse-harness.js"; + +const ASYNC_USE_MEMO_REASON = "useMemo() callbacks may not be async or generator functions"; +const ASYNC_USE_MEMO_DETAIL = + "useMemo() callbacks are called once and must synchronously return a value"; +const GENERIC_COMPILER_MESSAGE = + "This component misses React Compiler's automatic memoization & re-renders more than it should. Rewrite the flagged code so the compiler can optimize it."; + +describe("parseOxlintOutput react-hooks-js bail-out reason in primary message", () => { + it("weaves the reason summary into the message and leaves the elaboration in help", () => { + const stdout = buildOxlintStdout( + "react-hooks-js(use-memo)", + `${ASYNC_USE_MEMO_REASON}\n\n${ASYNC_USE_MEMO_DETAIL}`, + ); + const [diagnostic] = parseOxlintOutput(stdout, buildProject(), TEST_ROOT_DIRECTORY); + + expect(diagnostic.message).toContain("misses React Compiler's automatic memoization"); + expect(diagnostic.message).toContain(ASYNC_USE_MEMO_REASON); + expect(diagnostic.help).toBe(ASYNC_USE_MEMO_DETAIL); + }); + + it("does not repeat a single-line reason in help", () => { + const stdout = buildOxlintStdout("react-hooks-js(use-memo)", ASYNC_USE_MEMO_REASON); + const [diagnostic] = parseOxlintOutput(stdout, buildProject(), TEST_ROOT_DIRECTORY); + + expect(diagnostic.message).toContain(ASYNC_USE_MEMO_REASON); + expect(diagnostic.help).not.toContain(ASYNC_USE_MEMO_REASON); + }); + + it("falls back to the generic message when the compiler emits no reason", () => { + const stdout = buildOxlintStdout("react-hooks-js(use-memo)", ""); + const [diagnostic] = parseOxlintOutput(stdout, buildProject(), TEST_ROOT_DIRECTORY); + + expect(diagnostic.message).toBe(GENERIC_COMPILER_MESSAGE); + }); + + it("keeps compiler-internal `todo` reasons out of the message", () => { + const stdout = buildOxlintStdout( + "react-hooks-js(todo)", + "(BuildHIR::lowerExpression) Handle TaggedTemplateExpression expressions", + ); + const [diagnostic] = parseOxlintOutput(stdout, buildProject(), TEST_ROOT_DIRECTORY); + + expect(diagnostic.message).toBe(GENERIC_COMPILER_MESSAGE); + expect(diagnostic.help).toContain("BuildHIR::lowerExpression"); + }); + + it("strips oxlint's leading Error: label from the reason", () => { + const stdout = buildOxlintStdout("react-hooks-js(use-memo)", `Error: ${ASYNC_USE_MEMO_REASON}`); + const [diagnostic] = parseOxlintOutput(stdout, buildProject(), TEST_ROOT_DIRECTORY); + + expect(diagnostic.message).toContain(`: ${ASYNC_USE_MEMO_REASON}. Rewrite`); + expect(diagnostic.message).not.toContain("Error:"); + expect(diagnostic.help).not.toContain("Error:"); + }); + + it("strips oxlint's leading Warning: label, even behind whitespace", () => { + const stdout = buildOxlintStdout("react-hooks-js(purity)", " Warning: This value is impure"); + const [diagnostic] = parseOxlintOutput(stdout, buildProject(), TEST_ROOT_DIRECTORY); + + expect(diagnostic.message).toContain(": This value is impure. Rewrite"); + expect(diagnostic.message).not.toContain("Warning:"); + }); + + it("does not duplicate the trailing period of a reason summary", () => { + const stdout = buildOxlintStdout("react-hooks-js(purity)", "This value is impure."); + const [diagnostic] = parseOxlintOutput(stdout, buildProject(), TEST_ROOT_DIRECTORY); + + expect(diagnostic.message).toContain(": This value is impure. Rewrite"); + }); +}); diff --git a/packages/core/tests/react-compiler-diagnostic-title.test.ts b/packages/core/tests/react-compiler-diagnostic-title.test.ts index db3adf63d..e68c74e86 100644 --- a/packages/core/tests/react-compiler-diagnostic-title.test.ts +++ b/packages/core/tests/react-compiler-diagnostic-title.test.ts @@ -1,52 +1,10 @@ import { describe, expect, it } from "vite-plus/test"; -import type { ProjectInfo } from "@react-doctor/core"; import { parseOxlintOutput } from "../src/runners/oxlint/parse-output.js"; - -const ROOT_DIRECTORY = "/home/user/app"; - -const buildProject = (): ProjectInfo => ({ - rootDirectory: ROOT_DIRECTORY, - projectName: "app", - reactVersion: "19.2.0", - reactMajorVersion: 19, - tailwindVersion: null, - zodVersion: null, - zodMajorVersion: null, - framework: "nextjs", - hasTypeScript: true, - hasReactCompiler: true, - hasTanStackQuery: false, - nextjsVersion: "15.0.0", - nextjsMajorVersion: 15, - hasReactNativeWorkspace: false, - expoVersion: null, - shopifyFlashListVersion: null, - shopifyFlashListMajorVersion: null, - hasReanimated: false, - isPreES2023Target: false, - preactVersion: null, - preactMajorVersion: null, - sourceFileCount: 10, -}); - -const buildOxlintStdout = (code: string, message: string): string => - JSON.stringify({ - diagnostics: [ - { - message, - code, - severity: "error", - causes: [], - url: "", - help: "", - filename: "src/components/widget.tsx", - labels: [{ label: "", span: { offset: 0, length: 1, line: 12, column: 3 } }], - related: [], - }, - ], - number_of_files: 1, - number_of_rules: 1, - }); +import { + buildOxlintStdout, + buildProject, + TEST_ROOT_DIRECTORY, +} from "./helpers/oxlint-parse-harness.js"; describe("parseOxlintOutput react-hooks-js diagnostic titles", () => { it("titles `todo` diagnostics as unsupported syntax", () => { @@ -54,7 +12,7 @@ describe("parseOxlintOutput react-hooks-js diagnostic titles", () => { "react-hooks-js(todo)", "(BuildHIR::lowerExpression) Handle TaggedTemplateExpression expressions", ); - const [diagnostic] = parseOxlintOutput(stdout, buildProject(), ROOT_DIRECTORY); + const [diagnostic] = parseOxlintOutput(stdout, buildProject(), TEST_ROOT_DIRECTORY); expect(diagnostic).toMatchInlineSnapshot(` { @@ -77,7 +35,7 @@ describe("parseOxlintOutput react-hooks-js diagnostic titles", () => { it("keeps the generic headline for other react-hooks-js rules", () => { const stdout = buildOxlintStdout("react-hooks-js(refs)", "Cannot access ref during render"); - const [diagnostic] = parseOxlintOutput(stdout, buildProject(), ROOT_DIRECTORY); + const [diagnostic] = parseOxlintOutput(stdout, buildProject(), TEST_ROOT_DIRECTORY); expect(diagnostic.title).toBe("React Compiler can't optimize this"); }); diff --git a/packages/react-doctor/src/cli/utils/render-diagnostics.ts b/packages/react-doctor/src/cli/utils/render-diagnostics.ts index adf852977..896043b20 100644 --- a/packages/react-doctor/src/cli/utils/render-diagnostics.ts +++ b/packages/react-doctor/src/cli/utils/render-diagnostics.ts @@ -657,6 +657,10 @@ export const formatElapsedTime = (elapsedMilliseconds: number): string => { // disk alongside the machine-readable `diagnostics.json`. export const formatRuleSummary = (ruleKey: string, ruleDiagnostics: Diagnostic[]): string => { const firstDiagnostic = ruleDiagnostics[0]; + // Most rules emit one fixed message, but per-site messages (React + // Compiler bail-out reasons) vary — list every distinct one so the + // first site's reason isn't presented as if it described all N sites. + const distinctMessages = [...new Set(ruleDiagnostics.map((diagnostic) => diagnostic.message))]; const sections = [ `Rule: ${ruleKey}`, @@ -664,7 +668,7 @@ export const formatRuleSummary = (ruleKey: string, ruleDiagnostics: Diagnostic[] `Category: ${firstDiagnostic.category}`, `Count: ${ruleDiagnostics.length}`, "", - firstDiagnostic.message, + distinctMessages.join("\n\n"), ]; if (firstDiagnostic.help) { diff --git a/packages/react-doctor/tests/format-rule-summary.test.ts b/packages/react-doctor/tests/format-rule-summary.test.ts new file mode 100644 index 000000000..5365b1f5f --- /dev/null +++ b/packages/react-doctor/tests/format-rule-summary.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, it } from "vite-plus/test"; +import type { Diagnostic } from "@react-doctor/core"; +import { formatRuleSummary } from "../src/cli/utils/render-diagnostics.js"; + +const makeDiagnostic = (overrides: Partial = {}): Diagnostic => ({ + filePath: "src/App.tsx", + plugin: "react-hooks-js", + rule: "use-memo", + severity: "error", + title: "React Compiler can't optimize this", + message: + "This component misses React Compiler's automatic memoization & re-renders more than it should: useMemo() callbacks may not be async or generator functions. Rewrite the flagged code so the compiler can optimize it.", + help: "", + line: 3, + column: 1, + category: "Performance", + ...overrides, +}); + +describe("formatRuleSummary", () => { + it("prints the shared message once for a fixed-message rule", () => { + const summary = formatRuleSummary("react-hooks-js/use-memo", [ + makeDiagnostic(), + makeDiagnostic({ filePath: "src/Other.tsx", line: 9 }), + ]); + + expect(summary.match(/misses React Compiler's automatic memoization/g)).toHaveLength(1); + expect(summary).toContain("Count: 2"); + }); + + it("lists every distinct per-site message instead of presenting the first as rule-wide", () => { + const impureMessage = + "This component misses React Compiler's automatic memoization & re-renders more than it should: This value is impure. Rewrite the flagged code so the compiler can optimize it."; + const summary = formatRuleSummary("react-hooks-js/use-memo", [ + makeDiagnostic(), + makeDiagnostic({ filePath: "src/Other.tsx", line: 9, message: impureMessage }), + ]); + + expect(summary).toContain("useMemo() callbacks may not be async or generator functions"); + expect(summary).toContain("This value is impure"); + }); +});