From 758686c1dce655365b48eb126b755657db189d94 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Fri, 25 Jul 2025 12:15:53 -0700 Subject: [PATCH] [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 } ``` --- .../src/HIR/ObjectShape.ts | 2 ++ .../src/Inference/InferReactivePlaces.ts | 8 ----- .../src/TypeInference/InferTypes.ts | 31 ++++++++++++++++- ...rrent-inferred-ref-during-render.expect.md | 34 +++++++++++++++++++ ...sign-current-inferred-ref-during-render.js | 9 +++++ .../compiler/reanimated-no-memo-arg.expect.md | 4 +-- .../compiler/reanimated-no-memo-arg.js | 2 +- .../ref-like-name-in-effect.expect.md | 26 ++++++++------ .../ref-like-name-in-useCallback-2.expect.md | 18 +++++----- .../ref-like-name-in-useCallback-2.js | 2 +- .../ref-like-name-in-useCallback.expect.md | 18 +++++----- .../compiler/ref-like-name-in-useCallback.js | 2 +- 12 files changed, 115 insertions(+), 41 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assign-current-inferred-ref-during-render.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assign-current-inferred-ref-during-render.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts index baaf40d67e..eaf728db95 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts @@ -1211,6 +1211,8 @@ addObject(BUILTIN_SHAPES, BuiltInRefValueId, [ ['*', {kind: 'Object', shapeId: BuiltInRefValueId}], ]); +addObject(BUILTIN_SHAPES, ReanimatedSharedValueId, []); + addFunction( BUILTIN_SHAPES, [], diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts index 88faccd8cf..19e220b235 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts @@ -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; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts b/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts index 596244b383..73088fd852 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts @@ -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 `.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': diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assign-current-inferred-ref-during-render.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assign-current-inferred-ref-during-render.expect.md new file mode 100644 index 0000000000..9c12d955ae --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assign-current-inferred-ref-during-render.expect.md @@ -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 ; +} + +``` + + +## 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 ; + 9 | } +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assign-current-inferred-ref-during-render.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assign-current-inferred-ref-during-render.js new file mode 100644 index 0000000000..39df293ba6 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assign-current-inferred-ref-during-render.js @@ -0,0 +1,9 @@ +// @flow @enableTreatRefLikeIdentifiersAsRefs @validateRefAccessDuringRender +import {makeObject_Primitives} from 'shared-runtime'; + +component Example() { + const fooRef = makeObject_Primitives(); + fooRef.current = true; + + return ; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reanimated-no-memo-arg.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reanimated-no-memo-arg.expect.md index 5ebc821375..1f5fb54eae 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reanimated-no-memo-arg.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reanimated-no-memo-arg.expect.md @@ -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); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reanimated-no-memo-arg.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reanimated-no-memo-arg.js index ad87d08cdc..d2865ce99a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reanimated-no-memo-arg.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reanimated-no-memo-arg.js @@ -1,5 +1,5 @@ // @enableCustomTypeDefinitionForReanimated -import {useAnimatedProps} from 'react-native-reanimated'; +import {useAnimatedProps, useSharedValue} from 'react-native-reanimated'; function Component() { const radius = useSharedValue(50); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-effect.expect.md index 156c723360..a750afe6fe 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-effect.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-effect.expect.md @@ -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 =
foo
; - $[2] = t2; + $[3] = t2; } else { - t2 = $[2]; + t2 = $[3]; } return t2; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-useCallback-2.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-useCallback-2.expect.md index c5f2f6a762..f74962e0d6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-useCallback-2.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-useCallback-2.expect.md @@ -14,7 +14,7 @@ function Foo() { const onClick = useCallback(() => { ref.current?.click(); - }, []); + }, [ref]); return