[compiler] Don't include current field accesses in auto-deps (#31652)
## Summary Drops .current field accesses in inferred dep arrays --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31652). * #31657 * __->__ #31652
Jordan Brown committed
Dec 3, 2024 at 07:42 UTC
2ab471c8d2d689e3a5a81178d9fac9bb6d4805bd
3 files changed
+123
compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts
+7
@@ -222,6 +222,13 @@ function writeDependencyToInstructions(
222
*/
223
break;
224
}
225
+ if (path.property === 'current') {
226
+ /*
227
+ * Prune ref.current accesses. This may over-capture for non-ref values with
228
+ * a current property, but that's fine.
229
+ */
230
+ break;
231
+ }
232
const nextValue = createTemporaryPlace(env, GeneratedSource);
233
nextValue.reactive = reactive;
234
instructions.push({
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonreactive-ref-helper.expect.md
new
+89
@@ -0,0 +1,89 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// @inferEffectDependencies
6
+import {useEffect} from 'react';
7
+import {print} from 'shared-runtime';
8
+
9
+/**
10
+ * We never include a .current access in a dep array because it may be a ref access.
11
+ * This might over-capture objects that are not refs and happen to have fields named
12
+ * current, but that should be a rare case and the result would still be correct
13
+ * (assuming the effect is idempotent). In the worst case, you can always write a manual
14
+ * dep array.
15
+ */
16
+function RefsInEffects() {
17
+ const ref = useRefHelper();
18
+ const wrapped = useDeeperRefHelper();
19
+ useEffect(() => {
20
+ print(ref.current);
21
+ print(wrapped.foo.current);
22
+ });
23
+}
24
+
25
+function useRefHelper() {
26
+ return useRef(0);
27
+}
28
+
29
+function useDeeperRefHelper() {
30
+ return {foo: useRefHelper()};
31
+}
32
+
33
+```
34
+
35
+## Code
36
+
37
+```javascript
38
+import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies
39
+import { useEffect } from "react";
40
+import { print } from "shared-runtime";
41
+
42
+/**
43
+ * We never include a .current access in a dep array because it may be a ref access.
44
+ * This might over-capture objects that are not refs and happen to have fields named
45
+ * current, but that should be a rare case and the result would still be correct
46
+ * (assuming the effect is idempotent). In the worst case, you can always write a manual
47
+ * dep array.
48
+ */
49
+function RefsInEffects() {
50
+ const $ = _c(3);
51
+ const ref = useRefHelper();
52
+ const wrapped = useDeeperRefHelper();
53
+ let t0;
54
+ if ($[0] !== ref.current || $[1] !== wrapped.foo.current) {
55
+ t0 = () => {
56
+ print(ref.current);
57
+ print(wrapped.foo.current);
58
+ };
59
+ $[0] = ref.current;
60
+ $[1] = wrapped.foo.current;
61
+ $[2] = t0;
62
+ } else {
63
+ t0 = $[2];
64
+ }
65
+ useEffect(t0, [ref, wrapped.foo]);
66
+}
67
+
68
+function useRefHelper() {
69
+ return useRef(0);
70
+}
71
+
72
+function useDeeperRefHelper() {
73
+ const $ = _c(2);
74
+ const t0 = useRefHelper();
75
+ let t1;
76
+ if ($[0] !== t0) {
77
+ t1 = { foo: t0 };
78
+ $[0] = t0;
79
+ $[1] = t1;
80
+ } else {
81
+ t1 = $[1];
82
+ }
83
+ return t1;
84
+}
85
+
86
+```
87
+
88
+### Eval output
89
+(kind: exception) Fixture not implemented
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonreactive-ref-helper.js
new
+27
@@ -0,0 +1,27 @@
1
+// @inferEffectDependencies
2
+import {useEffect} from 'react';
3
+import {print} from 'shared-runtime';
4
+
5
+/**
6
+ * We never include a .current access in a dep array because it may be a ref access.
7
+ * This might over-capture objects that are not refs and happen to have fields named
8
+ * current, but that should be a rare case and the result would still be correct
9
+ * (assuming the effect is idempotent). In the worst case, you can always write a manual
10
+ * dep array.
11
+ */
12
+function RefsInEffects() {
13
+ const ref = useRefHelper();
14
+ const wrapped = useDeeperRefHelper();
15
+ useEffect(() => {
16
+ print(ref.current);
17
+ print(wrapped.foo.current);
18
+ });
19
+}
20
+
21
+function useRefHelper() {
22
+ return useRef(0);
23
+}
24
+
25
+function useDeeperRefHelper() {
26
+ return {foo: useRefHelper()};
27
+}