Skip to content

Commit 85bbe39

Browse files
authored
[compiler] Fixes to enableTreatRefLikeIdentifiersAsRefs (#34000)
We added the `@enableTreatRefLikeIdentifiersAsRefs` feature a while back but never enabled it. Since then we've continued to see examples that motivate this mode, so here we're fixing it up to prepare to enable by default. It now works as follows: * If we find a property load or property store where both a) the object's name is ref-like (`ref` or `-Ref`) and b) the property is `current`, we infer the object itself as a ref and the value of the property as a ref value. Originally the feature only detected property loads, not stores. * Inferred refs are not considered stable (this is a change from the original implementation). The only way to get a stable ref is by calling `useRef()`. We've seen issues with assuming refs are stable. With this change, cases like the following now correctly error: ```js function Foo(props) { const fooRef = props.fooRef; fooRef.current = true; ^^^^^^^^^^^^^^ cannot modify ref in render } ``` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34000). * #34027 * #34026 * #34025 * #34024 * #34005 * #34006 * #34004 * #34003 * __->__ #34000
1 parent 820af20 commit 85bbe39

12 files changed

+114
-40
lines changed

compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1211,6 +1211,8 @@ addObject(BUILTIN_SHAPES, BuiltInRefValueId, [
12111211
['*', {kind: 'Object', shapeId: BuiltInRefValueId}],
12121212
]);
12131213

1214+
addObject(BUILTIN_SHAPES, ReanimatedSharedValueId, []);
1215+
12141216
addFunction(
12151217
BUILTIN_SHAPES,
12161218
[],

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

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import {
2121
isStableType,
2222
isStableTypeContainer,
2323
isUseOperator,
24-
isUseRefType,
2524
} from '../HIR';
2625
import {PostDominator} from '../HIR/Dominator';
2726
import {
@@ -70,13 +69,6 @@ class StableSidemap {
7069
isStable: false,
7170
});
7271
}
73-
} else if (
74-
this.env.config.enableTreatRefLikeIdentifiersAsRefs &&
75-
isUseRefType(lvalue.identifier)
76-
) {
77-
this.map.set(lvalue.identifier.id, {
78-
isStable: true,
79-
});
8072
}
8173
break;
8274
}

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

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,36 @@ function* generateInstructionTypes(
466466
yield equation(left, returnType);
467467
break;
468468
}
469-
case 'PropertyStore':
469+
case 'PropertyStore': {
470+
/**
471+
* Infer types based on assignments to known object properties
472+
* This is important for refs, where assignment to `<maybeRef>.current`
473+
* can help us infer that an object itself is a ref
474+
*/
475+
yield equation(
476+
/**
477+
* Our property type declarations are best-effort and we haven't tested
478+
* using them to drive inference of rvalues from lvalues. We want to emit
479+
* a Property type in order to infer refs from `.current` accesses, but
480+
* stay conservative by not otherwise inferring anything about rvalues.
481+
* So we use a dummy type here.
482+
*
483+
* TODO: consider using the rvalue type here
484+
*/
485+
makeType(),
486+
// unify() only handles properties in the second position
487+
{
488+
kind: 'Property',
489+
objectType: value.object.identifier.type,
490+
objectName: getName(names, value.object.identifier.id),
491+
propertyName: {
492+
kind: 'literal',
493+
value: value.property,
494+
},
495+
},
496+
);
497+
break;
498+
}
470499
case 'DeclareLocal':
471500
case 'RegExpLiteral':
472501
case 'MetaProperty':
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @flow @enableTreatRefLikeIdentifiersAsRefs @validateRefAccessDuringRender
6+
import {makeObject_Primitives} from 'shared-runtime';
7+
8+
component Example() {
9+
const fooRef = makeObject_Primitives();
10+
fooRef.current = true;
11+
12+
return <Stringify foo={fooRef} />;
13+
}
14+
15+
```
16+
17+
18+
## Error
19+
20+
```
21+
Found 1 error:
22+
23+
Error: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)
24+
25+
4 | component Example() {
26+
5 | const fooRef = makeObject_Primitives();
27+
> 6 | fooRef.current = true;
28+
| ^^^^^^^^^^^^^^ Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)
29+
7 |
30+
8 | return <Stringify foo={fooRef} />;
31+
9 | }
32+
```
33+
34+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// @flow @enableTreatRefLikeIdentifiersAsRefs @validateRefAccessDuringRender
2+
import {makeObject_Primitives} from 'shared-runtime';
3+
4+
component Example() {
5+
const fooRef = makeObject_Primitives();
6+
fooRef.current = true;
7+
8+
return <Stringify foo={fooRef} />;
9+
}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reanimated-no-memo-arg.expect.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
```javascript
55
// @enableCustomTypeDefinitionForReanimated
6-
import {useAnimatedProps} from 'react-native-reanimated';
6+
import {useAnimatedProps, useSharedValue} from 'react-native-reanimated';
77
function Component() {
88
const radius = useSharedValue(50);
99

@@ -39,7 +39,7 @@ export const FIXTURE_ENTRYPOINT = {
3939

4040
```javascript
4141
import { c as _c } from "react/compiler-runtime"; // @enableCustomTypeDefinitionForReanimated
42-
import { useAnimatedProps } from "react-native-reanimated";
42+
import { useAnimatedProps, useSharedValue } from "react-native-reanimated";
4343
function Component() {
4444
const $ = _c(2);
4545
const radius = useSharedValue(50);

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reanimated-no-memo-arg.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// @enableCustomTypeDefinitionForReanimated
2-
import {useAnimatedProps} from 'react-native-reanimated';
2+
import {useAnimatedProps, useSharedValue} from 'react-native-reanimated';
33
function Component() {
44
const radius = useSharedValue(50);
55

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-effect.expect.md

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,28 +47,32 @@ function useCustomRef() {
4747
function _temp() {}
4848

4949
function Foo() {
50-
const $ = _c(3);
50+
const $ = _c(4);
5151
const ref = useCustomRef();
5252
let t0;
53-
let t1;
54-
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
53+
if ($[0] !== ref) {
5554
t0 = () => {
5655
ref.current?.click();
5756
};
57+
$[0] = ref;
58+
$[1] = t0;
59+
} else {
60+
t0 = $[1];
61+
}
62+
let t1;
63+
if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
5864
t1 = [];
59-
$[0] = t0;
60-
$[1] = t1;
65+
$[2] = t1;
6166
} else {
62-
t0 = $[0];
63-
t1 = $[1];
67+
t1 = $[2];
6468
}
6569
useEffect(t0, t1);
6670
let t2;
67-
if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
71+
if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
6872
t2 = <div>foo</div>;
69-
$[2] = t2;
73+
$[3] = t2;
7074
} else {
71-
t2 = $[2];
75+
t2 = $[3];
7276
}
7377
return t2;
7478
}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-useCallback-2.expect.md

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ function Foo() {
1414

1515
const onClick = useCallback(() => {
1616
ref.current?.click();
17-
}, []);
17+
}, [ref]);
1818

1919
return <button onClick={onClick} />;
2020
}
@@ -47,24 +47,26 @@ function useCustomRef() {
4747
function _temp() {}
4848

4949
function Foo() {
50-
const $ = _c(2);
50+
const $ = _c(4);
5151
const ref = useCustomRef();
5252
let t0;
53-
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
53+
if ($[0] !== ref) {
5454
t0 = () => {
5555
ref.current?.click();
5656
};
57-
$[0] = t0;
57+
$[0] = ref;
58+
$[1] = t0;
5859
} else {
59-
t0 = $[0];
60+
t0 = $[1];
6061
}
6162
const onClick = t0;
6263
let t1;
63-
if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
64+
if ($[2] !== onClick) {
6465
t1 = <button onClick={onClick} />;
65-
$[1] = t1;
66+
$[2] = onClick;
67+
$[3] = t1;
6668
} else {
67-
t1 = $[1];
69+
t1 = $[3];
6870
}
6971
return t1;
7072
}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-useCallback-2.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ function Foo() {
1010

1111
const onClick = useCallback(() => {
1212
ref.current?.click();
13-
}, []);
13+
}, [ref]);
1414

1515
return <button onClick={onClick} />;
1616
}

0 commit comments

Comments
 (0)