@samitouri / QOS-React / commits / 21a95239e1

[compiler] Allow functions containing refs to be returned

Summary: We previously were excessively strict about preventing functions that access refs from being returned--doing so is potentially valid for hooks, because the return value may only be used in an event or effect. ghstack-source-id: cfa8bb1b54e8eb365f2de50d051bd09e09162d7b Pull Request resolved: https://github.com/facebook/react/pull/30724

Mike Vitousek committed Aug 16, 2024 at 13:27 UTC 21a95239e15cf62e4bf3922af998383b844df2e3
4 files changed +90 -57
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccesInRender.ts
+33 -20
@@ -59,20 +59,7 @@ function validateNoRefAccessInRenderImpl(
59 case 'JsxExpression':
60 case 'JsxFragment': {
61 for (const operand of eachInstructionValueOperand(instr.value)) {
62 - if (isRefValueType(operand.identifier)) {
63 - errors.push({
64 - severity: ErrorSeverity.InvalidReact,
65 - reason:
66 - 'Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)',
67 - loc: lookupLocations.get(operand.identifier.id) ?? operand.loc,
68 - description:
69 - operand.identifier.name !== null &&
70 - operand.identifier.name.kind === 'named'
71 - ? `Cannot access ref value \`${operand.identifier.name.value}\``
72 - : null,
73 - suggestions: null,
74 - });
75 - }
62 + validateNoDirectRefValueAccess(errors, operand, lookupLocations);
63 }
64 break;
65 }
@@ -232,12 +219,17 @@ function validateNoRefAccessInRenderImpl(
219 }
220 }
221 for (const operand of eachTerminalOperand(block.terminal)) {
235 - validateNoRefValueAccess(
236 - errors,
237 - refAccessingFunctions,
238 - lookupLocations,
239 - operand,
240 - );
222 + if (block.terminal.kind !== 'return') {
223 + validateNoRefValueAccess(
224 + errors,
225 + refAccessingFunctions,
226 + lookupLocations,
227 + operand,
228 + );
229 + } else {
230 + // Allow functions containing refs to be returned, but not direct ref values
231 + validateNoDirectRefValueAccess(errors, operand, lookupLocations);
232 + }
233 }
234 }
235
@@ -297,3 +289,24 @@ function validateNoRefAccess(
289 });
290 }
291 }
292 +
293 +function validateNoDirectRefValueAccess(
294 + errors: CompilerError,
295 + operand: Place,
296 + lookupLocations: Map<IdentifierId, SourceLocation>,
297 +): void {
298 + if (isRefValueType(operand.identifier)) {
299 + errors.push({
300 + severity: ErrorSeverity.InvalidReact,
301 + reason:
302 + 'Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)',
303 + loc: lookupLocations.get(operand.identifier.id) ?? operand.loc,
304 + description:
305 + operand.identifier.name !== null &&
306 + operand.identifier.name.kind === 'named'
307 + ? `Cannot access ref value \`${operand.identifier.name.value}\``
308 + : null,
309 + suggestions: null,
310 + });
311 + }
312 +}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.return-ref-callback.expect.md deleted
-37
@@ -1,37 +0,0 @@
1 -
2 -## Input
3 -
4 -```javascript
5 -// @flow @validateRefAccessDuringRender @validatePreserveExistingMemoizationGuarantees
6 -
7 -component Foo() {
8 - const ref = useRef();
9 -
10 - const s = () => {
11 - return ref.current;
12 - };
13 -
14 - return s;
15 -}
16 -
17 -export const FIXTURE_ENTRYPOINT = {
18 - fn: Foo,
19 - params: [],
20 -};
21 -
22 -```
23 -
24 -
25 -## Error
26 -
27 -```
28 - 8 | };
29 - 9 |
30 -> 10 | return s;
31 - | ^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (10:10)
32 - 11 | }
33 - 12 |
34 - 13 | export const FIXTURE_ENTRYPOINT = {
35 -```
36 -
37 -
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/return-ref-callback.expect.md new
+55
@@ -0,0 +1,55 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @flow @validateRefAccessDuringRender @validatePreserveExistingMemoizationGuarantees
6 +
7 +import {useRef} from 'react';
8 +
9 +component Foo() {
10 + const ref = useRef();
11 +
12 + const s = () => {
13 + return ref.current;
14 + };
15 +
16 + return s;
17 +}
18 +
19 +export const FIXTURE_ENTRYPOINT = {
20 + fn: Foo,
21 + params: [],
22 +};
23 +
24 +```
25 +
26 +## Code
27 +
28 +```javascript
29 +import { c as _c } from "react/compiler-runtime";
30 +
31 +import { useRef } from "react";
32 +
33 +function Foo() {
34 + const $ = _c(1);
35 + const ref = useRef();
36 + let t0;
37 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
38 + t0 = () => ref.current;
39 + $[0] = t0;
40 + } else {
41 + t0 = $[0];
42 + }
43 + const s = t0;
44 + return s;
45 +}
46 +
47 +export const FIXTURE_ENTRYPOINT = {
48 + fn: Foo,
49 + params: [],
50 +};
51 +
52 +```
53 +
54 +### Eval output
55 +(kind: ok) "[[ function params=0 ]]"
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/return-ref-callback.js renamed
+2
@@ -1,5 +1,7 @@
1 // @flow @validateRefAccessDuringRender @validatePreserveExistingMemoizationGuarantees
2
3 +import {useRef} from 'react';
4 +
5 component Foo() {
6 const ref = useRef();
7