Skip to content

Commit 895c25b

Browse files
authored
fix(react-x/immutability): false positive on ref.current write inside useEffect, closes #1893 (#1894)
1 parent f8e27b3 commit 895c25b

5 files changed

Lines changed: 58 additions & 7 deletions

File tree

plugins/eslint-plugin-react-x/src/rules/immutability/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- Fixed a false positive where mutating `.current` on a `useRef()`-initialized variable whose name didn't follow the `ref`/`*Ref` naming convention (e.g. `const mounted = useRef(false); mounted.current = true;`) was incorrectly flagged when the containing function was passed to a hook. Added `isInitializedFromUseRef`/`isRefLikeChain` in `lib.ts` to also exempt refs by call-site detection (`useRef()`), not naming alone. Closes #1893. (#1893)
13+
1014
## [5.12.0] - 2026-07-08
1115

1216
### Added

plugins/eslint-plugin-react-x/src/rules/immutability/immutability.spec.diff.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,12 @@ The IMPL:
6464

6565
The SPEC exempts ref mutations via `isRefOrRefLikeMutableType`, a type-based check.
6666

67-
The IMPL exempts ref mutations via a purely syntactic naming heuristic (`isRefLikeName`/`hasRefLikeNameInChain`): any identifier or property named `ref` or ending in `Ref` anywhere in the mutated member-expression chain is treated as a ref and skipped. This mirrors the heuristic already used elsewhere in this rule family (see `refs.spec.diff.md` §2) and additionally covers refs received as props (e.g. `props.myRef.current = x`) without needing type information.
67+
The IMPL exempts ref mutations via two complementary, non-type-based checks in `lib.ts`:
6868

69-
**Verdict**: Functionally equivalent intent, different mechanism (naming convention rather than type). Can under- or over-exempt in cases where naming doesn't match the heuristic.
69+
- A syntactic naming heuristic (`isRefLikeName`/`hasRefLikeNameInChain`): any identifier or property named `ref` or ending in `Ref` anywhere in the mutated member-expression chain is treated as a ref and skipped. This mirrors the heuristic already used elsewhere in this rule family (see `refs.spec.diff.md` §2) and additionally covers refs received as props (e.g. `props.myRef.current = x`) without needing type information.
70+
- A call-site check (`isInitializedFromUseRef`, combined with the naming heuristic in `isRefLikeChain`): the root identifier of the mutated chain is resolved via `resolve()` and exempted if its initializer is a `useRef()` call, regardless of the variable's name (e.g. `const mounted = useRef(false); mounted.current = true;`). Added to fix #1893, where refs named without the `ref`/`*Ref` convention were incorrectly flagged.
71+
72+
**Verdict**: Closer to the SPEC's type-based exemption than naming alone, since `useRef()` results are now recognized independently of their variable name. Still not fully equivalent: a ref narrowed through an intermediate alias, a custom ref-like hook, or a prop that isn't named `ref`/`*Ref` and wasn't itself produced by a local `useRef()` call is not detected (matching the `refPassedToFunction`-style gaps noted in `refs.spec.diff.md`).
7073

7174
---
7275

@@ -89,3 +92,4 @@ The IMPL emits **two separate ESLint reports** per violation, since ESLint's rep
8992
2. **Limited aliasing**: Only `const`/`let`-with-initializer aliasing of the _function_ is followed; aliasing of the _mutated object_ itself, reassigned aliases, and property-stored functions are not tracked.
9093
3. **Fixed mutating-method list**: `MUTATING_METHODS` is a static allow-list rather than a type-driven determination of "known mutable" values.
9194
4. **Split diagnostics**: The usage-site and mutation-site messages are reported as two independent ESLint problems instead of one diagnostic with two annotated locations, since ESLint has no native multi-location diagnostic model.
95+
5. **Ref exemption still has blind spots**: `isRefLikeChain` now recognizes refs both by naming convention and by a direct `useRef()` initializer (fixed #1893), but `isInitializedFromUseRef` calls `resolve()` only once (no recursive alias-chasing like `resolveToFunctionNode` does for functions). So a ref aliased through a plain variable (`const r = mounted; r.current = true;`) is not recognized unless `r`/`mounted` itself matches the naming heuristic. Refs threaded through a custom hook, a `forwardRef`/`useImperativeHandle` boundary, or destructuring are likewise not recognized.

plugins/eslint-plugin-react-x/src/rules/immutability/immutability.spec.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -482,5 +482,18 @@ ruleTester.run(RULE_NAME, rule, {
482482
return <Foo fn={fn} />;
483483
}
484484
`,
485+
// https://github.com/Rel1cx/eslint-react/issues/1893
486+
tsx`
487+
import { useEffect, useRef } from "react";
488+
489+
export function Component() {
490+
const mounted = useRef<boolean>(false);
491+
useEffect(() => {
492+
if (mounted.current) return;
493+
mounted.current = true;
494+
}, []);
495+
return <div/>;
496+
}
497+
`,
485498
],
486499
});

plugins/eslint-plugin-react-x/src/rules/immutability/immutability.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import * as core from "@eslint-react/core";
44
import { type RuleContext, type RuleFeature, type RuleListener, merge } from "@eslint-react/eslint";
55
import { AST_NODE_TYPES as AST, type TSESTree } from "@typescript-eslint/types";
66
import { findVariable } from "@typescript-eslint/utils/ast-utils";
7-
import { MUTATING_METHODS, hasRefLikeNameInChain, isNodeWithin, isRefLikeName, resolveToFunctionNode } from "./lib";
7+
import { MUTATING_METHODS, isNodeWithin, isRefLikeChain, isRefLikeName, resolveToFunctionNode } from "./lib";
88

99
export const RULE_NAME = "immutability";
1010

@@ -63,7 +63,7 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
6363
return;
6464
}
6565
case AST.MemberExpression: {
66-
if (hasRefLikeNameInChain(left)) return;
66+
if (isRefLikeChain(context, left)) return;
6767
const rootId = Extract.getRootIdentifier(left);
6868
if (rootId == null) return;
6969
pushMutationSite(node, rootId);
@@ -75,7 +75,7 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
7575
const callee = Extract.unwrap(node.callee);
7676
if (callee.type === AST.MemberExpression) {
7777
const propName = Extract.getPropertyName(callee.property);
78-
if (propName != null && MUTATING_METHODS.has(propName) && !hasRefLikeNameInChain(callee.object)) {
78+
if (propName != null && MUTATING_METHODS.has(propName) && !isRefLikeChain(context, callee.object)) {
7979
const rootId = Extract.getRootIdentifier(callee.object);
8080
if (rootId != null) pushMutationSite(node, rootId);
8181
}
@@ -153,7 +153,7 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
153153
if (node.operator !== "delete") return;
154154
const arg = Extract.unwrap(node.argument);
155155
if (arg.type !== AST.MemberExpression) return;
156-
if (hasRefLikeNameInChain(arg)) return;
156+
if (isRefLikeChain(context, arg)) return;
157157
const rootId = Extract.getRootIdentifier(arg);
158158
if (rootId == null) return;
159159
pushMutationSite(node, rootId);
@@ -167,7 +167,7 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
167167
return;
168168
}
169169
case AST.MemberExpression: {
170-
if (hasRefLikeNameInChain(arg)) return;
170+
if (isRefLikeChain(context, arg)) return;
171171
const rootId = Extract.getRootIdentifier(arg);
172172
if (rootId == null) return;
173173
pushMutationSite(node, rootId);

plugins/eslint-plugin-react-x/src/rules/immutability/lib.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { Check, Extract, type TSESTreeFunction } from "@eslint-react/ast";
2+
import * as core from "@eslint-react/core";
23
import type { RuleContext } from "@eslint-react/eslint";
34
import { resolve } from "@eslint-react/var";
45
import { AST_NODE_TYPES as AST, type TSESTree } from "@typescript-eslint/types";
@@ -77,3 +78,32 @@ export function hasRefLikeNameInChain(node: TSESTree.Node): boolean {
7778
}
7879
return false;
7980
}
81+
82+
/**
83+
* Check if the root identifier of a member-expression chain (or the
84+
* identifier itself) is initialized directly from a `useRef()` call, e.g.
85+
* `const mounted = useRef(false); mounted.current = true;`.
86+
*
87+
* This catches refs regardless of naming convention, complementing
88+
* {@link hasRefLikeNameInChain}.
89+
* @param context The ESLint rule context.
90+
* @param node The AST node to inspect (an identifier or member-expression chain).
91+
*/
92+
export function isInitializedFromUseRef(context: RuleContext, node: TSESTree.Expression): boolean {
93+
const rootId = node.type === AST.Identifier ? node : Extract.getRootIdentifier(node);
94+
if (rootId == null) return false;
95+
const initNode = resolve(context, rootId);
96+
return initNode != null && initNode.type === AST.CallExpression && core.isUseRefCall(context, initNode);
97+
}
98+
99+
/**
100+
* Check if a mutated expression chain should be exempt from immutability
101+
* checks because it is rooted at a ref: either by naming convention
102+
* ({@link hasRefLikeNameInChain}) or because it is initialized from a
103+
* `useRef()` call ({@link isInitializedFromUseRef}).
104+
* @param context The ESLint rule context.
105+
* @param node The AST node to inspect (an identifier or member-expression chain).
106+
*/
107+
export function isRefLikeChain(context: RuleContext, node: TSESTree.Expression): boolean {
108+
return hasRefLikeNameInChain(node) || isInitializedFromUseRef(context, node);
109+
}

0 commit comments

Comments
 (0)