@samitouri / QOS-React / commits / 8d7b5e4903

[compiler] Show a ref name hint when assigning to non-ref in a callback (#34298)

In #34125 I added a hint where if you assign to the .current property of a frozen object, we suggest naming the variable as `ref` or `-Ref`. However, the tracking for mutations that assign to .current specifically wasn't propagated past function expression boundaries, which meant that the hint only showed up if you mutated the ref in the main body of the component/hook. That's less likely to happen since most folks know not to access refs in render. What's more likely is that you'll (correctly) assign a ref in an effect or callback, but the compiler will throw an error. By showing a hint in this case we can help people understand the naming pattern. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34298). * #34276 * __->__ #34298

Joseph Savona committed Aug 27, 2025 at 17:05 UTC 8d7b5e490320732f40d9c3aa4590b5b0ae5116f5
5 files changed +58 -2
compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts
+1 -1
@@ -984,7 +984,7 @@ export function printAliasingEffect(effect: AliasingEffect): string {
984 case 'MutateConditionally':
985 case 'MutateTransitive':
986 case 'MutateTransitiveConditionally': {
987 - return `${effect.kind} ${printPlaceForAliasEffect(effect.value)}`;
987 + return `${effect.kind} ${printPlaceForAliasEffect(effect.value)}${effect.kind === 'Mutate' && effect.reason?.kind === 'AssignCurrentProperty' ? ' (assign `.current`)' : ''}`;
988 }
989 case 'MutateFrozen': {
990 return `MutateFrozen ${printPlaceForAliasEffect(effect.place)} reason=${JSON.stringify(effect.error.reason)}`;
compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts
+11 -1
@@ -27,7 +27,7 @@ import {
27 } from '../HIR/visitors';
28 import {assertExhaustive, getOrInsertWith} from '../Utils/utils';
29 import {Err, Ok, Result} from '../Utils/Result';
30 -import {AliasingEffect} from './AliasingEffects';
30 +import {AliasingEffect, MutationReason} from './AliasingEffects';
31
32 /**
33 * This pass builds an abstract model of the heap and interprets the effects of the
@@ -101,6 +101,7 @@ export function inferMutationAliasingRanges(
101 transitive: boolean;
102 kind: MutationKind;
103 place: Place;
104 + reason: MutationReason | null;
105 }> = [];
106 const renders: Array<{index: number; place: Place}> = [];
107
@@ -176,6 +177,7 @@ export function inferMutationAliasingRanges(
177 effect.kind === 'MutateTransitive'
178 ? MutationKind.Definite
179 : MutationKind.Conditional,
180 + reason: null,
181 place: effect.value,
182 });
183 } else if (
@@ -190,6 +192,7 @@ export function inferMutationAliasingRanges(
192 effect.kind === 'Mutate'
193 ? MutationKind.Definite
194 : MutationKind.Conditional,
195 + reason: effect.kind === 'Mutate' ? (effect.reason ?? null) : null,
196 place: effect.value,
197 });
198 } else if (
@@ -241,6 +244,7 @@ export function inferMutationAliasingRanges(
244 mutation.transitive,
245 mutation.kind,
246 mutation.place.loc,
247 + mutation.reason,
248 errors,
249 );
250 }
@@ -267,6 +271,7 @@ export function inferMutationAliasingRanges(
271 functionEffects.push({
272 kind: 'Mutate',
273 value: {...place, loc: node.local.loc},
274 + reason: node.mutationReason,
275 });
276 }
277 }
@@ -507,6 +512,7 @@ export function inferMutationAliasingRanges(
512 true,
513 MutationKind.Conditional,
514 into.loc,
515 + null,
516 ignoredErrors,
517 );
518 for (const from of tracked) {
@@ -580,6 +586,7 @@ type Node = {
586 transitive: {kind: MutationKind; loc: SourceLocation} | null;
587 local: {kind: MutationKind; loc: SourceLocation} | null;
588 lastMutated: number;
589 + mutationReason: MutationReason | null;
590 value:
591 | {kind: 'Object'}
592 | {kind: 'Phi'}
@@ -599,6 +606,7 @@ class AliasingState {
606 transitive: null,
607 local: null,
608 lastMutated: 0,
609 + mutationReason: null,
610 value,
611 });
612 }
@@ -697,6 +705,7 @@ class AliasingState {
705 transitive: boolean,
706 startKind: MutationKind,
707 loc: SourceLocation,
708 + reason: MutationReason | null,
709 errors: CompilerError,
710 ): void {
711 const seen = new Map<Identifier, MutationKind>();
@@ -717,6 +726,7 @@ class AliasingState {
726 if (node == null) {
727 continue;
728 }
729 + node.mutationReason ??= reason;
730 node.lastMutated = Math.max(node.lastMutated, index);
731 if (end != null) {
732 node.id.mutableRange.end = makeInstructionId(
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-ref-in-effect-hint.expect.md new
+37
@@ -0,0 +1,37 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// Fixture to test that we show a hint to name as `ref` or `-Ref` when attempting
6 +// to assign .current inside an effect
7 +function Component({foo}) {
8 + useEffect(() => {
9 + foo.current = true;
10 + }, [foo]);
11 +}
12 +
13 +```
14 +
15 +
16 +## Error
17 +
18 +```
19 +Found 1 error:
20 +
21 +Error: This value cannot be modified
22 +
23 +Modifying component props or hook arguments is not allowed. Consider using a local variable instead.
24 +
25 +error.assign-ref-in-effect-hint.ts:5:4
26 + 3 | function Component({foo}) {
27 + 4 | useEffect(() => {
28 +> 5 | foo.current = true;
29 + | ^^^ `foo` cannot be modified
30 + 6 | }, [foo]);
31 + 7 | }
32 + 8 |
33 +
34 +Hint: If this value is a Ref (value returned by `useRef()`), rename the variable to end in "Ref".
35 +```
36 +
37 +
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-ref-in-effect-hint.js new
+7
@@ -0,0 +1,7 @@
1 +// Fixture to test that we show a hint to name as `ref` or `-Ref` when attempting
2 +// to assign .current inside an effect
3 +function Component({foo}) {
4 + useEffect(() => {
5 + foo.current = true;
6 + }, [foo]);
7 +}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-context-in-callback.expect.md
+2
@@ -38,6 +38,8 @@ error.invalid-mutate-context-in-callback.ts:12:4
38 13 | };
39 14 | return <div onClick={onClick} />;
40 15 | }
41 +
42 +Hint: If this value is a Ref (value returned by `useRef()`), rename the variable to end in "Ref".
43 ```
44
45
\ No newline at end of file