Skip to content

Commit 6c55e40

Browse files
committed
[compiler] Don't show hint about ref-like naming if we infer another type
Some components accept a union of a ref callback function or ref object. In this case we may infer the type as a function due to the presence of invoking the ref callback function. In that case, we currently report a "Hint: name `fooRef` as "ref" or with a "-Ref" suffix..." even though the variable is already named appropriately — the problem is that we inferred a non-ref type. So here we check the type and don't report this hint if we inferred another type.
1 parent 7899729 commit 6c55e40

File tree

3 files changed

+77
-1
lines changed

3 files changed

+77
-1
lines changed

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1816,8 +1816,16 @@ function computeSignatureForInstruction(
18161816
}
18171817
case 'PropertyStore':
18181818
case 'ComputedStore': {
1819+
/**
1820+
* Add a hint about naming as "ref"/"-Ref", but only if we weren't able to infer any
1821+
* type for the object. In some cases the variable may be named like a ref, but is
1822+
* also used as a ref callback such that we infer the type as a function rather than
1823+
* a ref.
1824+
*/
18191825
const mutationReason: MutationReason | null =
1820-
value.kind === 'PropertyStore' && value.property === 'current'
1826+
value.kind === 'PropertyStore' &&
1827+
value.property === 'current' &&
1828+
value.object.identifier.type.kind === 'Type'
18211829
? {kind: 'AssignCurrentProperty'}
18221830
: null;
18231831
effects.push({
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @validateRefAccessDuringRender
6+
7+
function useHook(parentRef) {
8+
// Some components accept a union of "callback" refs and ref objects, which
9+
// we can't currently represent
10+
const elementRef = useRef(null);
11+
const handler = instance => {
12+
elementRef.current = instance;
13+
if (parentRef != null) {
14+
if (typeof parentRef === 'function') {
15+
// This call infers the type of `parentRef` as a function...
16+
parentRef(instance);
17+
} else {
18+
// So this assignment fails since we don't know its a ref
19+
parentRef.current = instance;
20+
}
21+
}
22+
};
23+
return handler;
24+
}
25+
26+
```
27+
28+
29+
## Error
30+
31+
```
32+
Found 1 error:
33+
34+
Error: This value cannot be modified
35+
36+
Modifying component props or hook arguments is not allowed. Consider using a local variable instead.
37+
38+
error.todo-allow-assigning-to-inferred-ref-prop-in-callback.ts:15:8
39+
13 | } else {
40+
14 | // So this assignment fails since we don't know its a ref
41+
> 15 | parentRef.current = instance;
42+
| ^^^^^^^^^ `parentRef` cannot be modified
43+
16 | }
44+
17 | }
45+
18 | };
46+
```
47+
48+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// @validateRefAccessDuringRender
2+
3+
function useHook(parentRef) {
4+
// Some components accept a union of "callback" refs and ref objects, which
5+
// we can't currently represent
6+
const elementRef = useRef(null);
7+
const handler = instance => {
8+
elementRef.current = instance;
9+
if (parentRef != null) {
10+
if (typeof parentRef === 'function') {
11+
// This call infers the type of `parentRef` as a function...
12+
parentRef(instance);
13+
} else {
14+
// So this assignment fails since we don't know its a ref
15+
parentRef.current = instance;
16+
}
17+
}
18+
};
19+
return handler;
20+
}

0 commit comments

Comments
 (0)