@samitouri / QOS-React / commits / 724b324b96

[compiler] Add hint to name variables with "Ref" suffix (#34125)

If you have a ref that the compiler doesn't know is a ref (say, a value returned from a custom hook) and try to assign its `.current = ...`, we currently fail with a generic error that hook return values are not mutable. However, an assignment to `.current` specifically is a very strong hint that the value is likely to be a ref. So in this PR, we track the reason for the mutation and if it ends up being an error, we use it to show an additional hint to the user. See the fixture for an example of the message. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34125). * #34126 * __->__ #34125 * #34124

Joseph Savona committed Aug 15, 2025 at 15:05 UTC 724b324b966343f08ff0cb16ec9d8013e3891dfd
4 files changed +107 -21
compiler/packages/babel-plugin-react-compiler/src/Inference/AliasingEffects.ts
+3 -1
@@ -50,7 +50,7 @@ export type AliasingEffect =
50 /**
51 * Mutate the value and any direct aliases (not captures). Errors if the value is not mutable.
52 */
53 - | {kind: 'Mutate'; value: Place}
53 + | {kind: 'Mutate'; value: Place; reason?: MutationReason | null}
54 /**
55 * Mutate the value and any direct aliases (not captures), but only if the value is known mutable.
56 * This should be rare.
@@ -174,6 +174,8 @@ export type AliasingEffect =
174 place: Place;
175 };
176
177 +export type MutationReason = {kind: 'AssignCurrentProperty'};
178 +
179 export function hashEffect(effect: AliasingEffect): string {
180 switch (effect.kind) {
181 case 'Apply': {
compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts
+55 -20
@@ -68,7 +68,12 @@ import {FunctionSignature} from '../HIR/ObjectShape';
68 import {getWriteErrorReason} from './InferFunctionEffects';
69 import prettyFormat from 'pretty-format';
70 import {createTemporaryPlace} from '../HIR/HIRBuilder';
71 -import {AliasingEffect, AliasingSignature, hashEffect} from './AliasingEffects';
71 +import {
72 + AliasingEffect,
73 + AliasingSignature,
74 + hashEffect,
75 + MutationReason,
76 +} from './AliasingEffects';
77
78 const DEBUG = false;
79
@@ -452,18 +457,29 @@ function applySignature(
457 effect.value.identifier.name.kind === 'named'
458 ? `\`${effect.value.identifier.name.value}\``
459 : 'value';
460 + const diagnostic = CompilerDiagnostic.create({
461 + severity: ErrorSeverity.InvalidReact,
462 + category: 'This value cannot be modified',
463 + description: `${reason}.`,
464 + }).withDetail({
465 + kind: 'error',
466 + loc: effect.value.loc,
467 + message: `${variable} cannot be modified`,
468 + });
469 + if (
470 + effect.kind === 'Mutate' &&
471 + effect.reason?.kind === 'AssignCurrentProperty'
472 + ) {
473 + diagnostic.withDetail({
474 + kind: 'error',
475 + loc: effect.value.loc,
476 + message: `Hint: If this value is a Ref (value returned by \`useRef()\`), rename the variable to end in "Ref".`,
477 + });
478 + }
479 effects.push({
480 kind: 'MutateFrozen',
481 place: effect.value,
458 - error: CompilerDiagnostic.create({
459 - severity: ErrorSeverity.InvalidReact,
460 - category: 'This value cannot be modified',
461 - description: `${reason}.`,
462 - }).withDetail({
463 - kind: 'error',
464 - loc: effect.value.loc,
465 - message: `${variable} cannot be modified`,
466 - }),
482 + error: diagnostic,
483 });
484 }
485 }
@@ -1066,6 +1082,25 @@ function applyEffect(
1082 effect.value.identifier.name.kind === 'named'
1083 ? `\`${effect.value.identifier.name.value}\``
1084 : 'value';
1085 + const diagnostic = CompilerDiagnostic.create({
1086 + severity: ErrorSeverity.InvalidReact,
1087 + category: 'This value cannot be modified',
1088 + description: `${reason}.`,
1089 + }).withDetail({
1090 + kind: 'error',
1091 + loc: effect.value.loc,
1092 + message: `${variable} cannot be modified`,
1093 + });
1094 + if (
1095 + effect.kind === 'Mutate' &&
1096 + effect.reason?.kind === 'AssignCurrentProperty'
1097 + ) {
1098 + diagnostic.withDetail({
1099 + kind: 'error',
1100 + loc: effect.value.loc,
1101 + message: `Hint: If this value is a Ref (value returned by \`useRef()\`), rename the variable to end in "Ref".`,
1102 + });
1103 + }
1104 applyEffect(
1105 context,
1106 state,
@@ -1075,15 +1110,7 @@ function applyEffect(
1110 ? 'MutateFrozen'
1111 : 'MutateGlobal',
1112 place: effect.value,
1078 - error: CompilerDiagnostic.create({
1079 - severity: ErrorSeverity.InvalidReact,
1080 - category: 'This value cannot be modified',
1081 - description: `${reason}.`,
1082 - }).withDetail({
1083 - kind: 'error',
1084 - loc: effect.value.loc,
1085 - message: `${variable} cannot be modified`,
1086 - }),
1113 + error: diagnostic,
1114 },
1115 initialized,
1116 effects,
@@ -1680,7 +1707,15 @@ function computeSignatureForInstruction(
1707 }
1708 case 'PropertyStore':
1709 case 'ComputedStore': {
1683 - effects.push({kind: 'Mutate', value: value.object});
1710 + const mutationReason: MutationReason | null =
1711 + value.kind === 'PropertyStore' && value.property === 'current'
1712 + ? {kind: 'AssignCurrentProperty'}
1713 + : null;
1714 + effects.push({
1715 + kind: 'Mutate',
1716 + value: value.object,
1717 + reason: mutationReason,
1718 + });
1719 effects.push({
1720 kind: 'Capture',
1721 from: value.value,
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assing-to-ref-current-in-render.expect.md new
+42
@@ -0,0 +1,42 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @flow
6 +
7 +component Foo() {
8 + const foo = useFoo();
9 + foo.current = true;
10 + return <div />;
11 +}
12 +
13 +```
14 +
15 +
16 +## Error
17 +
18 +```
19 +Found 1 error:
20 +
21 +Error: This value cannot be modified
22 +
23 +Modifying a value returned from a hook is not allowed. Consider moving the modification into the hook where the value is constructed.
24 +
25 + 3 | component Foo() {
26 + 4 | const foo = useFoo();
27 +> 5 | foo.current = true;
28 + | ^^^ value cannot be modified
29 + 6 | return <div />;
30 + 7 | }
31 + 8 |
32 +
33 + 3 | component Foo() {
34 + 4 | const foo = useFoo();
35 +> 5 | foo.current = true;
36 + | ^^^ Hint: If this value is a Ref (value returned by `useRef()`), rename the variable to end in "Ref".
37 + 6 | return <div />;
38 + 7 | }
39 + 8 |
40 +```
41 +
42 +
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assing-to-ref-current-in-render.js new
+7
@@ -0,0 +1,7 @@
1 +// @flow
2 +
3 +component Foo() {
4 + const foo = useFoo();
5 + foo.current = true;
6 + return <div />;
7 +}