Skip to content

Commit 74b1e75

Browse files
committed
[compiler] fix false positive "mutate frozen" validation with refs
The test case here previously reported a "Cannot modify local variables after render completes" error (from ValidateNoFreezingKnownMutableFunctions). This happens because one of the functions passed to a hook clearly mutates a ref — except that we try to ignore mutations of refs! The problem in this case is that the `const ref = ...` was getting converted to a context variable since the ref is accessed in a function before its declaration. We don't infer types for context variables at all, and our ref handling is based on types, so we failed to ignore this ref mutation. The fix is to recognize that `StoreLocal const ...` is a special case: the variable may be referenced in code before the declaration, but at runtime it's either a TDZ error or the variable will have the type from the declaration. So we can safely infer a type.
1 parent 6cd727c commit 74b1e75

File tree

4 files changed

+160
-2
lines changed

4 files changed

+160
-2
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,7 @@ export function inferMutationAliasingRanges(
249249
}
250250
for (const param of [...fn.context, ...fn.params]) {
251251
const place = param.kind === 'Identifier' ? param : param.place;
252+
252253
const node = state.nodes.get(place.identifier);
253254
if (node == null) {
254255
continue;

compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
Identifier,
1515
IdentifierId,
1616
Instruction,
17+
InstructionKind,
1718
makePropertyLiteral,
1819
makeType,
1920
PropType,
@@ -194,12 +195,29 @@ function* generateInstructionTypes(
194195
break;
195196
}
196197

197-
// We intentionally do not infer types for context variables
198+
// We intentionally do not infer types for most context variables
198199
case 'DeclareContext':
199-
case 'StoreContext':
200200
case 'LoadContext': {
201201
break;
202202
}
203+
case 'StoreContext': {
204+
/**
205+
* The caveat is StoreContext const, where we know the value is
206+
* assigned once such that everywhere the value is accessed, it
207+
* must have the same type from the rvalue.
208+
*
209+
* A concrete example where this is useful is `const ref = useRef()`
210+
* where the ref is referenced before its declaration in a function
211+
* expression, causing it to be converted to a const context variable.
212+
*/
213+
if (value.lvalue.kind === InstructionKind.Const) {
214+
yield equation(
215+
value.lvalue.place.identifier.type,
216+
value.value.identifier.type,
217+
);
218+
}
219+
break;
220+
}
203221

204222
case 'StoreLocal': {
205223
if (env.config.enableUseTypeAnnotations) {
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @flow
6+
component Example() {
7+
const fooRef = useRef();
8+
9+
function updateStyles() {
10+
const foo = fooRef.current;
11+
if (barRef.current == null || foo == null) {
12+
return;
13+
}
14+
foo.style.height = '100px';
15+
}
16+
17+
const barRef = useRef(null);
18+
19+
const resizeRef = useResizeObserver(rect => {
20+
const {width} = rect;
21+
barRef.current = width;
22+
});
23+
24+
useLayoutEffect(() => {
25+
const observer = new ResizeObserver(_ => {
26+
updateStyles();
27+
});
28+
29+
return () => {
30+
observer.disconnect();
31+
};
32+
}, []);
33+
34+
return <div ref={resizeRef} />;
35+
}
36+
37+
```
38+
39+
## Code
40+
41+
```javascript
42+
import { c as _c } from "react/compiler-runtime";
43+
function Example() {
44+
const $ = _c(6);
45+
const fooRef = useRef();
46+
let t0;
47+
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
48+
t0 = function updateStyles() {
49+
const foo = fooRef.current;
50+
if (barRef.current == null || foo == null) {
51+
return;
52+
}
53+
54+
foo.style.height = "100px";
55+
};
56+
$[0] = t0;
57+
} else {
58+
t0 = $[0];
59+
}
60+
const updateStyles = t0;
61+
62+
const barRef = useRef(null);
63+
let t1;
64+
if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
65+
t1 = (rect) => {
66+
const { width } = rect;
67+
barRef.current = width;
68+
};
69+
$[1] = t1;
70+
} else {
71+
t1 = $[1];
72+
}
73+
const resizeRef = useResizeObserver(t1);
74+
let t2;
75+
let t3;
76+
if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
77+
t2 = () => {
78+
const observer = new ResizeObserver((_) => {
79+
updateStyles();
80+
});
81+
return () => {
82+
observer.disconnect();
83+
};
84+
};
85+
86+
t3 = [];
87+
$[2] = t2;
88+
$[3] = t3;
89+
} else {
90+
t2 = $[2];
91+
t3 = $[3];
92+
}
93+
useLayoutEffect(t2, t3);
94+
let t4;
95+
if ($[4] !== resizeRef) {
96+
t4 = <div ref={resizeRef} />;
97+
$[4] = resizeRef;
98+
$[5] = t4;
99+
} else {
100+
t4 = $[5];
101+
}
102+
return t4;
103+
}
104+
105+
```
106+
107+
### Eval output
108+
(kind: exception) Fixture not implemented
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// @flow
2+
component Example() {
3+
const fooRef = useRef();
4+
5+
function updateStyles() {
6+
const foo = fooRef.current;
7+
if (barRef.current == null || foo == null) {
8+
return;
9+
}
10+
foo.style.height = '100px';
11+
}
12+
13+
const barRef = useRef(null);
14+
15+
const resizeRef = useResizeObserver(rect => {
16+
const {width} = rect;
17+
barRef.current = width;
18+
});
19+
20+
useLayoutEffect(() => {
21+
const observer = new ResizeObserver(_ => {
22+
updateStyles();
23+
});
24+
25+
return () => {
26+
observer.disconnect();
27+
};
28+
}, []);
29+
30+
return <div ref={resizeRef} />;
31+
}

0 commit comments

Comments
 (0)