Facebook/react
1
0
Fork 0
mirror of https://github.com/facebook/react.git synced 2025-11-01 09:12:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

[compiler] Alternate take on ref validation

Browse Source 
Some of the false positives we've seen have to do with the need to align our ref validation with our understanding of which functions may be called during render. The new mutability/aliasing model makes this much more explicit, with the ability to create Impure effects which we then throw as errors if they are reachable during render. This means we can now revisit ref validation by just emitting impure effects.

That's what this new pass does. It's a bit simpler: it implements the check for `ref.current == null` guarded if blocks. Otherwise it disallows access to `ref.current` specifically. Unlike before, we intentionally allow passing ref objects to functions — we just see a lot of many false positives on disallowing things like `children({ref})` or similar.

Open to feedback! This is also still WIP.
This commit is contained in:
Joe Savona
2025-06-23 17:38:24 -07:00
parent b846dfa6a6
commit 629c19cffe
14 changed files with 179 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts
+1 -1
View File
@@ -285,7 +285,7 @@ function runWithEnvironment(
}
if (env.config.validateRefAccessDuringRender) {
validateNoRefAccessInRender(hir).unwrap();
// validateNoRefAccessInRender(hir).unwrap();
}
if (env.config.validateNoSetStateInRender) {
compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts
+129
View File
@@ -28,7 +28,9 @@ import {
isMapType,
isPrimitiveType,
isRefOrRefValue,
isRefValueType,
isSetType,
isUseRefType,
makeIdentifierId,
Phi,
Place,
@@ -219,6 +221,9 @@ export function inferMutationAliasingEffects(
}
}
}
if (fn.env.config.validateRefAccessDuringRender) {
inferRefAccessEffects(fn, isFunctionExpression);
}
return Ok(undefined);
}
@@ -2513,3 +2518,127 @@ export type AbstractValue = {
kind: ValueKind;
reason: ReadonlySet<ValueReason>;
};
function inferRefAccessEffects(
fn: HIRFunction,
_isFunctionExpression: boolean,
): void {
const nullish = new Set<IdentifierId>();
const nullishTest = new Map<IdentifierId, Place>();
let guard: {ref: IdentifierId; fallthrough: BlockId} | null = null;
const temporaries: Map<IdentifierId, Place> = new Map();
function visitOperand(operand: Place): AliasingEffect | null {
const nullTestRef = nullishTest.get(operand.identifier.id);
if (isRefValueType(operand.identifier) || nullTestRef != null) {
const refOperand = nullTestRef ?? operand;
return {
kind: 'Impure',
error: {
severity: ErrorSeverity.InvalidReact,
reason:
'Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)',
loc: refOperand.loc,
description:
refOperand.identifier.name !== null &&
refOperand.identifier.name.kind === 'named'
? `Cannot access ref value \`${refOperand.identifier.name.value}\``
: null,
suggestions: null,
},
place: refOperand,
};
}
return null;
}
for (const block of fn.body.blocks.values()) {
if (guard !== null && guard.fallthrough === block.id) {
guard = null;
}
for (const instr of block.instructions) {
const {lvalue, value} = instr;
if (value.kind === 'LoadLocal' && isUseRefType(value.place.identifier)) {
temporaries.set(lvalue.identifier.id, value.place);
} else if (
value.kind === 'StoreLocal' &&
isUseRefType(value.value.identifier)
) {
temporaries.set(value.lvalue.place.identifier.id, value.value);
temporaries.set(lvalue.identifier.id, value.value);
} else if (
value.kind === 'BinaryExpression' &&
((isRefValueType(value.left.identifier) &&
nullish.has(value.right.identifier.id)) ||
(nullish.has(value.left.identifier.id) &&
isRefValueType(value.right.identifier)))
) {
const refOperand = isRefValueType(value.left.identifier)
? value.left
: value.right;
const operand = temporaries.get(refOperand.identifier.id) ?? refOperand;
nullishTest.set(lvalue.identifier.id, operand);
} else if (value.kind === 'Primitive' && value.value == null) {
nullish.add(lvalue.identifier.id);
} else if (
value.kind === 'PropertyLoad' &&
isUseRefType(value.object.identifier) &&
value.property === 'current'
) {
const refOperand =
temporaries.get(value.object.identifier.id) ?? value.object;
temporaries.set(lvalue.identifier.id, refOperand);
} else if (
value.kind === 'PropertyStore' &&
value.property === 'current' &&
isUseRefType(value.object.identifier)
) {
const refOperand =
temporaries.get(value.object.identifier.id) ?? value.object;
if (guard != null && refOperand.identifier.id === guard.ref) {
// Allow a single write within the guard
guard = null;
} else {
instr.effects ??= [];
instr.effects.push({
kind: 'Impure',
error: {
severity: ErrorSeverity.InvalidReact,
reason:
'Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)',
loc: value.loc,
description:
value.object.identifier.name !== null &&
value.object.identifier.name.kind === 'named'
? `Cannot access ref value \`${value.object.identifier.name.value}\``
: null,
suggestions: null,
},
place: value.object,
});
}
} else {
for (const operand of eachInstructionValueOperand(value)) {
const error = visitOperand(operand);
if (error) {
instr.effects ??= [];
instr.effects.push(error);
}
}
}
}
if (
guard == null &&
block.terminal.kind === 'if' &&
nullishTest.has(block.terminal.test.identifier.id)
) {
const ref = nullishTest.get(block.terminal.test.identifier.id)!;
guard = {ref: ref.identifier.id, fallthrough: block.terminal.fallthrough};
} else {
for (const operand of eachTerminalOperand(block.terminal)) {
const _effect = visitOperand(operand);
// TODO: need a place to store terminal effects generically
}
}
}
}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hook-ref-value.expect.md
-2
View File
@@ -24,8 +24,6 @@ export const FIXTURE_ENTRYPOINT = {
4 | const ref = useRef();
> 5 | useEffect(() => {}, [ref.current]);
| ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (5:5)
InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (5:5)
6 | }
7 |
8 | export const FIXTURE_ENTRYPOINT = {
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-access-ref-during-render.expect.md
+2
View File
@@ -19,6 +19,8 @@ function Component(props) {
3 | const ref = useRef(null);
> 4 | const value = ref.current;
| ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (4:4)
InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value `value` (5:5)
5 | return value;
6 | }
7 |
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-aliased-ref-in-callback-invoked-during-render-.expect.md
+11 -6
View File
@@ -19,12 +19,17 @@ function Component(props) {
## Error
```
7 | return <Foo item={item} current={current} />;
8 | };
> 9 | return <Items>{props.items.map(item => renderItem(item))}</Items>;
| ^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (9:9)
10 | }
11 |
4 | const renderItem = item => {
5 | const aliasedRef = ref;
> 6 | const current = aliasedRef.current;
| ^^^^^^^^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (6:6)
InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value `current` (7:7)
InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (7:7)
7 | return <Foo item={item} current={current} />;
8 | };
9 | return <Items>{props.items.map(item => renderItem(item))}</Items>;
```
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-disallow-mutating-refs-in-render-transitive.expect.md
+7 -9
View File
@@ -21,15 +21,13 @@ function Component() {
## Error
```
7 | };
8 | const changeRef = setRef;
> 9 | changeRef();
| ^^^^^^^^^ InvalidReact: This function accesses a ref value (the `current` property), which may not be accessed during render. (https://react.dev/reference/react/useRef) (9:9)
InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (9:9)
10 |
11 | return <button ref={ref} />;
12 | }
4 |
5 | const setRef = () => {
> 6 | ref.current = false;
| ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (6:6)
7 | };
8 | const changeRef = setRef;
9 | changeRef();
```
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-read-ref-prop-in-render-destructure.expect.md
+4
View File
@@ -18,6 +18,10 @@ function Component({ref}) {
2 | function Component({ref}) {
> 3 | const value = ref.current;
| ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (3:3)
InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value `value` (4:4)
InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (4:4)
4 | return <div>{value}</div>;
5 | }
6 |
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-read-ref-prop-in-render-property-load.expect.md
+4
View File
@@ -18,6 +18,10 @@ function Component(props) {
2 | function Component(props) {
> 3 | const value = props.ref.current;
| ^^^^^^^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (3:3)
InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value `value` (4:4)
InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (4:4)
4 | return <div>{value}</div>;
5 | }
6 |
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-ref-in-callback-invoked-during-render.expect.md
+11 -6
View File
@@ -18,12 +18,17 @@ function Component(props) {
## Error
```
6 | return <Foo item={item} current={current} />;
7 | };
> 8 | return <Items>{props.items.map(item => renderItem(item))}</Items>;
| ^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (8:8)
9 | }
10 |
3 | const ref = useRef(null);
4 | const renderItem = item => {
> 5 | const current = ref.current;
| ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (5:5)
InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value `current` (6:6)
InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (6:6)
6 | return <Foo item={item} current={current} />;
7 | };
8 | return <Items>{props.items.map(item => renderItem(item))}</Items>;
```
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-set-and-read-ref-during-render.expect.md
-2
View File
@@ -19,8 +19,6 @@ function Component(props) {
3 | const ref = useRef(null);
> 4 | ref.current = props.value;
| ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (4:4)
InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (5:5)
5 | return ref.current;
6 | }
7 |
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-initialization-nonif.expect.md
+2 -2
View File
@@ -27,9 +27,9 @@ export const FIXTURE_ENTRYPOINT = {
4 | component C() {
5 | const r = useRef(null);
> 6 | const guard = r.current == null;
| ^^^^^^^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (6:6)
| ^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value `r` (6:6)
InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value `guard` (7:7)
InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (8:8)
7 | if (guard) {
8 | r.current = 1;
9 | }
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md
+5 -7
View File
@@ -34,15 +34,13 @@ export const FIXTURE_ENTRYPOINT = {
## Error
```
15 | ref.current.inner = null;
13 | // The ref is modified later, extending its range and preventing memoization of onChange
14 | const reset = () => {
> 15 | ref.current.inner = null;
| ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (15:15)
16 | };
> 17 | reset();
| ^^^^^ InvalidReact: This function accesses a ref value (the `current` property), which may not be accessed during render. (https://react.dev/reference/react/useRef) (17:17)
InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (17:17)
17 | reset();
18 |
19 | return <input onChange={onChange} />;
20 | }
```
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-current-aliased-no-added-to-dep.expect.md
+2 -2
View File
@@ -2,7 +2,7 @@
## Input
```javascript
// @validateRefAccessDuringRender false
// @validateRefAccessDuringRender:false
function VideoTab() {
const ref = useRef();
const t = ref.current;
@@ -18,7 +18,7 @@ function VideoTab() {
## Code
```javascript
import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRender false
import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRender:false
function VideoTab() {
const $ = _c(1);
const ref = useRef();
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-current-aliased-no-added-to-dep.js
+1 -1
View File
@@ -1,4 +1,4 @@
// @validateRefAccessDuringRender false
// @validateRefAccessDuringRender:false
function VideoTab() {
const ref = useRef();
const t = ref.current;