mirror of
https://github.com/zebrajr/react.git
synced 2025-12-06 12:20:20 +01:00
[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
This commit is contained in:
Joseph Savona
GitHub
parent
820af20971
commit
85bbe39ef8
|
|
@ -1211,6 +1211,8 @@ addObject(BUILTIN_SHAPES, BuiltInRefValueId, [
|
|||
['*', {kind: 'Object', shapeId: BuiltInRefValueId}],
|
||||
]);
|
||||
|
||||
addObject(BUILTIN_SHAPES, ReanimatedSharedValueId, []);
|
||||
|
||||
addFunction(
|
||||
BUILTIN_SHAPES,
|
||||
[],
|
||||
|
|
|
|||
|
|
@ -21,7 +21,6 @@ import {
|
|||
isStableType,
|
||||
isStableTypeContainer,
|
||||
isUseOperator,
|
||||
isUseRefType,
|
||||
} from '../HIR';
|
||||
import {PostDominator} from '../HIR/Dominator';
|
||||
import {
|
||||
|
|
@ -70,13 +69,6 @@ class StableSidemap {
|
|||
isStable: false,
|
||||
});
|
||||
}
|
||||
} else if (
|
||||
this.env.config.enableTreatRefLikeIdentifiersAsRefs &&
|
||||
isUseRefType(lvalue.identifier)
|
||||
) {
|
||||
this.map.set(lvalue.identifier.id, {
|
||||
isStable: true,
|
||||
});
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -466,7 +466,36 @@ function* generateInstructionTypes(
|
|||
yield equation(left, returnType);
|
||||
break;
|
||||
}
|
||||
case 'PropertyStore':
|
||||
case 'PropertyStore': {
|
||||
/**
|
||||
* Infer types based on assignments to known object properties
|
||||
* This is important for refs, where assignment to `<maybeRef>.current`
|
||||
* can help us infer that an object itself is a ref
|
||||
*/
|
||||
yield equation(
|
||||
/**
|
||||
* Our property type declarations are best-effort and we haven't tested
|
||||
* using them to drive inference of rvalues from lvalues. We want to emit
|
||||
* a Property type in order to infer refs from `.current` accesses, but
|
||||
* stay conservative by not otherwise inferring anything about rvalues.
|
||||
* So we use a dummy type here.
|
||||
*
|
||||
* TODO: consider using the rvalue type here
|
||||
*/
|
||||
makeType(),
|
||||
// unify() only handles properties in the second position
|
||||
{
|
||||
kind: 'Property',
|
||||
objectType: value.object.identifier.type,
|
||||
objectName: getName(names, value.object.identifier.id),
|
||||
propertyName: {
|
||||
kind: 'literal',
|
||||
value: value.property,
|
||||
},
|
||||
},
|
||||
);
|
||||
break;
|
||||
}
|
||||
case 'DeclareLocal':
|
||||
case 'RegExpLiteral':
|
||||
case 'MetaProperty':
|
||||
|
|
|
|||
|
|
@ -0,0 +1,34 @@
|
|||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
// @flow @enableTreatRefLikeIdentifiersAsRefs @validateRefAccessDuringRender
|
||||
import {makeObject_Primitives} from 'shared-runtime';
|
||||
|
||||
component Example() {
|
||||
const fooRef = makeObject_Primitives();
|
||||
fooRef.current = true;
|
||||
|
||||
return <Stringify foo={fooRef} />;
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
|
||||
## Error
|
||||
|
||||
```
|
||||
Found 1 error:
|
||||
|
||||
Error: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)
|
||||
|
||||
4 | component Example() {
|
||||
5 | const fooRef = makeObject_Primitives();
|
||||
> 6 | fooRef.current = true;
|
||||
| ^^^^^^^^^^^^^^ Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)
|
||||
7 |
|
||||
8 | return <Stringify foo={fooRef} />;
|
||||
9 | }
|
||||
```
|
||||
|
||||
|
||||
|
|
@ -0,0 +1,9 @@
|
|||
// @flow @enableTreatRefLikeIdentifiersAsRefs @validateRefAccessDuringRender
|
||||
import {makeObject_Primitives} from 'shared-runtime';
|
||||
|
||||
component Example() {
|
||||
const fooRef = makeObject_Primitives();
|
||||
fooRef.current = true;
|
||||
|
||||
return <Stringify foo={fooRef} />;
|
||||
}
|
||||
|
|
@ -3,7 +3,7 @@
|
|||
|
||||
```javascript
|
||||
// @enableCustomTypeDefinitionForReanimated
|
||||
import {useAnimatedProps} from 'react-native-reanimated';
|
||||
import {useAnimatedProps, useSharedValue} from 'react-native-reanimated';
|
||||
function Component() {
|
||||
const radius = useSharedValue(50);
|
||||
|
||||
|
|
@ -39,7 +39,7 @@ export const FIXTURE_ENTRYPOINT = {
|
|||
|
||||
```javascript
|
||||
import { c as _c } from "react/compiler-runtime"; // @enableCustomTypeDefinitionForReanimated
|
||||
import { useAnimatedProps } from "react-native-reanimated";
|
||||
import { useAnimatedProps, useSharedValue } from "react-native-reanimated";
|
||||
function Component() {
|
||||
const $ = _c(2);
|
||||
const radius = useSharedValue(50);
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
// @enableCustomTypeDefinitionForReanimated
|
||||
import {useAnimatedProps} from 'react-native-reanimated';
|
||||
import {useAnimatedProps, useSharedValue} from 'react-native-reanimated';
|
||||
function Component() {
|
||||
const radius = useSharedValue(50);
|
||||
|
||||
|
|
|
|||
|
|
@ -47,28 +47,32 @@ function useCustomRef() {
|
|||
function _temp() {}
|
||||
|
||||
function Foo() {
|
||||
const $ = _c(3);
|
||||
const $ = _c(4);
|
||||
const ref = useCustomRef();
|
||||
let t0;
|
||||
let t1;
|
||||
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
|
||||
if ($[0] !== ref) {
|
||||
t0 = () => {
|
||||
ref.current?.click();
|
||||
};
|
||||
t1 = [];
|
||||
$[0] = t0;
|
||||
$[1] = t1;
|
||||
$[0] = ref;
|
||||
$[1] = t0;
|
||||
} else {
|
||||
t0 = $[0];
|
||||
t1 = $[1];
|
||||
t0 = $[1];
|
||||
}
|
||||
let t1;
|
||||
if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
|
||||
t1 = [];
|
||||
$[2] = t1;
|
||||
} else {
|
||||
t1 = $[2];
|
||||
}
|
||||
useEffect(t0, t1);
|
||||
let t2;
|
||||
if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
|
||||
if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
|
||||
t2 = <div>foo</div>;
|
||||
$[2] = t2;
|
||||
$[3] = t2;
|
||||
} else {
|
||||
t2 = $[2];
|
||||
t2 = $[3];
|
||||
}
|
||||
return t2;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@ function Foo() {
|
|||
|
||||
const onClick = useCallback(() => {
|
||||
ref.current?.click();
|
||||
}, []);
|
||||
}, [ref]);
|
||||
|
||||
return <button onClick={onClick} />;
|
||||
}
|
||||
|
|
@ -47,24 +47,26 @@ function useCustomRef() {
|
|||
function _temp() {}
|
||||
|
||||
function Foo() {
|
||||
const $ = _c(2);
|
||||
const $ = _c(4);
|
||||
const ref = useCustomRef();
|
||||
let t0;
|
||||
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
|
||||
if ($[0] !== ref) {
|
||||
t0 = () => {
|
||||
ref.current?.click();
|
||||
};
|
||||
$[0] = t0;
|
||||
$[0] = ref;
|
||||
$[1] = t0;
|
||||
} else {
|
||||
t0 = $[0];
|
||||
t0 = $[1];
|
||||
}
|
||||
const onClick = t0;
|
||||
let t1;
|
||||
if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
|
||||
if ($[2] !== onClick) {
|
||||
t1 = <button onClick={onClick} />;
|
||||
$[1] = t1;
|
||||
$[2] = onClick;
|
||||
$[3] = t1;
|
||||
} else {
|
||||
t1 = $[1];
|
||||
t1 = $[3];
|
||||
}
|
||||
return t1;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ function Foo() {
|
|||
|
||||
const onClick = useCallback(() => {
|
||||
ref.current?.click();
|
||||
}, []);
|
||||
}, [ref]);
|
||||
|
||||
return <button onClick={onClick} />;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@ function Foo() {
|
|||
|
||||
const onClick = useCallback(() => {
|
||||
customRef.current?.click();
|
||||
}, []);
|
||||
}, [customRef]);
|
||||
|
||||
return <button onClick={onClick} />;
|
||||
}
|
||||
|
|
@ -47,24 +47,26 @@ function useCustomRef() {
|
|||
function _temp() {}
|
||||
|
||||
function Foo() {
|
||||
const $ = _c(2);
|
||||
const $ = _c(4);
|
||||
const customRef = useCustomRef();
|
||||
let t0;
|
||||
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
|
||||
if ($[0] !== customRef) {
|
||||
t0 = () => {
|
||||
customRef.current?.click();
|
||||
};
|
||||
$[0] = t0;
|
||||
$[0] = customRef;
|
||||
$[1] = t0;
|
||||
} else {
|
||||
t0 = $[0];
|
||||
t0 = $[1];
|
||||
}
|
||||
const onClick = t0;
|
||||
let t1;
|
||||
if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
|
||||
if ($[2] !== onClick) {
|
||||
t1 = <button onClick={onClick} />;
|
||||
$[1] = t1;
|
||||
$[2] = onClick;
|
||||
$[3] = t1;
|
||||
} else {
|
||||
t1 = $[1];
|
||||
t1 = $[3];
|
||||
}
|
||||
return t1;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ function Foo() {
|
|||
|
||||
const onClick = useCallback(() => {
|
||||
customRef.current?.click();
|
||||
}, []);
|
||||
}, [customRef]);
|
||||
|
||||
return <button onClick={onClick} />;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user