compiler: ValidateNoRefInRender detects writes of refs
Improves ValidateNoRefAccessInRender, detecting modifications of refs during render. Fixes #29161 ghstack-source-id: 99078b3cea5b2d9019dbf77ede9c2e4cd9fbfd27 Pull Request resolved: https://github.com/facebook/react/pull/29170
Joe Savona committed
May 29, 2024 at 07:56 UTC
867edc6576956f577718540c504a10bcfe2aad77
11 files changed
+132
-18
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccesInRender.ts
+49
-10
@@ -10,6 +10,7 @@ import {
10
HIRFunction,
11
IdentifierId,
12
Place,
13
+ SourceLocation,
14
isRefValueType,
15
isUseRefType,
16
} from "../HIR";
@@ -117,7 +118,12 @@ function validateNoRefAccessInRenderImpl(
118
case "MethodCall": {
119
if (!isEffectHook(instr.value.property.identifier)) {
120
for (const operand of eachInstructionValueOperand(instr.value)) {
120
- validateNoRefAccess(errors, refAccessingFunctions, operand);
121
+ validateNoRefAccess(
122
+ errors,
123
+ refAccessingFunctions,
124
+ operand,
125
+ operand.loc
126
+ );
127
}
128
}
129
break;
@@ -138,7 +144,12 @@ function validateNoRefAccessInRenderImpl(
144
});
145
}
146
for (const operand of eachInstructionValueOperand(instr.value)) {
141
- validateNoRefAccess(errors, refAccessingFunctions, operand);
147
+ validateNoRefAccess(
148
+ errors,
149
+ refAccessingFunctions,
150
+ operand,
151
+ operand.loc
152
+ );
153
}
154
}
155
break;
@@ -146,7 +157,30 @@ function validateNoRefAccessInRenderImpl(
157
case "ObjectExpression":
158
case "ArrayExpression": {
159
for (const operand of eachInstructionValueOperand(instr.value)) {
149
- validateNoRefAccess(errors, refAccessingFunctions, operand);
160
+ validateNoRefAccess(
161
+ errors,
162
+ refAccessingFunctions,
163
+ operand,
164
+ operand.loc
165
+ );
166
+ }
167
+ break;
168
+ }
169
+ case "PropertyDelete":
170
+ case "PropertyStore":
171
+ case "ComputedDelete":
172
+ case "ComputedStore": {
173
+ validateNoRefAccess(
174
+ errors,
175
+ refAccessingFunctions,
176
+ instr.value.object,
177
+ instr.loc
178
+ );
179
+ for (const operand of eachInstructionValueOperand(instr.value)) {
180
+ if (operand === instr.value.object) {
181
+ continue;
182
+ }
183
+ validateNoRefValueAccess(errors, refAccessingFunctions, operand);
184
}
185
break;
186
}
@@ -172,12 +206,12 @@ function validateNoRefAccessInRenderImpl(
206
207
function validateNoRefValueAccess(
208
errors: CompilerError,
175
- unconditionalSetStateFunctions: Set<IdentifierId>,
209
+ refAccessingFunctions: Set<IdentifierId>,
210
operand: Place
211
): void {
212
if (
213
isRefValueType(operand.identifier) ||
180
- unconditionalSetStateFunctions.has(operand.identifier.id)
214
+ refAccessingFunctions.has(operand.identifier.id)
215
) {
216
errors.push({
217
severity: ErrorSeverity.InvalidReact,
@@ -192,20 +226,25 @@ function validateNoRefValueAccess(
226
227
function validateNoRefAccess(
228
errors: CompilerError,
195
- unconditionalSetStateFunctions: Set<IdentifierId>,
196
- operand: Place
229
+ refAccessingFunctions: Set<IdentifierId>,
230
+ operand: Place,
231
+ loc: SourceLocation
232
): void {
233
if (
234
isRefValueType(operand.identifier) ||
235
isUseRefType(operand.identifier) ||
201
- unconditionalSetStateFunctions.has(operand.identifier.id)
236
+ refAccessingFunctions.has(operand.identifier.id)
237
) {
238
errors.push({
239
severity: ErrorSeverity.InvalidReact,
240
reason:
241
"Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)",
207
- loc: operand.loc,
208
- description: `Cannot access ref value at ${printPlace(operand)}`,
242
+ loc: loc,
243
+ description:
244
+ operand.identifier.name !== null &&
245
+ operand.identifier.name.kind === "named"
246
+ ? `Cannot access ref value \`${operand.identifier.name.value}\``
247
+ : null,
248
suggestions: null,
249
});
250
}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-aliased-ref-in-callback-invoked-during-render-.expect.md
+1
-1
@@ -22,7 +22,7 @@ function Component(props) {
22
7 | return <Foo item={item} current={current} />;
23
8 | };
24
> 9 | return <Items>{props.items.map((item) => renderItem(item))}</Items>;
25
- | ^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at mutate? $64[13:15]:TObject<BuiltInFunction> (9:9)
25
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (9:9)
26
10 | }
27
11 |
28
```
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-pass-ref-to-function.expect.md
+1
-1
@@ -18,7 +18,7 @@ function Component(props) {
18
2 | function Component(props) {
19
3 | const ref = useRef(null);
20
> 4 | const x = foo(ref);
21
- | ^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at mutate? $21[6:8]:TObject<BuiltInUseRefId> (4:4)
21
+ | ^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (4:4)
22
5 | return x.current;
23
6 | }
24
7 |
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-ref-in-callback-invoked-during-render.expect.md
+1
-1
@@ -21,7 +21,7 @@ function Component(props) {
21
6 | return <Foo item={item} current={current} />;
22
7 | };
23
> 8 | return <Items>{props.items.map((item) => renderItem(item))}</Items>;
24
- | ^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at mutate? $60[14:16]:TObject<BuiltInFunction> (8:8)
24
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (8:8)
25
9 | }
26
10 |
27
```
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-set-and-read-ref-during-render.expect.md
+6
-3
@@ -15,10 +15,13 @@ function Component(props) {
15
## Error
16
17
```
18
+ 2 | function Component(props) {
19
3 | const ref = useRef(null);
19
- 4 | ref.current = props.value;
20
-> 5 | return ref.current;
21
- | ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at freeze $24:TObject<BuiltInRefValue> (5:5)
20
+> 4 | ref.current = props.value;
21
+ | ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (4:4)
22
+
23
+InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at freeze $24:TObject<BuiltInRefValue> (5:5)
24
+ 5 | return ref.current;
25
6 | }
26
7 |
27
```
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-set-and-read-ref-nested-property-during-render.expect.md
new
+29
@@ -0,0 +1,29 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// @validateRefAccessDuringRender
6
+function Component(props) {
7
+ const ref = useRef({ inner: null });
8
+ ref.current.inner = props.value;
9
+ return ref.current.inner;
10
+}
11
+
12
+```
13
+
14
+
15
+## Error
16
+
17
+```
18
+ 2 | function Component(props) {
19
+ 3 | const ref = useRef({ inner: null });
20
+> 4 | ref.current.inner = props.value;
21
+ | ^^^^^^^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (4:4)
22
+
23
+InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at freeze $30:TObject<BuiltInRefValue> (5:5)
24
+ 5 | return ref.current.inner;
25
+ 6 | }
26
+ 7 |
27
+```
28
+
29
+
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-set-and-read-ref-nested-property-during-render.js
new
+6
@@ -0,0 +1,6 @@
1
+// @validateRefAccessDuringRender
2
+function Component(props) {
3
+ const ref = useRef({ inner: null });
4
+ ref.current.inner = props.value;
5
+ return ref.current.inner;
6
+}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-ref-added-to-dep-without-type-info.expect.md
+1
-1
@@ -25,7 +25,7 @@ function Foo({ a }) {
25
3 | const ref = useRef();
26
4 | // type information is lost here as we don't track types of fields
27
> 5 | const val = { ref };
28
- | ^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at capture $29:TObject<BuiltInUseRefId> (5:5)
28
+ | ^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (5:5)
29
6 | // without type info, we don't know that val.ref.current is a ref value so we
30
7 | // *would* end up depending on val.ref.current
31
8 | // however, this is an instance of accessing a ref during render and is disallowed
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-write-but-dont-read-ref-in-render.expect.md
new
+29
@@ -0,0 +1,29 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// @validateRefAccessDuringRender
6
+function useHook({ value }) {
7
+ const ref = useRef(null);
8
+ // Writing to a ref in render is against the rules:
9
+ ref.current = value;
10
+ // returning a ref is allowed, so this alone doesn't trigger an error:
11
+ return ref;
12
+}
13
+
14
+```
15
+
16
+
17
+## Error
18
+
19
+```
20
+ 3 | const ref = useRef(null);
21
+ 4 | // Writing to a ref in render is against the rules:
22
+> 5 | ref.current = value;
23
+ | ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (5:5)
24
+ 6 | // returning a ref is allowed, so this alone doesn't trigger an error:
25
+ 7 | return ref;
26
+ 8 | }
27
+```
28
+
29
+
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-write-but-dont-read-ref-in-render.js
new
+8
@@ -0,0 +1,8 @@
1
+// @validateRefAccessDuringRender
2
+function useHook({ value }) {
3
+ const ref = useRef(null);
4
+ // Writing to a ref in render is against the rules:
5
+ ref.current = value;
6
+ // returning a ref is allowed, so this alone doesn't trigger an error:
7
+ return ref;
8
+}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.validate-mutate-ref-arg-in-render.expect.md
+1
-1
@@ -23,7 +23,7 @@ export const FIXTURE_ENTRYPOINT = {
23
1 | // @validateRefAccessDuringRender:true
24
2 | function Foo(props, ref) {
25
> 3 | console.log(ref.current);
26
- | ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at read $16:TObject<BuiltInRefValue> (3:3)
26
+ | ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (3:3)
27
4 | return <div>{props.bar}</div>;
28
5 | }
29
6 |