Skip to content

Commit 8397a2f

Browse files
authored
refactor: refs rule for improved lazy-init and nested writes detection (#1892)
1 parent f03e298 commit 8397a2f

5 files changed

Lines changed: 220 additions & 130 deletions

File tree

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,21 @@ All notable changes to the `react-x/refs` rule will be documented in this file.
55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
66
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

8+
## [Unreleased]
9+
10+
### Added
11+
12+
- Added detection of nested property writes on a ref's value (e.g. `ref.current.inner = value`), which are now reported as `writeDuringRender` instead of being misclassified as a read.
13+
- Added tracking of functions bound to (and called through) simple object-member-expression targets (e.g. `object.foo = () => ref.current; object.foo();`), closing a gap in the render-reachability analysis that previously only covered plain variable bindings.
14+
- Added detection of `ref.current` accesses inside the lazy initializer function passed directly as `useState`'s first argument, since it runs synchronously during the initial render unlike other hook-callback arguments.
15+
- Added an exemption for calls to a function named `render` (e.g. `props.render(ref)`, a common render-prop pattern) from the `refPassedToFunction` diagnostic, alongside the existing `mergeRefs`/hook exemptions.
16+
17+
### Changed
18+
19+
- Made lazy-init guard-block detection direction-aware: inside the branch of an `if (ref.current == null)`-style guard that is guaranteed to see `ref.current` as null, only a direct write is now treated as the (single) valid initialization - reads or values passed to a function there are still reported, matching `refs.spec.md`'s `ValidateNoRefAccessInRender` semantics. Reading the already-initialized value back in the other branch (the `if (x.current !== null) { return x.current; }` memoization idiom) remains allowed.
20+
- Replaced `isRefCurrentNullCheck` with `getRefCurrentNullCheckBranch` in `lib.ts`, which reports which branch of a guard is the null branch instead of just whether the test is a recognized null check.
21+
- Updated `refs.spec.diff.md` to reflect the fixes above and corrected the "Non-null Lazy Init Guards" entry, which was not an actual gap since `refs.spec.md` itself treats guards like `if (r.current == DEFAULT_VALUE)` as errors.
22+
823
## [5.11.0] - 2026-07-05
924

1025
### Added

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

Lines changed: 91 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,20 @@ import type { Scope } from "@typescript-eslint/scope-manager";
55
import { AST_NODE_TYPES as AST, type TSESTree } from "@typescript-eslint/types";
66
import { findVariable } from "@typescript-eslint/utils/ast-utils";
77

8+
export function isFunctionExpressionLike(node: TSESTree.Node): node is TSESTree.FunctionExpression | TSESTree.ArrowFunctionExpression {
9+
return node.type === AST.FunctionExpression || node.type === AST.ArrowFunctionExpression;
10+
}
11+
12+
export function resolveAlias(name: string, aliases: Map<string, string>): string {
13+
const seen = new Set<string>();
14+
while (aliases.has(name) && !seen.has(name)) {
15+
seen.add(name);
16+
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
17+
name = aliases.get(name)!;
18+
}
19+
return name;
20+
}
21+
822
/**
923
* Check if the node is the operand of a `ref.current === null` test inside an IfStatement.
1024
* @param node The MemberExpression node for ref.current
@@ -52,47 +66,103 @@ export function isInNullCheckTest(node: TSESTree.MemberExpression): boolean {
5266
return false;
5367
}
5468

55-
function isBinaryNullCheck(test: TSESTree.Expression, refName: string): boolean {
69+
function isBinaryNullCheckOperand(a: TSESTree.Node, b: TSESTree.Node, refName: string): boolean {
70+
a = Check.isTypeExpression(a) ? Extract.unwrap(a) : a;
71+
if (a.type !== AST.MemberExpression) return false;
72+
if (Extract.getPropertyName(a.property) !== "current") return false;
73+
const obj = Extract.unwrap(a.object);
74+
return obj.type === AST.Identifier && obj.name === refName && b.type === AST.Literal && b.value == null;
75+
}
76+
77+
function isBinaryNullCheck(test: TSESTree.Expression, refName: string): test is TSESTree.BinaryExpression {
5678
if (test.type !== AST.BinaryExpression) return false;
5779
if (!/^(===|==|!==|!=)$/.test(test.operator)) return false;
5880
const { left, right } = test;
59-
const checkSides = (a: TSESTree.Node, b: TSESTree.Node) => {
60-
a = Check.isTypeExpression(a) ? Extract.unwrap(a) : a;
61-
if (a.type !== AST.MemberExpression) return false;
62-
if (Extract.getPropertyName(a.property) !== "current") return false;
63-
const obj = Extract.unwrap(a.object);
64-
return obj.type === AST.Identifier && obj.name === refName && b.type === AST.Literal && b.value == null;
65-
};
66-
return checkSides(left, right) || checkSides(right, left);
81+
return isBinaryNullCheckOperand(left, right, refName) || isBinaryNullCheckOperand(right, left, refName);
6782
}
6883

6984
/**
70-
* Check if a test expression is a null check on `ref.current` for a given ref name.
71-
* Matches forms like `ref.current === null`, `null === ref.current`, `!ref.current`,
72-
* `!(ref.current === null)`, and their != variants.
85+
* Which branch of an `if` statement is guaranteed to observe `ref.current` as `null`/`undefined`,
86+
* for a test expression recognized by `isRefCurrentNullCheck`.
87+
*
88+
* `"consequent"` means the true-branch is the null branch (e.g. `== null`, `=== null`,
89+
* `!ref.current`); `"alternate"` means the false-branch is the null branch (e.g. `!= null`,
90+
* `!== null`, `!(ref.current === null)`).
91+
*/
92+
export type NullCheckBranch = "consequent" | "alternate";
93+
94+
/**
95+
* Determine which branch of an `if` statement is guaranteed to see `ref.current` as null, given
96+
* a test expression. Returns `null` if the test isn't a recognized null check on `refName`.
7397
* @param test The test expression to check.
7498
* @param refName The name of the ref variable.
7599
*/
76-
export function isRefCurrentNullCheck(test: TSESTree.Expression, refName: string): boolean {
77-
// Direct binary check
78-
if (isBinaryNullCheck(test, refName)) return true;
100+
export function getRefCurrentNullCheckBranch(test: TSESTree.Expression, refName: string): NullCheckBranch | null {
101+
// Direct binary check: ref.current === null / == null -> consequent is null branch
102+
// ref.current !== null / != null -> alternate is null branch
103+
if (isBinaryNullCheck(test, refName)) {
104+
return test.operator === "===" || test.operator === "==" ? "consequent" : "alternate";
105+
}
79106

80107
// !ref.current or !(ref.current === null)
81108
if (test.type === AST.UnaryExpression && test.operator === "!") {
82109
const arg = Extract.unwrap(test.argument);
83-
// !ref.current
110+
// !ref.current: truthy check, so the true-branch (falsy, i.e. null-like) is the null branch
84111
if (arg.type === AST.MemberExpression) {
85112
const obj = Extract.unwrap(arg.object);
86-
return obj.type === AST.Identifier
113+
const isRefCurrent = obj.type === AST.Identifier
87114
&& obj.name === refName
88115
&& Extract.getPropertyName(arg.property) === "current";
116+
return isRefCurrent ? "consequent" : null;
117+
}
118+
// !(ref.current === null): flip the inner binary check's direction
119+
if (arg.type === AST.BinaryExpression) {
120+
const inner = getRefCurrentNullCheckBranch(arg, refName);
121+
if (inner === "consequent") return "alternate";
122+
if (inner === "alternate") return "consequent";
89123
}
90-
// !(ref.current === null)
91-
// tsl-ignore dx/no-unsafe-as
92-
return isBinaryNullCheck(arg as TSESTree.Expression, refName);
93124
}
94125

95-
return false;
126+
return null;
127+
}
128+
129+
/**
130+
* Check if a test expression is a null check on `ref.current` for a given ref name.
131+
* Matches forms like `ref.current === null`, `null === ref.current`, `!ref.current`,
132+
* `!(ref.current === null)`, and their != variants.
133+
* @param test The test expression to check.
134+
* @param refName The name of the ref variable.
135+
*/
136+
export function isRefCurrentNullCheck(test: TSESTree.Expression, refName: string): boolean {
137+
return getRefCurrentNullCheckBranch(test, refName) != null;
138+
}
139+
140+
/**
141+
* Check whether `node` (a `ref.current` MemberExpression) is being written to indirectly through
142+
* a nested property write, e.g. `ref.current.inner = value` or `ref.current.inner++`.
143+
* @param node The MemberExpression node for ref.current
144+
*/
145+
export function isNestedRefCurrentWrite(node: TSESTree.MemberExpression): boolean {
146+
let outer: TSESTree.Node = node;
147+
let sawNesting = false;
148+
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
149+
while (true) {
150+
let parent: TSESTree.Node = outer.parent;
151+
while (Check.isTypeExpression(parent)) parent = parent.parent;
152+
if (parent.type === AST.MemberExpression && (parent.object === outer || Extract.unwrap(parent.object) === outer)) {
153+
outer = parent;
154+
sawNesting = true;
155+
continue;
156+
}
157+
if (!sawNesting) return false;
158+
if (parent.type === AST.AssignmentExpression) {
159+
return parent.left === outer || Extract.unwrap(parent.left) === outer;
160+
}
161+
if (parent.type === AST.UpdateExpression) {
162+
return parent.argument === outer || Extract.unwrap(parent.argument) === outer;
163+
}
164+
return false;
165+
}
96166
}
97167

98168
export function isInitializedFromRef(context: RuleContext, name: string, initialScope: Scope) {

0 commit comments

Comments
 (0)