Skip to content

Commit 2ab471c

Browse files
authored
[compiler] Don't include current field accesses in auto-deps (facebook#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). * facebook#31657 * __->__ facebook#31652
1 parent 865d2c4 commit 2ab471c

File tree

3 files changed

+123
-0
lines changed

3 files changed

+123
-0
lines changed

compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,13 @@ function writeDependencyToInstructions(
222222
*/
223223
break;
224224
}
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+
}
225232
const nextValue = createTemporaryPlace(env, GeneratedSource);
226233
nextValue.reactive = reactive;
227234
instructions.push({
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -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
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -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+
}

0 commit comments

Comments
 (0)