@samitouri / QOS-React-2 / commits / b067c6fe79

[compiler] Improve error message for mutating hook args/return (#33513)

The previous error message was generic, because the old style function signature didn't support a way to specify a reason alongside a freeze effect. This meant we could only say why a value was frozen for instructions, but not hooks which use function signatures. By defining a new aliasing signature for custom hooks we can specify a reason and provide a better error message. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33513). * #33571 * #33558 * #33547 * #33543 * #33533 * #33532 * #33530 * #33526 * #33522 * #33518 * #33514 * __->__ #33513

Joseph Savona committed Jun 18, 2025 at 13:04 UTC b067c6fe7962ce2c8ddca6deeadcb21afe2538f8
10 files changed +71 -26
compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts
+10
@@ -1388,6 +1388,16 @@ export enum ValueReason {
1388 */
1389 JsxCaptured = 'jsx-captured',
1390
1391 + /**
1392 + * Argument to a hook
1393 + */
1394 + HookCaptured = 'hook-captured',
1395 +
1396 + /**
1397 + * Return value of a hook
1398 + */
1399 + HookReturn = 'hook-return',
1400 +
1401 /**
1402 * Passed to an effect
1403 */
compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts
+28
@@ -1302,6 +1302,34 @@ export const DefaultNonmutatingHook = addHook(
1302 calleeEffect: Effect.Read,
1303 hookKind: 'Custom',
1304 returnValueKind: ValueKind.Frozen,
1305 + aliasing: {
1306 + receiver: makeIdentifierId(0),
1307 + params: [],
1308 + rest: makeIdentifierId(1),
1309 + returns: makeIdentifierId(2),
1310 + temporaries: [],
1311 + effects: [
1312 + // Freeze the arguments
1313 + {
1314 + kind: 'Freeze',
1315 + value: signatureArgument(1),
1316 + reason: ValueReason.HookCaptured,
1317 + },
1318 + // Returns a frozen value
1319 + {
1320 + kind: 'Create',
1321 + into: signatureArgument(2),
1322 + value: ValueKind.Frozen,
1323 + reason: ValueReason.HookReturn,
1324 + },
1325 + // May alias any arguments into the return
1326 + {
1327 + kind: 'Alias',
1328 + from: signatureArgument(1),
1329 + into: signatureArgument(2),
1330 + },
1331 + ],
1332 + },
1333 },
1334 'DefaultNonmutatingHook',
1335 );
compiler/packages/babel-plugin-react-compiler/src/Inference/InferFunctionEffects.ts
+4
@@ -341,6 +341,10 @@ export function getWriteErrorReason(abstractValue: AbstractValue): string {
341 return "Mutating a value returned from 'useReducer()', which should not be mutated. Use the dispatch function to update instead";
342 } else if (abstractValue.reason.has(ValueReason.Effect)) {
343 return 'Updating a value used previously in an effect function or as an effect dependency is not allowed. Consider moving the mutation before calling useEffect()';
344 + } else if (abstractValue.reason.has(ValueReason.HookCaptured)) {
345 + return 'Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook';
346 + } else if (abstractValue.reason.has(ValueReason.HookReturn)) {
347 + return 'Updating a value returned from a hook is not allowed. Consider moving the mutation into the hook where the value is constructed';
348 } else {
349 return 'This mutates a variable that React considers immutable';
350 }
compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts
+23 -20
@@ -1980,28 +1980,17 @@ function computeEffectsForLegacySignature(
1980 break;
1981 }
1982 case Effect.ConditionallyMutateIterator: {
1983 - if (
1984 - isArrayType(place.identifier) ||
1985 - isSetType(place.identifier) ||
1986 - isMapType(place.identifier)
1987 - ) {
1988 - effects.push({
1989 - kind: 'Capture',
1990 - from: place,
1991 - into: lvalue,
1992 - });
1993 - } else {
1994 - effects.push({
1995 - kind: 'Capture',
1996 - from: place,
1997 - into: lvalue,
1998 - });
1983 + const mutateIterator = conditionallyMutateIterator(place);
1984 + if (mutateIterator != null) {
1985 + effects.push(mutateIterator);
1986 + // TODO: should we always push to captures?
1987 captures.push(place);
2000 - effects.push({
2001 - kind: 'MutateTransitiveConditionally',
2002 - value: place,
2003 - });
1988 }
1989 + effects.push({
1990 + kind: 'Capture',
1991 + from: place,
1992 + into: lvalue,
1993 + });
1994 break;
1995 }
1996 case Effect.Freeze: {
@@ -2191,6 +2180,7 @@ function computeEffectsForSignature(
2180 return null;
2181 }
2182 // Build substitutions
2183 + const mutableSpreads = new Set<IdentifierId>();
2184 const substitutions: Map<IdentifierId, Array<Place>> = new Map();
2185 substitutions.set(signature.receiver, [receiver]);
2186 substitutions.set(signature.returns, [lvalue]);
@@ -2208,6 +2198,13 @@ function computeEffectsForSignature(
2198 }
2199 const place = arg.kind === 'Identifier' ? arg : arg.place;
2200 getOrInsertWith(substitutions, signature.rest, () => []).push(place);
2201 +
2202 + if (arg.kind === 'Spread') {
2203 + const mutateIterator = conditionallyMutateIterator(arg.place);
2204 + if (mutateIterator != null) {
2205 + mutableSpreads.add(arg.place.identifier.id);
2206 + }
2207 + }
2208 } else {
2209 const param = params[i];
2210 substitutions.set(param, [arg]);
@@ -2279,6 +2276,12 @@ function computeEffectsForSignature(
2276 case 'Freeze': {
2277 const values = substitutions.get(effect.value.identifier.id) ?? [];
2278 for (const value of values) {
2279 + if (mutableSpreads.has(value.identifier.id)) {
2280 + CompilerError.throwTodo({
2281 + reason: 'Support spread syntax for hook arguments',
2282 + loc: value.loc,
2283 + });
2284 + }
2285 effects.push({kind: 'Freeze', value, reason: effect.reason});
2286 }
2287 break;
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hook-call-freezes-captured-identifier.expect.md
+1 -1
@@ -32,7 +32,7 @@ export const FIXTURE_ENTRYPOINT = {
32 11 | });
33 12 |
34 > 13 | x.value += count;
35 - | ^ InvalidReact: This mutates a variable that React considers immutable (13:13)
35 + | ^ InvalidReact: Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook (13:13)
36 14 | return <Stringify x={x} cb={cb} />;
37 15 | }
38 16 |
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hook-call-freezes-captured-memberexpr.expect.md
+1 -1
@@ -32,7 +32,7 @@ export const FIXTURE_ENTRYPOINT = {
32 11 | });
33 12 |
34 > 13 | x.value += count;
35 - | ^ InvalidReact: This mutates a variable that React considers immutable (13:13)
35 + | ^ InvalidReact: Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook (13:13)
36 14 | return <Stringify x={x} cb={cb} />;
37 15 | }
38 16 |
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-non-imported-reanimated-shared-value-writes.expect.md
+1 -1
@@ -27,7 +27,7 @@ function SomeComponent() {
27 9 | return (
28 10 | <Button
29 > 11 | onPress={() => (sharedVal.value = Math.random())}
30 - | ^^^^^^^^^ InvalidReact: This mutates a variable that React considers immutable. Found mutation of `sharedVal` (11:11)
30 + | ^^^^^^^^^ InvalidReact: Updating a value returned from a hook is not allowed. Consider moving the mutation into the hook where the value is constructed. Found mutation of `sharedVal` (11:11)
31 12 | title="Randomize"
32 13 | />
33 14 | );
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/retry-no-emit.expect.md
+1 -1
@@ -52,7 +52,7 @@ export const FIXTURE_ENTRYPOINT = {
52 ## Logs
53
54 ```
55 -{"kind":"CompileError","fnLoc":{"start":{"line":5,"column":0,"index":163},"end":{"line":13,"column":1,"index":357},"filename":"retry-no-emit.ts"},"detail":{"reason":"This mutates a variable that React considers immutable","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":11,"column":2,"index":320},"end":{"line":11,"column":6,"index":324},"filename":"retry-no-emit.ts","identifierName":"arr2"}}}
55 +{"kind":"CompileError","fnLoc":{"start":{"line":5,"column":0,"index":163},"end":{"line":13,"column":1,"index":357},"filename":"retry-no-emit.ts"},"detail":{"reason":"Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":11,"column":2,"index":320},"end":{"line":11,"column":6,"index":324},"filename":"retry-no-emit.ts","identifierName":"arr2"}}}
56 {"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":7,"column":2,"index":216},"end":{"line":7,"column":36,"index":250},"filename":"retry-no-emit.ts"},"decorations":[{"start":{"line":7,"column":31,"index":245},"end":{"line":7,"column":34,"index":248},"filename":"retry-no-emit.ts","identifierName":"arr"}]}
57 {"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":10,"column":2,"index":274},"end":{"line":10,"column":44,"index":316},"filename":"retry-no-emit.ts"},"decorations":[{"start":{"line":10,"column":25,"index":297},"end":{"line":10,"column":29,"index":301},"filename":"retry-no-emit.ts","identifierName":"arr2"},{"start":{"line":10,"column":25,"index":297},"end":{"line":10,"column":29,"index":301},"filename":"retry-no-emit.ts","identifierName":"arr2"},{"start":{"line":10,"column":35,"index":307},"end":{"line":10,"column":42,"index":314},"filename":"retry-no-emit.ts","identifierName":"propVal"}]}
58 {"kind":"CompileSuccess","fnLoc":{"start":{"line":5,"column":0,"index":163},"end":{"line":13,"column":1,"index":357},"filename":"retry-no-emit.ts"},"fnName":"Foo","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.mutate-frozen-value.expect.md
+1 -1
@@ -19,7 +19,7 @@ function Component({a, b}) {
19 3 | const x = {a};
20 4 | useFreeze(x);
21 > 5 | x.y = true;
22 - | ^ InvalidReact: This mutates a variable that React considers immutable (5:5)
22 + | ^ InvalidReact: Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook (5:5)
23 6 | return <div>error</div>;
24 7 | }
25 8 |
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/retry-no-emit.expect.md
+1 -1
@@ -52,7 +52,7 @@ export const FIXTURE_ENTRYPOINT = {
52 ## Logs
53
54 ```
55 -{"kind":"CompileError","fnLoc":{"start":{"line":5,"column":0,"index":195},"end":{"line":13,"column":1,"index":389},"filename":"retry-no-emit.ts"},"detail":{"reason":"This mutates a variable that React considers immutable","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":11,"column":2,"index":352},"end":{"line":11,"column":6,"index":356},"filename":"retry-no-emit.ts","identifierName":"arr2"}}}
55 +{"kind":"CompileError","fnLoc":{"start":{"line":5,"column":0,"index":195},"end":{"line":13,"column":1,"index":389},"filename":"retry-no-emit.ts"},"detail":{"reason":"Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":11,"column":2,"index":352},"end":{"line":11,"column":6,"index":356},"filename":"retry-no-emit.ts","identifierName":"arr2"}}}
56 {"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":7,"column":2,"index":248},"end":{"line":7,"column":36,"index":282},"filename":"retry-no-emit.ts"},"decorations":[{"start":{"line":7,"column":31,"index":277},"end":{"line":7,"column":34,"index":280},"filename":"retry-no-emit.ts","identifierName":"arr"}]}
57 {"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":10,"column":2,"index":306},"end":{"line":10,"column":44,"index":348},"filename":"retry-no-emit.ts"},"decorations":[{"start":{"line":10,"column":25,"index":329},"end":{"line":10,"column":29,"index":333},"filename":"retry-no-emit.ts","identifierName":"arr2"},{"start":{"line":10,"column":25,"index":329},"end":{"line":10,"column":29,"index":333},"filename":"retry-no-emit.ts","identifierName":"arr2"},{"start":{"line":10,"column":35,"index":339},"end":{"line":10,"column":42,"index":346},"filename":"retry-no-emit.ts","identifierName":"propVal"}]}
58 {"kind":"CompileSuccess","fnLoc":{"start":{"line":5,"column":0,"index":195},"end":{"line":13,"column":1,"index":389},"filename":"retry-no-emit.ts"},"fnName":"Foo","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0}