Skip to content

Conversation

josephsavona
Copy link
Member

@josephsavona josephsavona commented Jul 25, 2025

Two related changes:

  • ValidateNoRefAccessInRender now allows the mergeRefs pattern, ie a function that aggregates multiple refs into a new ref. This is the main case where we have seen false positive no-ref-in-render errors.
  • Behind @enableTreatRefLikeIdentifiersAsRefs, we infer values passed as the ref prop to some JSX as refs.

The second change is potentially helpful for situations such as

function Component({ref: parentRef}) {
  const childRef = useRef(null);
  const mergedRef = mergeRefs(parentRef, childRef);
  useEffect(() => {
    // generally accesses childRef, not mergedRef
  }, []);
  return <Foo ref={mergedRef} />;
}

Ie where you create a merged ref but don't access its .current property. Without inferring ref props as refs, we'd fail to allow this merge refs case.


Stack created with Sapling. Best reviewed with ReviewStack.

@josephsavona
Copy link
Member Author

Closes #31742

josephsavona added a commit that referenced this pull request Jul 29, 2025
We added the `@enableTreatRefLikeIdentifiersAsRefs` feature a while back
but never enabled it. Since then we've continued to see examples that
motivate this mode, so here we're fixing it up to prepare to enable by
default. It now works as follows:

* If we find a property load or property store where both a) the
object's name is ref-like (`ref` or `-Ref`) and b) the property is
`current`, we infer the object itself as a ref and the value of the
property as a ref value. Originally the feature only detected property
loads, not stores.
* Inferred refs are not considered stable (this is a change from the
original implementation). The only way to get a stable ref is by calling
`useRef()`. We've seen issues with assuming refs are stable.

With this change, cases like the following now correctly error:

```js
function Foo(props) {
  const fooRef = props.fooRef;
  fooRef.current = true;
  ^^^^^^^^^^^^^^ cannot modify ref in render
}
```

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34000).
* #34027
* #34026
* #34025
* #34024
* #34005
* #34006
* #34004
* #34003
* __->__ #34000
josephsavona added a commit that referenced this pull request Jul 29, 2025
Improves the error message for ValidateNoRefAccessInRender, using the
new diagnostic type as well as providing a longer but succinct summary
of what refs are for and why they're unsafe to access in render.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34003).
* #34027
* #34026
* #34025
* #34024
* #34005
* #34006
* #34004
* __->__ #34003
Two related changes:
* ValidateNoRefAccessInRender now allows the mergeRefs pattern, ie a function that aggregates multiple refs into a new ref. This is the main case where we have seen false positive no-ref-in-render errors.
* Behind `@enableTreatRefLikeIdentifiersAsRefs`, we infer values passed as the `ref` prop to some JSX as refs.

The second change is potentially helpful for situations such as

```js
function Component({ref: parentRef}) {
  const childRef = useRef(null);
  const mergedRef = mergeRefs(parentRef, childRef);
  useEffect(() => {
    // generally accesses childRef, not mergedRef
  }, []);
  return <Foo ref={mergedRef} />;
}
```

Ie where you create a merged ref but don't access its `.current` property. Without inferring `ref` props as refs, we'd fail to allow this merge refs case.
github-actions bot pushed a commit that referenced this pull request Jul 29, 2025
We added the `@enableTreatRefLikeIdentifiersAsRefs` feature a while back
but never enabled it. Since then we've continued to see examples that
motivate this mode, so here we're fixing it up to prepare to enable by
default. It now works as follows:

* If we find a property load or property store where both a) the
object's name is ref-like (`ref` or `-Ref`) and b) the property is
`current`, we infer the object itself as a ref and the value of the
property as a ref value. Originally the feature only detected property
loads, not stores.
* Inferred refs are not considered stable (this is a change from the
original implementation). The only way to get a stable ref is by calling
`useRef()`. We've seen issues with assuming refs are stable.

With this change, cases like the following now correctly error:

```js
function Foo(props) {
  const fooRef = props.fooRef;
  fooRef.current = true;
  ^^^^^^^^^^^^^^ cannot modify ref in render
}
```

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34000).
* #34027
* #34026
* #34025
* #34024
* #34005
* #34006
* #34004
* #34003
* __->__ #34000

DiffTrain build for [85bbe39](85bbe39)
github-actions bot pushed a commit that referenced this pull request Jul 29, 2025
We added the `@enableTreatRefLikeIdentifiersAsRefs` feature a while back
but never enabled it. Since then we've continued to see examples that
motivate this mode, so here we're fixing it up to prepare to enable by
default. It now works as follows:

* If we find a property load or property store where both a) the
object's name is ref-like (`ref` or `-Ref`) and b) the property is
`current`, we infer the object itself as a ref and the value of the
property as a ref value. Originally the feature only detected property
loads, not stores.
* Inferred refs are not considered stable (this is a change from the
original implementation). The only way to get a stable ref is by calling
`useRef()`. We've seen issues with assuming refs are stable.

With this change, cases like the following now correctly error:

```js
function Foo(props) {
  const fooRef = props.fooRef;
  fooRef.current = true;
  ^^^^^^^^^^^^^^ cannot modify ref in render
}
```

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34000).
* #34027
* #34026
* #34025
* #34024
* #34005
* #34006
* #34004
* #34003
* __->__ #34000

DiffTrain build for [85bbe39](85bbe39)
@josephsavona josephsavona merged commit 1d7e942 into main Jul 29, 2025
20 of 22 checks passed
josephsavona added a commit that referenced this pull request Jul 29, 2025
We infer render helpers as functions whose result is immediately
interpolated into jsx. This is a very conservative approximation, to
help with common cases like `<Foo>{props.renderItem(ref)}</Foo>`. The
idea is similar to hooks that it's ultimately on the developer to catch
ref-in-render validations (and the runtime detects them too), so we can
be a bit more relaxed since there are valid reasons to use this pattern.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34006).
* #34027
* #34026
* #34025
* #34024
* #34005
* __->__ #34006
* #34004
github-actions bot pushed a commit that referenced this pull request Jul 29, 2025
Improves the error message for ValidateNoRefAccessInRender, using the
new diagnostic type as well as providing a longer but succinct summary
of what refs are for and why they're unsafe to access in render.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34003).
* #34027
* #34026
* #34025
* #34024
* #34005
* #34006
* #34004
* __->__ #34003

DiffTrain build for [79dc706](79dc706)
github-actions bot pushed a commit that referenced this pull request Jul 29, 2025
Improves the error message for ValidateNoRefAccessInRender, using the
new diagnostic type as well as providing a longer but succinct summary
of what refs are for and why they're unsafe to access in render.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34003).
* #34027
* #34026
* #34025
* #34024
* #34005
* #34006
* #34004
* __->__ #34003

DiffTrain build for [79dc706](79dc706)
github-actions bot pushed a commit that referenced this pull request Jul 29, 2025
…p) (#34004)

Two related changes:
* ValidateNoRefAccessInRender now allows the mergeRefs pattern, ie a
function that aggregates multiple refs into a new ref. This is the main
case where we have seen false positive no-ref-in-render errors.
* Behind `@enableTreatRefLikeIdentifiersAsRefs`, we infer values passed
as the `ref` prop to some JSX as refs.

The second change is potentially helpful for situations such as

```js
function Component({ref: parentRef}) {
  const childRef = useRef(null);
  const mergedRef = mergeRefs(parentRef, childRef);
  useEffect(() => {
    // generally accesses childRef, not mergedRef
  }, []);
  return <Foo ref={mergedRef} />;
}
```

Ie where you create a merged ref but don't access its `.current`
property. Without inferring `ref` props as refs, we'd fail to allow this
merge refs case.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34004).
* #34027
* #34026
* #34025
* #34024
* #34005
* #34006
* __->__ #34004

DiffTrain build for [1d7e942](1d7e942)
github-actions bot pushed a commit that referenced this pull request Jul 29, 2025
…p) (#34004)

Two related changes:
* ValidateNoRefAccessInRender now allows the mergeRefs pattern, ie a
function that aggregates multiple refs into a new ref. This is the main
case where we have seen false positive no-ref-in-render errors.
* Behind `@enableTreatRefLikeIdentifiersAsRefs`, we infer values passed
as the `ref` prop to some JSX as refs.

The second change is potentially helpful for situations such as

```js
function Component({ref: parentRef}) {
  const childRef = useRef(null);
  const mergedRef = mergeRefs(parentRef, childRef);
  useEffect(() => {
    // generally accesses childRef, not mergedRef
  }, []);
  return <Foo ref={mergedRef} />;
}
```

Ie where you create a merged ref but don't access its `.current`
property. Without inferring `ref` props as refs, we'd fail to allow this
merge refs case.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34004).
* #34027
* #34026
* #34025
* #34024
* #34005
* #34006
* __->__ #34004

DiffTrain build for [1d7e942](1d7e942)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants