Skip to content

Clarify purity for null-guarded ref initialization#674

Closed
everettbu wants to merge 1 commit into
mainfrom
codex/clarify-ref-init-purity
Closed

Clarify purity for null-guarded ref initialization#674
everettbu wants to merge 1 commit into
mainfrom
codex/clarify-ref-init-purity

Conversation

@everettbu

Copy link
Copy Markdown

Mirror of facebook/react#35987
Original author: angular-bootstrap


Summary

Clarifies the interaction between the refs and purity lints for null-guarded ref initialization.

The null-guard pattern:

if (ref.current == null) {
  ref.current = ...
}

is allowed by the refs rule, but purity still applies to the initializer on the right-hand side. This change adds regression coverage for that behavior and updates the ref-validation docs to make the distinction explicit.

Changes

Add a purity regression test showing Date.now() still fails inside a null-guarded ref initialization.
Add a refs regression test showing the null-guarded stable assignment pattern remains allowed.
Clarify docs for validateNoRefAccessInRender to note that the exception only applies to ref access, not to impure initializer expressions.

Test Plan

yarn test ImpureFunctionCallsRule-test --runInBand
yarn test NoRefAccessInRender-tests --runInBand

Issue

Closes #35973

@greptile-apps

greptile-apps Bot commented Mar 10, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds regression tests and documentation clarifying that the null-guard ref initialization exception (if (ref.current == null) { ref.current = ... }) only exempts the ref access pattern from the Refs rule — it does not exempt the initializer expression from the Purity rule. The changes are narrowly scoped to tests and docs with no production code modifications.

  • Added a valid test in NoRefAccessInRender-tests.ts confirming that a null-guarded stable ref assignment passes the Refs rule.
  • Added an invalid test in ImpureFunctionCallsRule-test.ts confirming that Date.now() inside a null-guarded ref initialization still fails the Purity rule (one error expected).
  • Updated 49-validateNoRefAccessInRender.md with an explicit clarification paragraph scoping the exception to ref access only.

Confidence Score: 5/5

  • Safe to merge — changes are limited to tests and documentation with no production code impact.
  • All three changed files are either test files or markdown documentation. No production logic is modified. The new test cases are consistent with the documented behavior, and the documentation addition accurately reflects the existing implementation semantics.
  • No files require special attention, though the new purity test case in ImpureFunctionCallsRule-test.ts includes a ref.current read in the return that would also violate the Refs rule — it's harmless here since only the Purity rule is exercised, but could be simplified.

Important Files Changed

Filename Overview
compiler/packages/babel-plugin-react-compiler/docs/passes/49-validateNoRefAccessInRender.md Added a clarifying paragraph explicitly scoping the null-guard exception to ref access only, leaving purity checks intact for initializer expressions.
compiler/packages/eslint-plugin-react-compiler/tests/ImpureFunctionCallsRule-test.ts Added an invalid test case verifying Date.now() inside a null-guarded ref init still triggers a purity error; test uses strict equality (===) while the docs canonical pattern uses loose equality (==).
compiler/packages/eslint-plugin-react-compiler/tests/NoRefAccessInRender-tests.ts Added first valid test case demonstrating that a null-guarded stable ref initialization is allowed by the refs rule.

Fix All in Claude Code Fix All in Codex

Last reviewed commit: a494195

if (ref.current === null) {
ref.current = Date.now();
}
return <div>{ref.current}</div>;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ref read in return also violates the Refs rule

ref.current is read inside the JSX return expression, which would itself be a Refs violation in addition to the purity violation being tested. Because this test suite runs only the Purity rule, the refs error is never surfaced, but the test component as written is invalid under both rules.

To keep the test focused solely on the purity regression, consider dropping the ref read from the return value:

Suggested change
return <div>{ref.current}</div>;
return <div />;

This matches the style used in the new valid test in NoRefAccessInRender-tests.ts (line 28) and avoids unintentionally implying that reading ref.current in JSX is acceptable in this context.

Prompt To Fix With AI
This is a comment left during a code review.
Path: compiler/packages/eslint-plugin-react-compiler/__tests__/ImpureFunctionCallsRule-test.ts
Line: 45

Comment:
**Ref read in return also violates the Refs rule**

`ref.current` is read inside the JSX return expression, which would itself be a `Refs` violation in addition to the purity violation being tested. Because this test suite runs only the `Purity` rule, the refs error is never surfaced, but the test component as written is invalid under _both_ rules.

To keep the test focused solely on the purity regression, consider dropping the ref read from the return value:

```suggestion
        return <div />;
```

This matches the style used in the new valid test in `NoRefAccessInRender-tests.ts` (line 28) and avoids unintentionally implying that reading `ref.current` in JSX is acceptable in this context.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code Fix in Codex

@everettbu

Copy link
Copy Markdown
Author

Upstream PR was closed or merged. Code is synced via branch mirror.

@everettbu everettbu closed this Mar 10, 2026
@everettbu
everettbu deleted the codex/clarify-ref-init-purity branch March 10, 2026 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants