Compare commits

..

4 Commits

Author SHA1 Message Date
Joe Savona
be41c94e5c [compiler] ref guards apply up to fallthrough of the test
Fixes #30782

When developers do an `if (ref.current == null)` guard for lazy ref initialization, the "safe" blocks should extend up to the if's fallthrough. Previously we only allowed writing to the ref in the if consequent, but this meant that you couldn't use a ternary, logical, etc in the if body.
2025-07-29 10:52:47 -07:00
Joseph Savona
6891dcb87d [compiler] treat ref-like identifiers as refs by default (#34005)
`@enableTreatRefLikeIdentifiersAsRefs` is now on by default. I made one
small fix to the render helper logic as part of this, uncovered by
including more tests.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34005).
* #34027
* #34026
* #34025
* #34024
* __->__ #34005
2025-07-29 10:51:10 -07:00
Joseph Savona
3f40eb73a8 [compiler] Allow passing refs to render helpers (#34006)
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
2025-07-29 10:06:23 -07:00
Joseph Savona
1d7e942da7 [compiler] Allow mergeRefs pattern (and detect refs passed as ref prop) (#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
2025-07-29 10:06:11 -07:00
24 changed files with 309 additions and 159 deletions

View File

@@ -608,7 +608,7 @@ export const EnvironmentConfigSchema = z.object({
*
* Here the variables `ref` and `myRef` will be typed as Refs.
*/
enableTreatRefLikeIdentifiersAsRefs: z.boolean().default(false),
enableTreatRefLikeIdentifiersAsRefs: z.boolean().default(true),
/*
* If specified a value, the compiler lowers any calls to `useContext` to use

View File

@@ -27,6 +27,7 @@ import {
eachTerminalOperand,
} from '../HIR/visitors';
import {Err, Ok, Result} from '../Utils/Result';
import {retainWhere} from '../Utils/utils';
/**
* Validates that a function does not access a ref value during render. This includes a partial check
@@ -279,9 +280,10 @@ function validateNoRefAccessInRenderImpl(
for (let i = 0; (i == 0 || env.hasChanged()) && i < 10; i++) {
env.resetChanged();
returnValues = [];
const safeBlocks = new Map<BlockId, RefId>();
const safeBlocks: Array<{block: BlockId; ref: RefId}> = [];
const errors = new CompilerError();
for (const [, block] of fn.body.blocks) {
retainWhere(safeBlocks, entry => entry.block !== block.id);
for (const phi of block.phis) {
env.set(
phi.place.identifier.id,
@@ -432,15 +434,11 @@ function validateNoRefAccessInRenderImpl(
* By default we check that function call operands are not refs,
* ref values, or functions that can access refs.
*/
if (
isRefLValue ||
interpolatedAsJsx.has(instr.lvalue.identifier.id) ||
hookKind != null
) {
if (isRefLValue || hookKind != null) {
/**
* Special cases:
*
* 1) the lvalue is a ref
* 1. the lvalue is a ref
* In general passing a ref to a function may access that ref
* value during render, so we disallow it.
*
@@ -452,18 +450,21 @@ function validateNoRefAccessInRenderImpl(
*
* Eg `const mergedRef = mergeRefs(ref1, ref2)`
*
* 2) the lvalue is passed as a jsx child
*
* For example `<Foo>{renderHelper(ref)}</Foo>`. Here we have more
* context and infer that the ref is being passed to a component-like
* render function which attempts to obey the rules.
*
* 3) hooks
* 2. calling hooks
*
* Hooks are independently checked to ensure they don't access refs
* during render.
*/
validateNoDirectRefValueAccess(errors, operand, env);
} else if (interpolatedAsJsx.has(instr.lvalue.identifier.id)) {
/**
* Special case: the lvalue is passed as a jsx child
*
* For example `<Foo>{renderHelper(ref)}</Foo>`. Here we have more
* context and infer that the ref is being passed to a component-like
* render function which attempts to obey the rules.
*/
validateNoRefValueAccess(errors, env, operand);
} else {
validateNoRefPassedToFunction(
errors,
@@ -504,15 +505,17 @@ function validateNoRefAccessInRenderImpl(
case 'PropertyStore':
case 'ComputedDelete':
case 'ComputedStore': {
const safe = safeBlocks.get(block.id);
const target = env.get(instr.value.object.identifier.id);
let safe: (typeof safeBlocks)['0'] | null | undefined = null;
if (
instr.value.kind === 'PropertyStore' &&
safe != null &&
target?.kind === 'Ref' &&
target.refId === safe
target != null &&
target.kind === 'Ref'
) {
safeBlocks.delete(block.id);
safe = safeBlocks.find(entry => entry.ref === target.refId);
}
if (safe != null) {
retainWhere(safeBlocks, entry => entry !== safe);
} else {
validateNoRefUpdate(errors, env, instr.value.object, instr.loc);
}
@@ -600,8 +603,11 @@ function validateNoRefAccessInRenderImpl(
if (block.terminal.kind === 'if') {
const test = env.get(block.terminal.test.identifier.id);
if (test?.kind === 'Guard') {
safeBlocks.set(block.terminal.consequent, test.refId);
if (
test?.kind === 'Guard' &&
safeBlocks.find(entry => entry.ref === test.refId) == null
) {
safeBlocks.push({block: block.terminal.fallthrough, ref: test.refId});
}
}

View File

@@ -27,6 +27,7 @@ function Component() {
}
function Child({ref}) {
'use no memo';
// This violates the rules of React, so we access the ref in a child
// component
return ref.current;
@@ -100,8 +101,10 @@ function Component() {
return t6;
}
function Child(t0) {
const { ref } = t0;
function Child({ ref }) {
"use no memo";
// This violates the rules of React, so we access the ref in a child
// component
return ref.current;
}

View File

@@ -23,6 +23,7 @@ function Component() {
}
function Child({ref}) {
'use no memo';
// This violates the rules of React, so we access the ref in a child
// component
return ref.current;

View File

@@ -23,6 +23,7 @@ function Component() {
}
function Child({ref}) {
'use no memo';
// This violates the rules of React, so we access the ref in a child
// component
return ref.current;
@@ -86,8 +87,10 @@ function Component() {
return t5;
}
function Child(t0) {
const { ref } = t0;
function Child({ ref }) {
"use no memo";
// This violates the rules of React, so we access the ref in a child
// component
return ref.current;
}

View File

@@ -19,6 +19,7 @@ function Component() {
}
function Child({ref}) {
'use no memo';
// This violates the rules of React, so we access the ref in a child
// component
return ref.current;

View File

@@ -25,6 +25,7 @@ function Component() {
}
function Child({ref}) {
'use no memo';
// This violates the rules of React, so we access the ref in a child
// component
return ref.current;
@@ -83,8 +84,10 @@ function Component() {
}
function _temp() {}
function Child(t0) {
const { ref } = t0;
function Child({ ref }) {
"use no memo";
// This violates the rules of React, so we access the ref in a child
// component
return ref.current;
}

View File

@@ -21,6 +21,7 @@ function Component() {
}
function Child({ref}) {
'use no memo';
// This violates the rules of React, so we access the ref in a child
// component
return ref.current;

View File

@@ -0,0 +1,68 @@
## Input
```javascript
// @validateRefAccessDuringRender
import {useRef} from 'react';
function Component(props) {
const ref = useRef(null);
if (ref.current == null) {
// the logical means the ref write is in a different block
// from the if consequent. this tests that the "safe" blocks
// extend up to the if's fallthrough
ref.current = props.unknownKey ?? props.value;
}
return <Child ref={ref} />;
}
function Child({ref}) {
'use no memo';
return ref.current;
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{value: 42}],
};
```
## Code
```javascript
import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRender
import { useRef } from "react";
function Component(props) {
const $ = _c(1);
const ref = useRef(null);
if (ref.current == null) {
ref.current = props.unknownKey ?? props.value;
}
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = <Child ref={ref} />;
$[0] = t0;
} else {
t0 = $[0];
}
return t0;
}
function Child({ ref }) {
"use no memo";
return ref.current;
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{ value: 42 }],
};
```
### Eval output
(kind: ok) 42

View File

@@ -0,0 +1,24 @@
// @validateRefAccessDuringRender
import {useRef} from 'react';
function Component(props) {
const ref = useRef(null);
if (ref.current == null) {
// the logical means the ref write is in a different block
// from the if consequent. this tests that the "safe" blocks
// extend up to the if's fallthrough
ref.current = props.unknownKey ?? props.value;
}
return <Child ref={ref} />;
}
function Child({ref}) {
'use no memo';
return ref.current;
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{value: 42}],
};

View File

@@ -29,7 +29,7 @@ error.invalid-aliased-ref-in-callback-invoked-during-render-.ts:9:33
7 | return <Foo item={item} current={current} />;
8 | };
> 9 | return <Items>{props.items.map(item => renderItem(item))}</Items>;
| ^^^^^^^^^^^^^^^^^^^^^^^^ Passing a ref to a function may read its value during render
| ^^^^^^^^^^^^^^^^^^^^^^^^ Cannot access ref value during render
10 | }
11 |
```

View File

@@ -28,7 +28,7 @@ error.invalid-ref-in-callback-invoked-during-render.ts:8:33
6 | return <Foo item={item} current={current} />;
7 | };
> 8 | return <Items>{props.items.map(item => renderItem(item))}</Items>;
| ^^^^^^^^^^^^^^^^^^^^^^^^ Passing a ref to a function may read its value during render
| ^^^^^^^^^^^^^^^^^^^^^^^^ Cannot access ref value during render
9 | }
10 |
```

View File

@@ -1,47 +0,0 @@
## Input
```javascript
import {Stringify, identity, mutate, CONST_TRUE} from 'shared-runtime';
function Foo(props, ref) {
const value = {};
if (CONST_TRUE) {
mutate(value);
return <Stringify ref={ref} />;
}
mutate(value);
if (CONST_TRUE) {
return <Stringify ref={identity(ref)} />;
}
return value;
}
export const FIXTURE_ENTRYPOINT = {
fn: Foo,
params: [{}, {current: 'fake-ref-object'}],
};
```
## Error
```
Found 1 error:
Error: Cannot access refs during render
React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef)
error.repro-ref-mutable-range.ts:11:36
9 | mutate(value);
10 | if (CONST_TRUE) {
> 11 | return <Stringify ref={identity(ref)} />;
| ^^^ Passing a ref to a function may read its value during render
12 | }
13 | return value;
14 | }
```

View File

@@ -47,7 +47,7 @@ export const FIXTURE_ENTRYPOINT = {
## Logs
```
{"kind":"CompileError","fnLoc":{"start":{"line":6,"column":0,"index":158},"end":{"line":11,"column":1,"index":331},"filename":"mutate-after-useeffect-ref-access.ts"},"detail":{"options":{"severity":"InvalidReact","category":"This value cannot be modified","description":"Modifying component props or hook arguments is not allowed. Consider using a local variable instead.","details":[{"kind":"error","loc":{"start":{"line":9,"column":2,"index":289},"end":{"line":9,"column":16,"index":303},"filename":"mutate-after-useeffect-ref-access.ts"},"message":"value cannot be modified"}]}}}
{"kind":"CompileError","fnLoc":{"start":{"line":6,"column":0,"index":158},"end":{"line":11,"column":1,"index":331},"filename":"mutate-after-useeffect-ref-access.ts"},"detail":{"options":{"severity":"InvalidReact","category":"Cannot access refs during render","description":"React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef)","details":[{"kind":"error","loc":{"start":{"line":9,"column":2,"index":289},"end":{"line":9,"column":16,"index":303},"filename":"mutate-after-useeffect-ref-access.ts"},"message":"Cannot update ref during render"}]}}}
{"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":8,"column":2,"index":237},"end":{"line":8,"column":50,"index":285},"filename":"mutate-after-useeffect-ref-access.ts"},"decorations":[{"start":{"line":8,"column":24,"index":259},"end":{"line":8,"column":30,"index":265},"filename":"mutate-after-useeffect-ref-access.ts","identifierName":"arrRef"}]}
{"kind":"CompileSuccess","fnLoc":{"start":{"line":6,"column":0,"index":158},"end":{"line":11,"column":1,"index":331},"filename":"mutate-after-useeffect-ref-access.ts"},"fnName":"Component","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0}
```

View File

@@ -51,12 +51,12 @@ function RefsInEffects() {
const ref = useRefHelper();
const wrapped = useDeeperRefHelper();
let t0;
if ($[0] !== ref.current || $[1] !== wrapped.foo.current) {
if ($[0] !== ref || $[1] !== wrapped.foo.current) {
t0 = () => {
print(ref.current);
print(wrapped.foo.current);
};
$[0] = ref.current;
$[0] = ref;
$[1] = wrapped.foo.current;
$[2] = t0;
} else {

View File

@@ -47,7 +47,7 @@ export const FIXTURE_ENTRYPOINT = {
## Logs
```
{"kind":"CompileError","fnLoc":{"start":{"line":6,"column":0,"index":190},"end":{"line":11,"column":1,"index":363},"filename":"mutate-after-useeffect-ref-access.ts"},"detail":{"options":{"severity":"InvalidReact","category":"This value cannot be modified","description":"Modifying component props or hook arguments is not allowed. Consider using a local variable instead.","details":[{"kind":"error","loc":{"start":{"line":9,"column":2,"index":321},"end":{"line":9,"column":16,"index":335},"filename":"mutate-after-useeffect-ref-access.ts"},"message":"value cannot be modified"}]}}}
{"kind":"CompileError","fnLoc":{"start":{"line":6,"column":0,"index":190},"end":{"line":11,"column":1,"index":363},"filename":"mutate-after-useeffect-ref-access.ts"},"detail":{"options":{"severity":"InvalidReact","category":"Cannot access refs during render","description":"React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef)","details":[{"kind":"error","loc":{"start":{"line":9,"column":2,"index":321},"end":{"line":9,"column":16,"index":335},"filename":"mutate-after-useeffect-ref-access.ts"},"message":"Cannot update ref during render"}]}}}
{"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":8,"column":2,"index":269},"end":{"line":8,"column":50,"index":317},"filename":"mutate-after-useeffect-ref-access.ts"},"decorations":[{"start":{"line":8,"column":24,"index":291},"end":{"line":8,"column":30,"index":297},"filename":"mutate-after-useeffect-ref-access.ts","identifierName":"arrRef"}]}
{"kind":"CompileSuccess","fnLoc":{"start":{"line":6,"column":0,"index":190},"end":{"line":11,"column":1,"index":363},"filename":"mutate-after-useeffect-ref-access.ts"},"fnName":"Component","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0}
```

View File

@@ -1,39 +0,0 @@
## Input
```javascript
// @validatePreserveExistingMemoizationGuarantees
import {useCallback} from 'react';
function useHook(maybeRef) {
return useCallback(() => {
return [maybeRef.current];
}, [maybeRef]);
}
```
## Error
```
Found 1 error:
Memoization: Compilation skipped because existing memoization could not be preserved
React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected. The inferred dependency was `maybeRef.current`, but the source dependencies were [maybeRef]. Differences in ref.current access.
error.maybe-invalid-useCallback-read-maybeRef.ts:5:21
3 |
4 | function useHook(maybeRef) {
> 5 | return useCallback(() => {
| ^^^^^^^
> 6 | return [maybeRef.current];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 7 | }, [maybeRef]);
| ^^^^ Could not preserve existing manual memoization
8 | }
9 |
```

View File

@@ -1,39 +0,0 @@
## Input
```javascript
// @validatePreserveExistingMemoizationGuarantees
import {useMemo} from 'react';
function useHook(maybeRef, shouldRead) {
return useMemo(() => {
return () => [maybeRef.current];
}, [shouldRead, maybeRef]);
}
```
## Error
```
Found 1 error:
Memoization: Compilation skipped because existing memoization could not be preserved
React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected. The inferred dependency was `maybeRef.current`, but the source dependencies were [shouldRead, maybeRef]. Differences in ref.current access.
error.maybe-invalid-useMemo-read-maybeRef.ts:5:17
3 |
4 | function useHook(maybeRef, shouldRead) {
> 5 | return useMemo(() => {
| ^^^^^^^
> 6 | return () => [maybeRef.current];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 7 | }, [shouldRead, maybeRef]);
| ^^^^ Could not preserve existing manual memoization
8 | }
9 |
```

View File

@@ -0,0 +1,38 @@
## Input
```javascript
// @validatePreserveExistingMemoizationGuarantees
import {useCallback} from 'react';
function useHook(maybeRef) {
return useCallback(() => {
return [maybeRef.current];
}, [maybeRef]);
}
```
## Code
```javascript
import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees
import { useCallback } from "react";
function useHook(maybeRef) {
const $ = _c(2);
let t0;
if ($[0] !== maybeRef) {
t0 = () => [maybeRef.current];
$[0] = maybeRef;
$[1] = t0;
} else {
t0 = $[1];
}
return t0;
}
```
### Eval output
(kind: exception) Fixture not implemented

View File

@@ -0,0 +1,38 @@
## Input
```javascript
// @validatePreserveExistingMemoizationGuarantees
import {useMemo} from 'react';
function useHook(maybeRef, shouldRead) {
return useMemo(() => {
return () => [maybeRef.current];
}, [shouldRead, maybeRef]);
}
```
## Code
```javascript
import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees
import { useMemo } from "react";
function useHook(maybeRef, shouldRead) {
const $ = _c(2);
let t0;
if ($[0] !== maybeRef) {
t0 = () => [maybeRef.current];
$[0] = maybeRef;
$[1] = t0;
} else {
t0 = $[1];
}
return t0;
}
```
### Eval output
(kind: exception) Fixture not implemented

View File

@@ -0,0 +1,89 @@
## Input
```javascript
import {Stringify, identity, mutate, CONST_TRUE} from 'shared-runtime';
function Foo(props, ref) {
const value = {};
if (CONST_TRUE) {
mutate(value);
return <Stringify ref={ref} />;
}
mutate(value);
if (CONST_TRUE) {
return <Stringify ref={identity(ref)} />;
}
return value;
}
export const FIXTURE_ENTRYPOINT = {
fn: Foo,
params: [{}, {current: 'fake-ref-object'}],
};
```
## Code
```javascript
import { c as _c } from "react/compiler-runtime";
import { Stringify, identity, mutate, CONST_TRUE } from "shared-runtime";
function Foo(props, ref) {
const $ = _c(7);
let t0;
let value;
if ($[0] !== ref) {
t0 = Symbol.for("react.early_return_sentinel");
bb0: {
value = {};
if (CONST_TRUE) {
mutate(value);
t0 = <Stringify ref={ref} />;
break bb0;
}
mutate(value);
}
$[0] = ref;
$[1] = t0;
$[2] = value;
} else {
t0 = $[1];
value = $[2];
}
if (t0 !== Symbol.for("react.early_return_sentinel")) {
return t0;
}
if (CONST_TRUE) {
let t1;
if ($[3] !== ref) {
t1 = identity(ref);
$[3] = ref;
$[4] = t1;
} else {
t1 = $[4];
}
let t2;
if ($[5] !== t1) {
t2 = <Stringify ref={t1} />;
$[5] = t1;
$[6] = t2;
} else {
t2 = $[6];
}
return t2;
}
return value;
}
export const FIXTURE_ENTRYPOINT = {
fn: Foo,
params: [{}, { current: "fake-ref-object" }],
};
```
### Eval output
(kind: ok) <div>{"ref":{"current":"fake-ref-object"}}</div>