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
11 changes: 11 additions & 0 deletions .changeset/fix-react-hooks-js-plugin-load-fallback.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"@react-doctor/core": patch
"react-doctor": patch
---

Stop a broken `eslint-plugin-react-hooks` install from sinking the whole lint pass, and fix the misleading error it produced (issue #833).

When the optional `react-hooks-js` (React Compiler) plugin can't be imported in the user's environment, oxlint fails the entire config load — which previously dropped every curated react-doctor diagnostic too and left the scan with `skippedChecks: ["lint"]` and zero results. The oxlint error is also multi-line, and the 200-char error preview truncated its plugin path mid-string (often right at `…/node_modules/`), so it read as react-doctor passing an invalid directory rather than a plugin that failed to load.

- **Graceful degradation:** the oxlint runner now detects a `react-hooks-js` plugin-load failure and retries once with that plugin (and its compiler rules) dropped — mirroring the existing adopted-`extends` fallback. The curated react-doctor rules, dead-code, and environment checks all still run; only the React Compiler rules are skipped, surfaced as a clear `lint:partial` note that includes oxlint's real underlying reason.
- **Readable error:** the unparseable-output preview grew from 200 to 600 chars so the full plugin path and the underlying `Error:` line survive instead of being cut at `…/node_modules/`.
10 changes: 9 additions & 1 deletion packages/core/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,15 @@ export const LATEST_KNOWN_PREACT_MAJOR = 20;
// for. Preact X (10) is the modern baseline.
export const EARLIEST_GATED_PREACT_MAJOR = 10;

export const ERROR_PREVIEW_LENGTH_CHARS = 200;
// Max chars of an unparseable oxlint stdout we keep for the error
// message. oxlint prints a multi-line, framed error to stdout when it
// can't load the config (e.g. a JS plugin failed to import) — the first
// useful line is the plugin path, the second the underlying
// `Error: …` reason. 200 chars truncated mid-path (landing on a bare
// `…/node_modules/`), which read as react-doctor passing an invalid
// directory and hid the real cause (issue #833). 600 keeps the path
// AND the reason line for realistic (deep pnpm) paths.
export const ERROR_PREVIEW_LENGTH_CHARS = 600;

// Minimum length for the generic high-entropy token sweep in
// `redactSensitiveText`. Real API keys / tokens run 32+ chars; the
Expand Down
71 changes: 65 additions & 6 deletions packages/core/src/run-oxlint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { buildRuleSeverityControls } from "./build-rule-severity-controls.js";
import { canOxlintExtendConfig } from "./can-oxlint-extend-config.js";
import { collectIgnorePatterns } from "./collect-ignore-patterns.js";
import { detectUserLintConfigPaths } from "./detect-user-lint-config.js";
import { ReactDoctorError } from "./errors.js";
import { neutralizeDisableDirectives } from "./neutralize-disable-directives.js";
import { createOxlintConfig } from "./runners/oxlint/config.js";
import { resolveUserPlugins } from "./runners/oxlint/plugin-resolution.js";
Expand Down Expand Up @@ -95,6 +96,42 @@ const writeOxlintConfig = (
}
};

const REACT_HOOKS_JS_DROP_PREFIX =
"React Compiler rules (react-hooks-js/*) skipped — eslint-plugin-react-hooks failed to load in this environment";

/**
* Detects an oxlint config-load crash caused by the optional
* `react-hooks-js` (eslint-plugin-react-hooks) React Compiler plugin and
* builds the partial-failure note for it; returns `null` when the failure
* was anything else.
*
* oxlint prints a framed error to stdout (not stderr) and exits non-zero
* when a `jsPlugins` entry can't be imported; that non-JSON stdout
* surfaces as `OxlintOutputUnparseable`. Because oxlint fails the WHOLE
* config load on it, leaving the plugin in would drop every curated
* react-doctor diagnostic too — so the caller retries with the plugin
* stripped (issue #833). Both markers sit at the start of oxlint's
* message, so they survive the `preview` slice even for deep pnpm paths.
*/
export const reactHooksJsPluginDropNote = (error: unknown): string | null => {
if (!(error instanceof ReactDoctorError) || error.reason._tag !== "OxlintOutputUnparseable") {
return null;
}
const { preview } = error.reason;
if (
!preview.includes("Failed to load JS plugin") ||
!preview.includes("eslint-plugin-react-hooks")
) {
return null;
}
// Surface oxlint's underlying reason ("Error: Cannot find module …")
// instead of echoing its whole framed dump; omit it if the line didn't
// survive the preview slice.
const underlyingReason = preview.match(/Error:[^\n]*/)?.[0]?.trim();
const reasonSuffix = underlyingReason ? `: ${underlyingReason}` : "";
return `${REACT_HOOKS_JS_DROP_PREFIX}${reasonSuffix}. Other rules ran normally.`;
};

/**
* The oxlint runner. Composed of three pieces in `runners/oxlint/`:
*
Expand Down Expand Up @@ -164,16 +201,20 @@ export const runOxlint = async (options: RunOxlintOptions): Promise<Diagnostic[]
const extendsPaths = detectedConfigPaths.filter(canOxlintExtendConfig);
const userPlugins = resolveUserPlugins(userConfig?.plugins, configSourceDirectory);

const buildConfig = (extendsForThisAttempt: string[]) =>
const buildConfig = (overrides: {
extendsPaths: string[];
disableReactHooksJsPlugin?: boolean;
}) =>
createOxlintConfig({
pluginPath,
project,
customRulesOnly,
extendsPaths: extendsForThisAttempt,
extendsPaths: overrides.extendsPaths,
ignoredTags,
serverAuthFunctionNames,
severityControls,
userPlugins,
disableReactHooksJsPlugin: overrides.disableReactHooksJsPlugin,
});

// HACK: only neutralize disable comments in audit mode. Default
Expand Down Expand Up @@ -242,10 +283,28 @@ export const runOxlint = async (options: RunOxlintOptions): Promise<Diagnostic[]
concurrency: options.concurrency,
});

writeOxlintConfig(configPath, buildConfig(extendsPaths));
writeOxlintConfig(configPath, buildConfig({ extendsPaths }));
try {
return await runBatches();
} catch (error) {
// The optional `react-hooks-js` React Compiler plugin failed to
// `import()` in this environment. oxlint fails the ENTIRE config
// load on it, which would otherwise drop every curated
// react-doctor diagnostic too. Retry once with the plugin stripped
// so the rest of the scan still runs; the React Compiler rules are
// the only casualty, and the user is told why via a partial
// failure (issue #833). Reported only after the retry succeeds, so
// a still-failing scan surfaces the original error untouched.
const reactHooksJsDropNote = reactHooksJsPluginDropNote(error);
if (reactHooksJsDropNote !== null) {
writeOxlintConfig(
configPath,
buildConfig({ extendsPaths, disableReactHooksJsPlugin: true }),
);
const diagnostics = await runBatches();
onPartialFailure?.(reactHooksJsDropNote);
return diagnostics;
}
// HACK: if the user's adopted lint config is the reason oxlint
// crashed (broken JSON, missing plugin, unknown rule), failing
// the entire lint pass would leave the user with a 100/100
Expand All @@ -256,10 +315,10 @@ export const runOxlint = async (options: RunOxlintOptions): Promise<Diagnostic[]
// it as react-doctor itself crashing; the curated-rules scan
// is the graceful path.
if (extendsPaths.length === 0) throw error;
// `buildConfig([])` carries every other option through — most
// importantly `userPlugins`, so custom rules from
// `buildConfig({ extendsPaths: [] })` carries every other option
// through — most importantly `userPlugins`, so custom rules from
// `config.plugins` still run on the retry.
writeOxlintConfig(configPath, buildConfig([]));
writeOxlintConfig(configPath, buildConfig({ extendsPaths: [] }));
return await runBatches();
}
} finally {
Expand Down
13 changes: 12 additions & 1 deletion packages/core/src/runners/oxlint/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@ export interface OxlintConfigOptions {
* set to `"warn"` or `"error"`.
*/
userPlugins?: ReadonlyArray<ResolvedUserPlugin>;
/**
* Skip the optional `react-hooks-js` (eslint-plugin-react-hooks) JS
* plugin and its React Compiler rules. The `runOxlint` fallback sets
* this and retries after the plugin fails to import in the user's
* environment, so the curated react-doctor rules still run instead of
* the whole lint pass failing (issue #833). See `run-oxlint.ts`.
*/
disableReactHooksJsPlugin?: boolean;
}

const resolveSettingsRootDirectory = (rootDirectory: string): string => {
Expand Down Expand Up @@ -93,8 +101,11 @@ export const createOxlintConfig = ({
serverAuthFunctionNames,
severityControls,
userPlugins = [],
disableReactHooksJsPlugin = false,
}: OxlintConfigOptions) => {
const reactHooksJsPlugin = resolveReactHooksJsPlugin(project.hasReactCompiler, customRulesOnly);
const reactHooksJsPlugin = disableReactHooksJsPlugin
? null
: resolveReactHooksJsPlugin(project.hasReactCompiler, customRulesOnly);
const reactCompilerRules = reactHooksJsPlugin
? applyRuleSeverityControls(
filterRulesToAvailable(
Expand Down
33 changes: 33 additions & 0 deletions packages/core/tests/oxlint-config-settings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,37 @@ describe("createOxlintConfig settings", () => {
expect(config.rules).not.toHaveProperty("react-doctor/artifact-secret-leak");
expect(config.rules).not.toHaveProperty("react-doctor/raw-sql-injection-risk");
});

const hasReactHooksJsEntry = (config: ReturnType<typeof createOxlintConfig>): boolean =>
config.jsPlugins.some(
(entry) => typeof entry === "object" && "name" in entry && entry.name === "react-hooks-js",
);

it("registers the react-hooks-js plugin + compiler rules when React Compiler is present", () => {
const config = createOxlintConfig({
pluginPath: "/tmp/plugin.js",
project: buildProject({ hasReactCompiler: true }),
});

expect(hasReactHooksJsEntry(config)).toBe(true);
expect(Object.keys(config.rules).some((ruleKey) => ruleKey.startsWith("react-hooks-js/"))).toBe(
true,
);
});

it("drops the react-hooks-js plugin + compiler rules under disableReactHooksJsPlugin (the load-failure fallback)", () => {
const config = createOxlintConfig({
pluginPath: "/tmp/plugin.js",
project: buildProject({ hasReactCompiler: true }),
disableReactHooksJsPlugin: true,
});

expect(hasReactHooksJsEntry(config)).toBe(false);
expect(Object.keys(config.rules).some((ruleKey) => ruleKey.startsWith("react-hooks-js/"))).toBe(
false,
);
// The curated react-doctor rules still register — only the optional
// React Compiler frontend is dropped.
expect(config.jsPlugins).toContain("/tmp/plugin.js");
});
});
76 changes: 76 additions & 0 deletions packages/core/tests/react-hooks-js-plugin-drop-note.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { describe, expect, it } from "vite-plus/test";
import { OxlintBatchExceeded, OxlintOutputUnparseable, ReactDoctorError } from "../src/errors.js";
import { reactHooksJsPluginDropNote } from "../src/run-oxlint.js";

// oxlint prints this framed block to stdout (not stderr) and exits
// non-zero when a `jsPlugins` entry can't be imported. `parseOxlintOutput`
// can't JSON-parse it, so it surfaces as `OxlintOutputUnparseable` whose
// `preview` is the start of that block. See issue #833.
const buildPreview = (specifier: string, reason: string): string =>
`Failed to parse oxlint configuration file.\n\n x Failed to load JS plugin: ${specifier}\n | ${reason}`;

const reactHooksSpecifier =
"/repo/node_modules/.pnpm/eslint-plugin-react-hooks@7.1.1_eslint@9.39.4_jiti@2.7.0_/node_modules/eslint-plugin-react-hooks/index.js";

const unparseable = (preview: string): ReactDoctorError =>
new ReactDoctorError({ reason: new OxlintOutputUnparseable({ preview }) });

describe("reactHooksJsPluginDropNote", () => {
it("recognizes a react-hooks-js load failure and surfaces the underlying reason", () => {
const error = unparseable(
buildPreview(
reactHooksSpecifier,
"Error: Cannot find module './cjs/eslint-plugin-react-hooks.development.js'",
),
);

const note = reactHooksJsPluginDropNote(error);

expect(note).toContain("React Compiler rules (react-hooks-js/*) skipped");
expect(note).toContain(
"Error: Cannot find module './cjs/eslint-plugin-react-hooks.development.js'",
);
expect(note).toContain("Other rules ran normally.");
});

it("still recognizes the failure when the preview is truncated before the reason line", () => {
// The historical bug: a short `preview` cut the path at `…/node_modules/`,
// which looked like react-doctor passing an invalid directory. The
// markers still survive, so the failure is detected; the reason is just
// omitted from the note.
const truncated =
"Failed to parse oxlint configuration file.\n\n x Failed to load JS plugin: /repo/node_modules/.pnpm/eslint-plugin-react-hooks@7.1.1_/node_modules/";

const note = reactHooksJsPluginDropNote(unparseable(truncated));

expect(note).toBe(
"React Compiler rules (react-hooks-js/*) skipped — eslint-plugin-react-hooks failed to load in this environment. Other rules ran normally.",
);
});

it("returns null for a JS-plugin failure in some OTHER plugin", () => {
const error = unparseable(
buildPreview("/repo/node_modules/eslint-plugin-import/index.js", "Error: boom"),
);

expect(reactHooksJsPluginDropNote(error)).toBeNull();
});

it("returns null when the failure is not a JS-plugin load failure", () => {
const error = unparseable(
"Some other oxlint configuration error mentioning eslint-plugin-react-hooks",
);

expect(reactHooksJsPluginDropNote(error)).toBeNull();
});

it("returns null for non-OxlintOutputUnparseable errors", () => {
const splittable = new ReactDoctorError({
reason: new OxlintBatchExceeded({ kind: "timeout", detail: "5s budget exceeded" }),
});

expect(reactHooksJsPluginDropNote(splittable)).toBeNull();
expect(reactHooksJsPluginDropNote(new Error("plain error"))).toBeNull();
expect(reactHooksJsPluginDropNote(null)).toBeNull();
});
});
Loading