Skip to content

Commit c47d815

Browse files
committed
[compiler] Allow assigning ref-accessing functions to objects if not mutated
Allows assigning a ref-accessing function to an object so long as that object is not subsequently transitively mutated. We should likely rewrite the ref validation to use the new mutation/aliasing effects, which would provide a more consistent behavior across instruction types and require fewer special cases like this.
1 parent e6eddf6 commit c47d815

5 files changed

+131
-9
lines changed

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -524,11 +524,25 @@ function validateNoRefAccessInRenderImpl(
524524
} else {
525525
validateNoRefUpdate(errors, env, instr.value.object, instr.loc);
526526
}
527-
for (const operand of eachInstructionValueOperand(instr.value)) {
528-
if (operand === instr.value.object) {
529-
continue;
527+
if (
528+
instr.value.kind === 'ComputedDelete' ||
529+
instr.value.kind === 'ComputedStore'
530+
) {
531+
validateNoRefValueAccess(errors, env, instr.value.property);
532+
}
533+
if (
534+
instr.value.kind === 'ComputedStore' ||
535+
instr.value.kind === 'PropertyStore'
536+
) {
537+
validateNoDirectRefValueAccess(errors, instr.value.value, env);
538+
const type = env.get(instr.value.value.identifier.id);
539+
if (type != null && type.kind === 'Structure') {
540+
let objectType: RefAccessType = type;
541+
if (target != null) {
542+
objectType = joinRefAccessTypes(objectType, target);
543+
}
544+
env.set(instr.value.object.identifier.id, objectType);
530545
}
531-
validateNoRefValueAccess(errors, env, operand);
532546
}
533547
break;
534548
}
@@ -730,11 +744,7 @@ function validateNoRefUpdate(
730744
loc: SourceLocation,
731745
): void {
732746
const type = destructure(env.get(operand.identifier.id));
733-
if (
734-
type?.kind === 'Ref' ||
735-
type?.kind === 'RefValue' ||
736-
(type?.kind === 'Structure' && type.fn?.readRefEffect)
737-
) {
747+
if (type?.kind === 'Ref' || type?.kind === 'RefValue') {
738748
errors.pushDiagnostic(
739749
CompilerDiagnostic.create({
740750
severity: ErrorSeverity.InvalidReact,
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
2+
## Input
3+
4+
```javascript
5+
import {useRef} from 'react';
6+
import {Stringify} from 'shared-runtime';
7+
8+
function Component(props) {
9+
const ref = useRef(props.value);
10+
const object = {};
11+
object.foo = () => ref.current;
12+
return <Stringify object={object} shouldInvokeFns={true} />;
13+
}
14+
15+
export const FIXTURE_ENTRYPOINT = {
16+
fn: Component,
17+
params: [{value: 42}],
18+
};
19+
20+
```
21+
22+
## Code
23+
24+
```javascript
25+
import { c as _c } from "react/compiler-runtime";
26+
import { useRef } from "react";
27+
import { Stringify } from "shared-runtime";
28+
29+
function Component(props) {
30+
const $ = _c(1);
31+
const ref = useRef(props.value);
32+
let t0;
33+
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
34+
const object = {};
35+
object.foo = () => ref.current;
36+
t0 = <Stringify object={object} shouldInvokeFns={true} />;
37+
$[0] = t0;
38+
} else {
39+
t0 = $[0];
40+
}
41+
return t0;
42+
}
43+
44+
export const FIXTURE_ENTRYPOINT = {
45+
fn: Component,
46+
params: [{ value: 42 }],
47+
};
48+
49+
```
50+
51+
### Eval output
52+
(kind: ok) <div>{"object":{"foo":{"kind":"Function","result":42}},"shouldInvokeFns":true}</div>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import {useRef} from 'react';
2+
import {Stringify} from 'shared-runtime';
3+
4+
function Component(props) {
5+
const ref = useRef(props.value);
6+
const object = {};
7+
object.foo = () => ref.current;
8+
return <Stringify object={object} shouldInvokeFns={true} />;
9+
}
10+
11+
export const FIXTURE_ENTRYPOINT = {
12+
fn: Component,
13+
params: [{value: 42}],
14+
};
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
2+
## Input
3+
4+
```javascript
5+
import {useRef} from 'react';
6+
7+
function Component() {
8+
const ref = useRef(null);
9+
const object = {};
10+
object.foo = () => ref.current;
11+
const refValue = object.foo();
12+
return <div>{refValue}</div>;
13+
}
14+
15+
```
16+
17+
18+
## Error
19+
20+
```
21+
Found 1 error:
22+
23+
Error: Cannot access refs during render
24+
25+
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)
26+
27+
error.invalid-access-ref-in-render-mutate-object-with-ref-function.ts:6:2
28+
4 | const ref = useRef(null);
29+
5 | const object = {};
30+
> 6 | object.foo = () => ref.current;
31+
| ^^^^^^^^^^ Cannot update ref during render
32+
7 | const refValue = object.foo();
33+
8 | return <div>{refValue}</div>;
34+
9 | }
35+
```
36+
37+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import {useRef} from 'react';
2+
3+
function Component() {
4+
const ref = useRef(null);
5+
const object = {};
6+
object.foo = () => ref.current;
7+
const refValue = object.foo();
8+
return <div>{refValue}</div>;
9+
}

0 commit comments

Comments
 (0)