Skip to content

Commit 68d59d4

Browse files
authored
[compiler][ez] Fix reanimated custom type defs for imports (#31137)
When we added support for Reanimated, we didn't distinguish between true globals (i.e. identifiers with no static resolutions), module types, and imports #29188. For the past 3-4 months, Reanimated imports were not being matched to the correct hook / function shape we match globals and module imports against two different registries. This PR fixes our support for Reanimated library functions imported under `react-native-reanimated`. See test fixtures for details
1 parent 91c42a1 commit 68d59d4

9 files changed

+149
-16
lines changed

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
DEFAULT_SHAPES,
1717
Global,
1818
GlobalRegistry,
19-
installReAnimatedTypes,
19+
getReanimatedModuleType,
2020
installTypeConfig,
2121
} from './Globals';
2222
import {
@@ -688,7 +688,8 @@ export class Environment {
688688
}
689689

690690
if (config.enableCustomTypeDefinitionForReanimated) {
691-
installReAnimatedTypes(this.#globals, this.#shapes);
691+
const reanimatedModuleType = getReanimatedModuleType(this.#shapes);
692+
this.#moduleTypes.set(REANIMATED_MODULE_NAME, reanimatedModuleType);
692693
}
693694

694695
this.#contextIdentifiers = contextIdentifiers;
@@ -734,11 +735,11 @@ export class Environment {
734735
}
735736

736737
#resolveModuleType(moduleName: string, loc: SourceLocation): Global | null {
737-
if (this.config.moduleTypeProvider == null) {
738-
return null;
739-
}
740738
let moduleType = this.#moduleTypes.get(moduleName);
741739
if (moduleType === undefined) {
740+
if (this.config.moduleTypeProvider == null) {
741+
return null;
742+
}
742743
const unparsedModuleConfig = this.config.moduleTypeProvider(moduleName);
743744
if (unparsedModuleConfig != null) {
744745
const parsedModuleConfig = TypeSchema.safeParse(unparsedModuleConfig);
@@ -957,6 +958,8 @@ export class Environment {
957958
}
958959
}
959960

961+
const REANIMATED_MODULE_NAME = 'react-native-reanimated';
962+
960963
// From https://github.com/facebook/react/blob/main/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js#LL18C1-L23C2
961964
export function isHookName(name: string): boolean {
962965
return /^use[A-Z0-9]/.test(name);

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

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import {
2525
addHook,
2626
addObject,
2727
} from './ObjectShape';
28-
import {BuiltInType, PolyType} from './Types';
28+
import {BuiltInType, ObjectType, PolyType} from './Types';
2929
import {TypeConfig} from './TypeSchema';
3030
import {assertExhaustive} from '../Utils/utils';
3131
import {isHookName} from './Environment';
@@ -652,10 +652,7 @@ export function installTypeConfig(
652652
}
653653
}
654654

655-
export function installReAnimatedTypes(
656-
globals: GlobalRegistry,
657-
registry: ShapeRegistry,
658-
): void {
655+
export function getReanimatedModuleType(registry: ShapeRegistry): ObjectType {
659656
// hooks that freeze args and return frozen value
660657
const frozenHooks = [
661658
'useFrameCallback',
@@ -665,8 +662,9 @@ export function installReAnimatedTypes(
665662
'useAnimatedReaction',
666663
'useWorkletCallback',
667664
];
665+
const reanimatedType: Array<[string, BuiltInType]> = [];
668666
for (const hook of frozenHooks) {
669-
globals.set(
667+
reanimatedType.push([
670668
hook,
671669
addHook(registry, {
672670
positionalParams: [],
@@ -677,7 +675,7 @@ export function installReAnimatedTypes(
677675
calleeEffect: Effect.Read,
678676
hookKind: 'Custom',
679677
}),
680-
);
678+
]);
681679
}
682680

683681
/**
@@ -686,7 +684,7 @@ export function installReAnimatedTypes(
686684
*/
687685
const mutableHooks = ['useSharedValue', 'useDerivedValue'];
688686
for (const hook of mutableHooks) {
689-
globals.set(
687+
reanimatedType.push([
690688
hook,
691689
addHook(registry, {
692690
positionalParams: [],
@@ -697,7 +695,7 @@ export function installReAnimatedTypes(
697695
calleeEffect: Effect.Read,
698696
hookKind: 'Custom',
699697
}),
700-
);
698+
]);
701699
}
702700

703701
// functions that return mutable value
@@ -711,7 +709,7 @@ export function installReAnimatedTypes(
711709
'executeOnUIRuntimeSync',
712710
];
713711
for (const fn of funcs) {
714-
globals.set(
712+
reanimatedType.push([
715713
fn,
716714
addFunction(registry, [], {
717715
positionalParams: [],
@@ -721,6 +719,8 @@ export function installReAnimatedTypes(
721719
returnValueKind: ValueKind.Mutable,
722720
noAlias: true,
723721
}),
724-
);
722+
]);
725723
}
724+
725+
return addObject(registry, null, reanimatedType);
726726
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @enableCustomTypeDefinitionForReanimated
6+
7+
/**
8+
* Test that a global (i.e. non-imported) useSharedValue is treated as an
9+
* unknown hook.
10+
*/
11+
function SomeComponent() {
12+
const sharedVal = useSharedValue(0);
13+
return (
14+
<Button
15+
onPress={() => (sharedVal.value = Math.random())}
16+
title="Randomize"
17+
/>
18+
);
19+
}
20+
21+
```
22+
23+
24+
## Error
25+
26+
```
27+
9 | return (
28+
10 | <Button
29+
> 11 | onPress={() => (sharedVal.value = Math.random())}
30+
| ^^^^^^^^^ InvalidReact: Mutating a value returned from a function whose return value should not be mutated. Found mutation of `sharedVal` (11:11)
31+
12 | title="Randomize"
32+
13 | />
33+
14 | );
34+
```
35+
36+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// @enableCustomTypeDefinitionForReanimated
2+
3+
/**
4+
* Test that a global (i.e. non-imported) useSharedValue is treated as an
5+
* unknown hook.
6+
*/
7+
function SomeComponent() {
8+
const sharedVal = useSharedValue(0);
9+
return (
10+
<Button
11+
onPress={() => (sharedVal.value = Math.random())}
12+
title="Randomize"
13+
/>
14+
);
15+
}

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

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

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

@@ -38,6 +39,7 @@ export const FIXTURE_ENTRYPOINT = {
3839

3940
```javascript
4041
import { c as _c } from "react/compiler-runtime"; // @enableCustomTypeDefinitionForReanimated
42+
import { useAnimatedProps } from "react-native-reanimated";
4143
function Component() {
4244
const $ = _c(2);
4345
const radius = useSharedValue(50);

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// @enableCustomTypeDefinitionForReanimated
2+
import {useAnimatedProps} from 'react-native-reanimated';
23
function Component() {
34
const radius = useSharedValue(50);
45

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @enableCustomTypeDefinitionForReanimated
6+
import {useSharedValue} from 'react-native-reanimated';
7+
8+
/**
9+
* https://docs.swmansion.com/react-native-reanimated/docs/2.x/api/hooks/useSharedValue/
10+
*
11+
* Test that shared values are treated as ref-like, i.e. allowing writes outside
12+
* of render
13+
*/
14+
function SomeComponent() {
15+
const sharedVal = useSharedValue(0);
16+
return (
17+
<Button
18+
onPress={() => (sharedVal.value = Math.random())}
19+
title="Randomize"
20+
/>
21+
);
22+
}
23+
24+
```
25+
26+
## Code
27+
28+
```javascript
29+
import { c as _c } from "react/compiler-runtime"; // @enableCustomTypeDefinitionForReanimated
30+
import { useSharedValue } from "react-native-reanimated";
31+
32+
/**
33+
* https://docs.swmansion.com/react-native-reanimated/docs/2.x/api/hooks/useSharedValue/
34+
*
35+
* Test that shared values are treated as ref-like, i.e. allowing writes outside
36+
* of render
37+
*/
38+
function SomeComponent() {
39+
const $ = _c(3);
40+
const sharedVal = useSharedValue(0);
41+
42+
const T0 = Button;
43+
const t0 = () => (sharedVal.value = Math.random());
44+
let t1;
45+
if ($[0] !== T0 || $[1] !== t0) {
46+
t1 = <T0 onPress={t0} title="Randomize" />;
47+
$[0] = T0;
48+
$[1] = t0;
49+
$[2] = t1;
50+
} else {
51+
t1 = $[2];
52+
}
53+
return t1;
54+
}
55+
56+
```
57+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// @enableCustomTypeDefinitionForReanimated
2+
import {useSharedValue} from 'react-native-reanimated';
3+
4+
/**
5+
* https://docs.swmansion.com/react-native-reanimated/docs/2.x/api/hooks/useSharedValue/
6+
*
7+
* Test that shared values are treated as ref-like, i.e. allowing writes outside
8+
* of render
9+
*/
10+
function SomeComponent() {
11+
const sharedVal = useSharedValue(0);
12+
return (
13+
<Button
14+
onPress={() => (sharedVal.value = Math.random())}
15+
title="Randomize"
16+
/>
17+
);
18+
}

compiler/packages/snap/src/SproutTodoFilter.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,7 @@ const skipFilter = new Set([
434434
'todo.useContext-mutate-context-in-callback',
435435
'loop-unused-let',
436436
'reanimated-no-memo-arg',
437+
'reanimated-shared-value-writes',
437438

438439
'userspace-use-memo-cache',
439440
'transitive-freeze-function-expressions',

0 commit comments

Comments
 (0)