Skip to content

Commit dacde53

Browse files
cursoragentaidenybai
cursoragent
and
aidenybai
committed
refactor(oxlint-plugin): extract exhaustive-deps message + settings modules
The exhaustive-deps rule file (1363 lines) opened with 17 single-line buildXMessage builder functions and a separate resolveSettings block before any analysis logic. Split out: - exhaustive-deps-messages.ts: 17 user-facing diagnostic strings - exhaustive-deps-settings.ts: ExhaustiveDepsSettings interface + resolveExhaustiveDepsSettings (renamed from resolveSettings for namespace clarity) The main rule file drops from 1363 -> 1307 lines and now opens directly on analysis logic instead of message bookkeeping. Symbol- stability helpers (symbolHasStableHookOrigin / symbolHasStableValue / symbolHasStableFunctionOrigin) stay co-located with the rule for now because they depend on later-declared helpers (unwrapExpression, isOutsideAllFunctions); extracting them is a larger surgery. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
1 parent df89dcd commit dacde53

3 files changed

Lines changed: 123 additions & 77 deletions

File tree

‎packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/exhaustive-deps-messages.ts‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
// User-facing diagnostic strings emitted by the `exhaustive-deps` rule.
2+
// Kept beside the rule (same bucket directory) so authors editing
3+
// wording don't need to scroll past 900 lines of analysis logic;
4+
// otherwise behavior-neutral.
5+
6+
export const buildMissingDepMessage = (hookName: string, depName: string): string =>
7+
`React Hook \`${hookName}\` is missing dependency \`${depName}\` — list it in the dependency array, or call the hook unconditionally.`;
8+
9+
export const buildUnnecessaryDepMessage = (hookName: string, depName: string): string =>
10+
`React Hook \`${hookName}\` has an unnecessary dependency \`${depName}\` — it isn't referenced inside the callback.`;
11+
12+
export const buildDuplicateDepMessage = (hookName: string, depName: string): string =>
13+
`React Hook \`${hookName}\` has duplicate dependency \`${depName}\`.`;
14+
15+
export const buildLiteralDepMessage = (hookName: string): string =>
16+
`React Hook \`${hookName}\` was passed a literal as a dependency. Literals never change so they cannot trigger an update — remove them from the dependency array.`;
17+
18+
export const buildRefCurrentDepMessage = (hookName: string, depName: string): string =>
19+
`React Hook \`${hookName}\` shouldn't include \`${depName}\` in the dependency array — mutable values like \`.current\` aren't valid deps; depend on \`${depName.replace(/\.current$/, "")}\` itself instead.`;
20+
21+
export const buildNonArrayDepsMessage = (hookName: string): string =>
22+
`React Hook \`${hookName}\` has a second argument which is not an array literal. This means oxlint cannot statically verify whether the dependencies are exhaustive — replace the variable with an inline array.`;
23+
24+
export const buildMissingDepArrayMessage = (hookName: string): string =>
25+
`React Hook \`${hookName}\` does nothing when called with only one argument — pass a dependency array as the second argument.`;
26+
27+
export const buildMissingCallbackMessage = (hookName: string): string =>
28+
`React Hook \`${hookName}\` requires an effect callback — pass a function as the first argument.`;
29+
30+
export const buildEffectEventDepMessage = (depName: string): string =>
31+
`Functions returned from \`useEffectEvent\` must not be included in the dependency array. Remove \`${depName}\` from the list.`;
32+
33+
export const buildSpreadDepMessage = (hookName: string): string =>
34+
`React Hook \`${hookName}\` has a spread element in its dependency array. This means oxlint cannot statically verify whether the dependencies are exhaustive.`;
35+
36+
export const buildComplexDepMessage = (hookName: string): string =>
37+
`React Hook \`${hookName}\` has a complex expression in the dependency array. Extract it to a separate variable so it can be statically checked.`;
38+
39+
export const buildAsyncEffectMessage = (hookName: string): string =>
40+
`React Hook \`${hookName}\` received an async callback. Put the async function inside the effect instead.`;
41+
42+
export const buildUnknownCallbackMessage = (hookName: string): string =>
43+
`React Hook \`${hookName}\` received a function whose dependencies are unknown. Pass an inline function instead.`;
44+
45+
export const buildUnstableDepMessage = (hookName: string, depName: string): string =>
46+
`The \`${depName}\` value makes the dependencies of \`${hookName}\` change on every render. Move it inside the hook callback or wrap it in its own memoization hook.`;
47+
48+
export const buildSetStateWithoutDepsMessage = (hookName: string, setterName: string): string =>
49+
`React Hook \`${hookName}\` contains a call to \`${setterName}\`. Without a dependency array, this can lead to an infinite chain of updates.`;
50+
51+
export const buildRefCleanupMessage = (depName: string): string =>
52+
`The ref value \`${depName}\` will likely have changed by the time this effect cleanup function runs. Copy it to a variable inside the hook callback and use that variable in cleanup.`;
53+
54+
export const buildAssignmentMessage = (name: string): string =>
55+
`Assignments to the \`${name}\` variable from inside a React Hook will be lost after each render. Store it in a ref to preserve the value over time.`;

‎packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/exhaustive-deps-settings.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/**
2+
* Settings shape + resolver for the `exhaustive-deps` rule. Honors
3+
* both the canonical `settings["react-doctor"].exhaustiveDeps`
4+
* namespace and the upstream `settings["react-hooks"]` shape so a
5+
* project migrating from `eslint-plugin-react-hooks` keeps the same
6+
* configuration.
7+
*/
8+
export interface ExhaustiveDepsSettings {
9+
additionalHooks?: string;
10+
additionalEffectHooks?: string;
11+
enableDangerousAutofixThisMayCauseInfiniteLoops?: boolean;
12+
experimental_autoDependenciesHooks?: ReadonlyArray<string>;
13+
requireExplicitEffectDeps?: boolean;
14+
}
15+
16+
export const resolveExhaustiveDepsSettings = (
17+
settings: Readonly<Record<string, unknown>> | undefined,
18+
): Required<ExhaustiveDepsSettings> => {
19+
const reactDoctor = settings?.["react-doctor"];
20+
const reactHooks = settings?.["react-hooks"];
21+
const ruleSettings =
22+
typeof reactDoctor === "object" && reactDoctor !== null
23+
? ((reactDoctor as { exhaustiveDeps?: ExhaustiveDepsSettings }).exhaustiveDeps ?? {})
24+
: {};
25+
const upstreamSettings =
26+
typeof reactHooks === "object" && reactHooks !== null
27+
? (reactHooks as ExhaustiveDepsSettings)
28+
: {};
29+
return {
30+
additionalHooks:
31+
ruleSettings.additionalHooks ??
32+
ruleSettings.additionalEffectHooks ??
33+
upstreamSettings.additionalHooks ??
34+
upstreamSettings.additionalEffectHooks ??
35+
"",
36+
additionalEffectHooks:
37+
ruleSettings.additionalEffectHooks ?? upstreamSettings.additionalEffectHooks ?? "",
38+
enableDangerousAutofixThisMayCauseInfiniteLoops:
39+
ruleSettings.enableDangerousAutofixThisMayCauseInfiniteLoops ?? false,
40+
experimental_autoDependenciesHooks:
41+
ruleSettings.experimental_autoDependenciesHooks ??
42+
upstreamSettings.experimental_autoDependenciesHooks ??
43+
[],
44+
requireExplicitEffectDeps:
45+
ruleSettings.requireExplicitEffectDeps ?? upstreamSettings.requireExplicitEffectDeps ?? false,
46+
};
47+
};

‎packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/exhaustive-deps.ts‎

Lines changed: 21 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -13,88 +13,32 @@ import { isAstNode } from "../../utils/is-ast-node.js";
1313
import { isReactComponentOrHookName } from "../../utils/is-react-component-or-hook-name.js";
1414
import { isNodeOfType } from "../../utils/is-node-of-type.js";
1515
import type { Rule } from "../../utils/rule.js";
16+
import {
17+
buildAssignmentMessage,
18+
buildAsyncEffectMessage,
19+
buildComplexDepMessage,
20+
buildDuplicateDepMessage,
21+
buildEffectEventDepMessage,
22+
buildLiteralDepMessage,
23+
buildMissingCallbackMessage,
24+
buildMissingDepArrayMessage,
25+
buildMissingDepMessage,
26+
buildNonArrayDepsMessage,
27+
buildRefCleanupMessage,
28+
buildRefCurrentDepMessage,
29+
buildSetStateWithoutDepsMessage,
30+
buildSpreadDepMessage,
31+
buildUnknownCallbackMessage,
32+
buildUnnecessaryDepMessage,
33+
buildUnstableDepMessage,
34+
} from "./exhaustive-deps-messages.js";
35+
import { resolveExhaustiveDepsSettings } from "./exhaustive-deps-settings.js";
1636

1737
// Port of `oxc_linter::rules::react::exhaustive_deps`. Diffs the
1838
// closure-captured set of an effect / memo callback against its
1939
// declared dependency array. Built on top of Phase A's scope analyzer
2040
// and Phase C's closure-capture helper.
2141

22-
const buildMissingDepMessage = (hookName: string, depName: string): string =>
23-
`React Hook \`${hookName}\` is missing dependency \`${depName}\` — list it in the dependency array, or call the hook unconditionally.`;
24-
const buildUnnecessaryDepMessage = (hookName: string, depName: string): string =>
25-
`React Hook \`${hookName}\` has an unnecessary dependency \`${depName}\` — it isn't referenced inside the callback.`;
26-
const buildDuplicateDepMessage = (hookName: string, depName: string): string =>
27-
`React Hook \`${hookName}\` has duplicate dependency \`${depName}\`.`;
28-
const buildLiteralDepMessage = (hookName: string): string =>
29-
`React Hook \`${hookName}\` was passed a literal as a dependency. Literals never change so they cannot trigger an update — remove them from the dependency array.`;
30-
const buildRefCurrentDepMessage = (hookName: string, depName: string): string =>
31-
`React Hook \`${hookName}\` shouldn't include \`${depName}\` in the dependency array — mutable values like \`.current\` aren't valid deps; depend on \`${depName.replace(/\.current$/, "")}\` itself instead.`;
32-
const buildNonArrayDepsMessage = (hookName: string): string =>
33-
`React Hook \`${hookName}\` has a second argument which is not an array literal. This means oxlint cannot statically verify whether the dependencies are exhaustive — replace the variable with an inline array.`;
34-
const buildMissingDepArrayMessage = (hookName: string): string =>
35-
`React Hook \`${hookName}\` does nothing when called with only one argument — pass a dependency array as the second argument.`;
36-
const buildMissingCallbackMessage = (hookName: string): string =>
37-
`React Hook \`${hookName}\` requires an effect callback — pass a function as the first argument.`;
38-
const buildEffectEventDepMessage = (depName: string): string =>
39-
`Functions returned from \`useEffectEvent\` must not be included in the dependency array. Remove \`${depName}\` from the list.`;
40-
const buildSpreadDepMessage = (hookName: string): string =>
41-
`React Hook \`${hookName}\` has a spread element in its dependency array. This means oxlint cannot statically verify whether the dependencies are exhaustive.`;
42-
const buildComplexDepMessage = (hookName: string): string =>
43-
`React Hook \`${hookName}\` has a complex expression in the dependency array. Extract it to a separate variable so it can be statically checked.`;
44-
const buildAsyncEffectMessage = (hookName: string): string =>
45-
`React Hook \`${hookName}\` received an async callback. Put the async function inside the effect instead.`;
46-
const buildUnknownCallbackMessage = (hookName: string): string =>
47-
`React Hook \`${hookName}\` received a function whose dependencies are unknown. Pass an inline function instead.`;
48-
const buildUnstableDepMessage = (hookName: string, depName: string): string =>
49-
`The \`${depName}\` value makes the dependencies of \`${hookName}\` change on every render. Move it inside the hook callback or wrap it in its own memoization hook.`;
50-
const buildSetStateWithoutDepsMessage = (hookName: string, setterName: string): string =>
51-
`React Hook \`${hookName}\` contains a call to \`${setterName}\`. Without a dependency array, this can lead to an infinite chain of updates.`;
52-
const buildRefCleanupMessage = (depName: string): string =>
53-
`The ref value \`${depName}\` will likely have changed by the time this effect cleanup function runs. Copy it to a variable inside the hook callback and use that variable in cleanup.`;
54-
const buildAssignmentMessage = (name: string): string =>
55-
`Assignments to the \`${name}\` variable from inside a React Hook will be lost after each render. Store it in a ref to preserve the value over time.`;
56-
57-
interface ExhaustiveDepsSettings {
58-
additionalHooks?: string;
59-
additionalEffectHooks?: string;
60-
enableDangerousAutofixThisMayCauseInfiniteLoops?: boolean;
61-
experimental_autoDependenciesHooks?: ReadonlyArray<string>;
62-
requireExplicitEffectDeps?: boolean;
63-
}
64-
65-
const resolveSettings = (
66-
settings: Readonly<Record<string, unknown>> | undefined,
67-
): Required<ExhaustiveDepsSettings> => {
68-
const reactDoctor = settings?.["react-doctor"];
69-
const reactHooks = settings?.["react-hooks"];
70-
const ruleSettings =
71-
typeof reactDoctor === "object" && reactDoctor !== null
72-
? ((reactDoctor as { exhaustiveDeps?: ExhaustiveDepsSettings }).exhaustiveDeps ?? {})
73-
: {};
74-
const upstreamSettings =
75-
typeof reactHooks === "object" && reactHooks !== null
76-
? (reactHooks as ExhaustiveDepsSettings)
77-
: {};
78-
return {
79-
additionalHooks:
80-
ruleSettings.additionalHooks ??
81-
ruleSettings.additionalEffectHooks ??
82-
upstreamSettings.additionalHooks ??
83-
upstreamSettings.additionalEffectHooks ??
84-
"",
85-
additionalEffectHooks:
86-
ruleSettings.additionalEffectHooks ?? upstreamSettings.additionalEffectHooks ?? "",
87-
enableDangerousAutofixThisMayCauseInfiniteLoops:
88-
ruleSettings.enableDangerousAutofixThisMayCauseInfiniteLoops ?? false,
89-
experimental_autoDependenciesHooks:
90-
ruleSettings.experimental_autoDependenciesHooks ??
91-
upstreamSettings.experimental_autoDependenciesHooks ??
92-
[],
93-
requireExplicitEffectDeps:
94-
ruleSettings.requireExplicitEffectDeps ?? upstreamSettings.requireExplicitEffectDeps ?? false,
95-
};
96-
};
97-
9842
// Hooks whose callback captures must match a deps array.
9943
const HOOKS_REQUIRING_DEPS_MATCH: ReadonlySet<string> = new Set([
10044
"useEffect",
@@ -914,7 +858,7 @@ export const exhaustiveDeps = defineRule<Rule>({
914858
recommendation: "List every value the hook callback captures in its dependency array.",
915859
category: "Correctness",
916860
create: (context) => {
917-
const settings = resolveSettings(context.settings);
861+
const settings = resolveExhaustiveDepsSettings(context.settings);
918862
const additionalHooksRegex = buildAdditionalHooksRegex(settings.additionalHooks);
919863
const isHookOfInterest = (hookName: string, callee: EsTreeNode): boolean => {
920864
if (HOOKS_REQUIRING_DEPS_MATCH.has(hookName)) return true;

0 commit comments

Comments
 (0)