@samitouri / QOS-React / commits / d4e24b349e

[autodeps] Do not include nonreactive refs or setStates in inferred deps (#32236)

Jordan Brown committed Mar 3, 2025 at 15:26 UTC d4e24b349e6530a8e6c95d79ad40b32f93b47070
11 files changed +301 -3
compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts
+12 -1
@@ -15,6 +15,8 @@ import {
15 ReactiveScopeDependency,
16 Place,
17 ReactiveScopeDependencies,
18 + isUseRefType,
19 + isSetStateType,
20 } from '../HIR';
21 import {DEFAULT_EXPORT} from '../HIR/Environment';
22 import {
@@ -181,11 +183,20 @@ export function inferEffectDependencies(fn: HIRFunction): void {
183 /**
184 * Step 1: push dependencies to the effect deps array
185 *
184 - * Note that it's invalid to prune non-reactive deps in this pass, see
186 + * Note that it's invalid to prune all non-reactive deps in this pass, see
187 * the `infer-effect-deps/pruned-nonreactive-obj` fixture for an
188 * explanation.
189 */
190 for (const dep of scopeInfo.deps) {
191 + if (
192 + (isUseRefType(dep.identifier) ||
193 + isSetStateType(dep.identifier)) &&
194 + !reactiveIds.has(dep.identifier.id)
195 + ) {
196 + // exclude non-reactive hook results, which will never be in a memo block
197 + continue;
198 + }
199 +
200 const {place, instructions} = writeDependencyToInstructions(
201 dep,
202 reactiveIds.has(dep.identifier.id),
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/helper-nonreactive.expect.md new
+49
@@ -0,0 +1,49 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @inferEffectDependencies
6 +import {useEffect, useRef} from 'react';
7 +function useCustomRef() {
8 + const ref = useRef();
9 + return ref;
10 +}
11 +function NonReactiveWrapper() {
12 + const ref = useCustomRef();
13 + useEffect(() => {
14 + print(ref);
15 + });
16 +}
17 +
18 +```
19 +
20 +## Code
21 +
22 +```javascript
23 +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies
24 +import { useEffect, useRef } from "react";
25 +function useCustomRef() {
26 + const ref = useRef();
27 + return ref;
28 +}
29 +
30 +function NonReactiveWrapper() {
31 + const $ = _c(2);
32 + const ref = useCustomRef();
33 + let t0;
34 + if ($[0] !== ref) {
35 + t0 = () => {
36 + print(ref);
37 + };
38 + $[0] = ref;
39 + $[1] = t0;
40 + } else {
41 + t0 = $[1];
42 + }
43 + useEffect(t0, [ref]);
44 +}
45 +
46 +```
47 +
48 +### Eval output
49 +(kind: exception) Fixture not implemented
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/helper-nonreactive.js new
+12
@@ -0,0 +1,12 @@
1 +// @inferEffectDependencies
2 +import {useEffect, useRef} from 'react';
3 +function useCustomRef() {
4 + const ref = useRef();
5 + return ref;
6 +}
7 +function NonReactiveWrapper() {
8 + const ref = useCustomRef();
9 + useEffect(() => {
10 + print(ref);
11 + });
12 +}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/infer-effect-dependencies.expect.md
-1
@@ -101,7 +101,6 @@ function Component(t0) {
101 useEffect(t3, [
102 foo,
103 bar,
104 - ref,
104 localNonPrimitiveReactive,
105 localNonPrimitiveNonreactive,
106 ]);
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/nonreactive-ref.expect.md
+1 -1
@@ -42,7 +42,7 @@ function NonReactiveRefInEffect() {
42 } else {
43 t0 = $[0];
44 }
45 - useEffect(t0, [ref]);
45 + useEffect(t0, []);
46 }
47
48 ```
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/nonreactive-setState.expect.md new
+51
@@ -0,0 +1,51 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @inferEffectDependencies
6 +import {useEffect, useState} from 'react';
7 +import {print} from 'shared-runtime';
8 +
9 +/**
10 + * Special case of `infer-effect-deps/nonreactive-dep`.
11 + *
12 + * We know that local `useRef` return values are stable, regardless of
13 + * inferred memoization.
14 + */
15 +function NonReactiveSetStateInEffect() {
16 + const [_, setState] = useState('initial value');
17 + useEffect(() => print(setState));
18 +}
19 +
20 +```
21 +
22 +## Code
23 +
24 +```javascript
25 +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies
26 +import { useEffect, useState } from "react";
27 +import { print } from "shared-runtime";
28 +
29 +/**
30 + * Special case of `infer-effect-deps/nonreactive-dep`.
31 + *
32 + * We know that local `useRef` return values are stable, regardless of
33 + * inferred memoization.
34 + */
35 +function NonReactiveSetStateInEffect() {
36 + const $ = _c(1);
37 + const [, setState] = useState("initial value");
38 + let t0;
39 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
40 + t0 = () => print(setState);
41 + $[0] = t0;
42 + } else {
43 + t0 = $[0];
44 + }
45 + useEffect(t0, []);
46 +}
47 +
48 +```
49 +
50 +### Eval output
51 +(kind: exception) Fixture not implemented
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/nonreactive-setState.js new
+14
@@ -0,0 +1,14 @@
1 +// @inferEffectDependencies
2 +import {useEffect, useState} from 'react';
3 +import {print} from 'shared-runtime';
4 +
5 +/**
6 + * Special case of `infer-effect-deps/nonreactive-dep`.
7 + *
8 + * We know that local `useRef` return values are stable, regardless of
9 + * inferred memoization.
10 + */
11 +function NonReactiveSetStateInEffect() {
12 + const [_, setState] = useState('initial value');
13 + useEffect(() => print(setState));
14 +}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-ref.expect.md new
+66
@@ -0,0 +1,66 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @inferEffectDependencies
6 +import {useEffect, useRef} from 'react';
7 +import {print} from 'shared-runtime';
8 +
9 +/*
10 + * Ref types are not enough to determine to omit from deps. Must also take reactivity into account.
11 + */
12 +function ReactiveRefInEffect(props) {
13 + const ref1 = useRef('initial value');
14 + const ref2 = useRef('initial value');
15 + let ref;
16 + if (props.foo) {
17 + ref = ref1;
18 + } else {
19 + ref = ref2;
20 + }
21 + useEffect(() => print(ref));
22 +}
23 +
24 +```
25 +
26 +## Code
27 +
28 +```javascript
29 +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies
30 +import { useEffect, useRef } from "react";
31 +import { print } from "shared-runtime";
32 +
33 +/*
34 + * Ref types are not enough to determine to omit from deps. Must also take reactivity into account.
35 + */
36 +function ReactiveRefInEffect(props) {
37 + const $ = _c(4);
38 + const ref1 = useRef("initial value");
39 + const ref2 = useRef("initial value");
40 + let ref;
41 + if ($[0] !== props.foo) {
42 + if (props.foo) {
43 + ref = ref1;
44 + } else {
45 + ref = ref2;
46 + }
47 + $[0] = props.foo;
48 + $[1] = ref;
49 + } else {
50 + ref = $[1];
51 + }
52 + let t0;
53 + if ($[2] !== ref) {
54 + t0 = () => print(ref);
55 + $[2] = ref;
56 + $[3] = t0;
57 + } else {
58 + t0 = $[3];
59 + }
60 + useEffect(t0, [ref]);
61 +}
62 +
63 +```
64 +
65 +### Eval output
66 +(kind: exception) Fixture not implemented
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-ref.js new
+18
@@ -0,0 +1,18 @@
1 +// @inferEffectDependencies
2 +import {useEffect, useRef} from 'react';
3 +import {print} from 'shared-runtime';
4 +
5 +/*
6 + * Ref types are not enough to determine to omit from deps. Must also take reactivity into account.
7 + */
8 +function ReactiveRefInEffect(props) {
9 + const ref1 = useRef('initial value');
10 + const ref2 = useRef('initial value');
11 + let ref;
12 + if (props.foo) {
13 + ref = ref1;
14 + } else {
15 + ref = ref2;
16 + }
17 + useEffect(() => print(ref));
18 +}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-setState.expect.md new
+60
@@ -0,0 +1,60 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @inferEffectDependencies
6 +import {useEffect, useState} from 'react';
7 +import {print} from 'shared-runtime';
8 +
9 +/*
10 + * setState types are not enough to determine to omit from deps. Must also take reactivity into account.
11 + */
12 +function ReactiveRefInEffect(props) {
13 + const [_state1, setState1] = useRef('initial value');
14 + const [_state2, setState2] = useRef('initial value');
15 + let setState;
16 + if (props.foo) {
17 + setState = setState1;
18 + } else {
19 + setState = setState2;
20 + }
21 + useEffect(() => print(setState));
22 +}
23 +
24 +```
25 +
26 +## Code
27 +
28 +```javascript
29 +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies
30 +import { useEffect, useState } from "react";
31 +import { print } from "shared-runtime";
32 +
33 +/*
34 + * setState types are not enough to determine to omit from deps. Must also take reactivity into account.
35 + */
36 +function ReactiveRefInEffect(props) {
37 + const $ = _c(2);
38 + const [, setState1] = useRef("initial value");
39 + const [, setState2] = useRef("initial value");
40 + let setState;
41 + if (props.foo) {
42 + setState = setState1;
43 + } else {
44 + setState = setState2;
45 + }
46 + let t0;
47 + if ($[0] !== setState) {
48 + t0 = () => print(setState);
49 + $[0] = setState;
50 + $[1] = t0;
51 + } else {
52 + t0 = $[1];
53 + }
54 + useEffect(t0, [setState]);
55 +}
56 +
57 +```
58 +
59 +### Eval output
60 +(kind: exception) Fixture not implemented
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-setState.js new
+18
@@ -0,0 +1,18 @@
1 +// @inferEffectDependencies
2 +import {useEffect, useState} from 'react';
3 +import {print} from 'shared-runtime';
4 +
5 +/*
6 + * setState types are not enough to determine to omit from deps. Must also take reactivity into account.
7 + */
8 +function ReactiveRefInEffect(props) {
9 + const [_state1, setState1] = useRef('initial value');
10 + const [_state2, setState2] = useRef('initial value');
11 + let setState;
12 + if (props.foo) {
13 + setState = setState1;
14 + } else {
15 + setState = setState2;
16 + }
17 + useEffect(() => print(setState));
18 +}