Skip to content

Commit 8379e63

Browse files
committed
[compiler] Fixes to enableTreatRefLikeIdentifiersAsRefs
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 } ```
1 parent 2aa5f9d commit 8379e63

9 files changed

+94
-37
lines changed

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: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,31 @@ 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+
kind: 'Property',
478+
objectType: value.object.identifier.type,
479+
objectName: getName(names, value.object.identifier.id),
480+
propertyName: {
481+
kind: 'literal',
482+
value: value.property,
483+
},
484+
},
485+
/**
486+
* We don't want the rvalue to be assigned a RefValue type just
487+
* because it was assigned as the ref's value, since that twill trigger
488+
* additional errors about accessing ref values during render.
489+
*/
490+
makeType(),
491+
);
492+
break;
493+
}
470494
case 'DeclareLocal':
471495
case 'RegExpLiteral':
472496
case 'MetaProperty':
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
2+
## Input
3+
4+
```javascript
5+
import {makeObject_Primitives} from 'shared-runtime';
6+
7+
// @flow @enableTreatRefLikeIdentifiersAsRefs
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+
Missing semicolon. (4:9)
22+
```
23+
24+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import {makeObject_Primitives} from 'shared-runtime';
2+
3+
// @flow @enableTreatRefLikeIdentifiersAsRefs
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/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
}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-useCallback.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
customRef.current?.click();
17-
}, []);
17+
}, [customRef]);
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 customRef = useCustomRef();
5252
let t0;
53-
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
53+
if ($[0] !== customRef) {
5454
t0 = () => {
5555
customRef.current?.click();
5656
};
57-
$[0] = t0;
57+
$[0] = customRef;
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.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
customRef.current?.click();
13-
}, []);
13+
}, [customRef]);
1414

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

0 commit comments

Comments
 (0)