Skip to content

Commit bd9e6e0

Browse files
authored
[compiler] More flexible/helpful lazy ref initialization (#34449)
Two small QoL improvements inspired by feedback: * `if (ref.current === undefined) { ref.current = ... }` is now allowed. * `if (!ref.current) { ref.current = ... }` is still disallowed, but we emit an extra hint suggesting the `if (!ref.current == null)` pattern. I was on the fence about the latter. We got feedback asking to allow `if (!ref.current)` but if your ref stores a boolean value then this would allow reading the ref in render. The unary form is also less precise in general due to sketchy truthiness conversions. I figured a hint is a good compromise. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34449). * __->__ #34449 * #34424
1 parent 835b009 commit bd9e6e0

File tree

7 files changed

+247
-0
lines changed

7 files changed

+247
-0
lines changed

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -639,12 +639,55 @@ function validateNoRefAccessInRenderImpl(
639639
case 'StartMemoize':
640640
case 'FinishMemoize':
641641
break;
642+
case 'LoadGlobal': {
643+
if (instr.value.binding.name === 'undefined') {
644+
env.set(instr.lvalue.identifier.id, {kind: 'Nullable'});
645+
}
646+
break;
647+
}
642648
case 'Primitive': {
643649
if (instr.value.value == null) {
644650
env.set(instr.lvalue.identifier.id, {kind: 'Nullable'});
645651
}
646652
break;
647653
}
654+
case 'UnaryExpression': {
655+
if (instr.value.operator === '!') {
656+
const value = env.get(instr.value.value.identifier.id);
657+
const refId =
658+
value?.kind === 'RefValue' && value.refId != null
659+
? value.refId
660+
: null;
661+
if (refId !== null) {
662+
/*
663+
* Record an error suggesting the `if (ref.current == null)` pattern,
664+
* but also record the lvalue as a guard so that we don't emit a second
665+
* error for the write to the ref
666+
*/
667+
env.set(instr.lvalue.identifier.id, {kind: 'Guard', refId});
668+
errors.pushDiagnostic(
669+
CompilerDiagnostic.create({
670+
category: ErrorCategory.Refs,
671+
reason: 'Cannot access refs during render',
672+
description: ERROR_DESCRIPTION,
673+
})
674+
.withDetails({
675+
kind: 'error',
676+
loc: instr.value.value.loc,
677+
message: `Cannot access ref value during render`,
678+
})
679+
.withDetails({
680+
kind: 'hint',
681+
message:
682+
'To initialize a ref only once, check that the ref is null with the pattern `if (ref.current == null) { ref.current = ... }`',
683+
}),
684+
);
685+
break;
686+
}
687+
}
688+
validateNoRefValueAccess(errors, env, instr.value.value);
689+
break;
690+
}
648691
case 'BinaryExpression': {
649692
const left = env.get(instr.value.left.identifier.id);
650693
const right = env.get(instr.value.right.identifier.id);
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
2+
## Input
3+
4+
```javascript
5+
//@flow
6+
import {useRef} from 'react';
7+
8+
component C() {
9+
const r = useRef(null);
10+
if (r.current == undefined) {
11+
r.current = 1;
12+
}
13+
}
14+
15+
export const FIXTURE_ENTRYPOINT = {
16+
fn: C,
17+
params: [{}],
18+
};
19+
20+
```
21+
22+
## Code
23+
24+
```javascript
25+
import { useRef } from "react";
26+
27+
function C() {
28+
const r = useRef(null);
29+
if (r.current == undefined) {
30+
r.current = 1;
31+
}
32+
}
33+
34+
export const FIXTURE_ENTRYPOINT = {
35+
fn: C,
36+
params: [{}],
37+
};
38+
39+
```
40+
41+
### Eval output
42+
(kind: ok)
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
//@flow
2+
import {useRef} from 'react';
3+
4+
component C() {
5+
const r = useRef(null);
6+
if (r.current == undefined) {
7+
r.current = 1;
8+
}
9+
}
10+
11+
export const FIXTURE_ENTRYPOINT = {
12+
fn: C,
13+
params: [{}],
14+
};
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
2+
## Input
3+
4+
```javascript
5+
//@flow
6+
import {useRef} from 'react';
7+
8+
component C() {
9+
const r = useRef(null);
10+
const current = !r.current;
11+
return <div>{current}</div>;
12+
}
13+
14+
export const FIXTURE_ENTRYPOINT = {
15+
fn: C,
16+
params: [{}],
17+
};
18+
19+
```
20+
21+
22+
## Error
23+
24+
```
25+
Found 4 errors:
26+
27+
Error: Cannot access refs during render
28+
29+
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).
30+
31+
4 | component C() {
32+
5 | const r = useRef(null);
33+
> 6 | const current = !r.current;
34+
| ^^^^^^^^^ Cannot access ref value during render
35+
7 | return <div>{current}</div>;
36+
8 | }
37+
9 |
38+
39+
To initialize a ref only once, check that the ref is null with the pattern `if (ref.current == null) { ref.current = ... }`
40+
41+
Error: Cannot access refs during render
42+
43+
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).
44+
45+
4 | component C() {
46+
5 | const r = useRef(null);
47+
> 6 | const current = !r.current;
48+
| ^^^^^^^^^^ Cannot access ref value during render
49+
7 | return <div>{current}</div>;
50+
8 | }
51+
9 |
52+
53+
Error: Cannot access refs during render
54+
55+
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).
56+
57+
5 | const r = useRef(null);
58+
6 | const current = !r.current;
59+
> 7 | return <div>{current}</div>;
60+
| ^^^^^^^ Cannot access ref value during render
61+
8 | }
62+
9 |
63+
10 | export const FIXTURE_ENTRYPOINT = {
64+
65+
Error: Cannot access refs during render
66+
67+
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).
68+
69+
5 | const r = useRef(null);
70+
6 | const current = !r.current;
71+
> 7 | return <div>{current}</div>;
72+
| ^^^^^^^ Cannot access ref value during render
73+
8 | }
74+
9 |
75+
10 | export const FIXTURE_ENTRYPOINT = {
76+
```
77+
78+
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
//@flow
2+
import {useRef} from 'react';
3+
4+
component C() {
5+
const r = useRef(null);
6+
const current = !r.current;
7+
return <div>{current}</div>;
8+
}
9+
10+
export const FIXTURE_ENTRYPOINT = {
11+
fn: C,
12+
params: [{}],
13+
};
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
2+
## Input
3+
4+
```javascript
5+
//@flow
6+
import {useRef} from 'react';
7+
8+
component C() {
9+
const r = useRef(null);
10+
if (!r.current) {
11+
r.current = 1;
12+
}
13+
}
14+
15+
export const FIXTURE_ENTRYPOINT = {
16+
fn: C,
17+
params: [{}],
18+
};
19+
20+
```
21+
22+
23+
## Error
24+
25+
```
26+
Found 1 error:
27+
28+
Error: Cannot access refs during render
29+
30+
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).
31+
32+
4 | component C() {
33+
5 | const r = useRef(null);
34+
> 6 | if (!r.current) {
35+
| ^^^^^^^^^ Cannot access ref value during render
36+
7 | r.current = 1;
37+
8 | }
38+
9 | }
39+
40+
To initialize a ref only once, check that the ref is null with the pattern `if (ref.current == null) { ref.current = ... }`
41+
```
42+
43+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
//@flow
2+
import {useRef} from 'react';
3+
4+
component C() {
5+
const r = useRef(null);
6+
if (!r.current) {
7+
r.current = 1;
8+
}
9+
}
10+
11+
export const FIXTURE_ENTRYPOINT = {
12+
fn: C,
13+
params: [{}],
14+
};

0 commit comments

Comments
 (0)