Facebook/react
1
0
Fork 0
mirror of https://github.com/facebook/react.git synced 2025-11-01 09:12:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

[compiler] Improve error message for mutating hook args/return

Browse Source 
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.
This commit is contained in:
Joe Savona
2025-06-11 13:21:27 -07:00
parent 142272e10a
commit 69cb23bbcd
10 changed files with 71 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts
+10
View File
@@ -1388,6 +1388,16 @@ export enum ValueReason {
*/
JsxCaptured = 'jsx-captured',
/**
* Argument to a hook
*/
HookCaptured = 'hook-captured',
/**
* Return value of a hook
*/
HookReturn = 'hook-return',
/**
* Passed to an effect
*/
compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts
+28
View File
@@ -1302,6 +1302,34 @@ export const DefaultNonmutatingHook = addHook(
calleeEffect: Effect.Read,
hookKind: 'Custom',
returnValueKind: ValueKind.Frozen,
aliasing: {
receiver: makeIdentifierId(0),
params: [],
rest: makeIdentifierId(1),
returns: makeIdentifierId(2),
temporaries: [],
effects: [
// Freeze the arguments
{
kind: 'Freeze',
value: signatureArgument(1),
reason: ValueReason.HookCaptured,
},
// Returns a frozen value
{
kind: 'Create',
into: signatureArgument(2),
value: ValueKind.Frozen,
reason: ValueReason.HookReturn,
},
// May alias any arguments into the return
{
kind: 'Alias',
from: signatureArgument(1),
into: signatureArgument(2),
},
],
},
},
'DefaultNonmutatingHook',
);
compiler/packages/babel-plugin-react-compiler/src/Inference/InferFunctionEffects.ts
+4
View File
@@ -341,6 +341,10 @@ export function getWriteErrorReason(abstractValue: AbstractValue): string {
return "Mutating a value returned from 'useReducer()', which should not be mutated. Use the dispatch function to update instead";
} else if (abstractValue.reason.has(ValueReason.Effect)) {
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()';
} else if (abstractValue.reason.has(ValueReason.HookCaptured)) {
return 'Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook';
} else if (abstractValue.reason.has(ValueReason.HookReturn)) {
return 'Updating a value returned from a hook is not allowed. Consider moving the mutation into the hook where the value is constructed';
} else {
return 'This mutates a variable that React considers immutable';
}
compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts
+23 -20
View File
@@ -1980,28 +1980,17 @@ function computeEffectsForLegacySignature(
break;
}
case Effect.ConditionallyMutateIterator: {
if (
isArrayType(place.identifier) ||
isSetType(place.identifier) ||
isMapType(place.identifier)
) {
effects.push({
kind: 'Capture',
from: place,
into: lvalue,
});
} else {
effects.push({
kind: 'Capture',
from: place,
into: lvalue,
});
const mutateIterator = conditionallyMutateIterator(place);
if (mutateIterator != null) {
effects.push(mutateIterator);
// TODO: should we always push to captures?
captures.push(place);
effects.push({
kind: 'MutateTransitiveConditionally',
value: place,
});
}
effects.push({
kind: 'Capture',
from: place,
into: lvalue,
});
break;
}
case Effect.Freeze: {
@@ -2191,6 +2180,7 @@ function computeEffectsForSignature(
return null;
}
// Build substitutions
const mutableSpreads = new Set<IdentifierId>();
const substitutions: Map<IdentifierId, Array<Place>> = new Map();
substitutions.set(signature.receiver, [receiver]);
substitutions.set(signature.returns, [lvalue]);
@@ -2208,6 +2198,13 @@ function computeEffectsForSignature(
}
const place = arg.kind === 'Identifier' ? arg : arg.place;
getOrInsertWith(substitutions, signature.rest, () => []).push(place);
if (arg.kind === 'Spread') {
const mutateIterator = conditionallyMutateIterator(arg.place);
if (mutateIterator != null) {
mutableSpreads.add(arg.place.identifier.id);
}
}
} else {
const param = params[i];
substitutions.set(param, [arg]);
@@ -2279,6 +2276,12 @@ function computeEffectsForSignature(
case 'Freeze': {
const values = substitutions.get(effect.value.identifier.id) ?? [];
for (const value of values) {
if (mutableSpreads.has(value.identifier.id)) {
CompilerError.throwTodo({
reason: 'Support spread syntax for hook arguments',
loc: value.loc,
});
}
effects.push({kind: 'Freeze', value, reason: effect.reason});
}
break;
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hook-call-freezes-captured-identifier.expect.md
+1 -1
View File
@@ -32,7 +32,7 @@ export const FIXTURE_ENTRYPOINT = {
11 | });
12 |
> 13 | x.value += count;
| ^ InvalidReact: This mutates a variable that React considers immutable (13:13)
| ^ 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)
14 | return <Stringify x={x} cb={cb} />;
15 | }
16 |
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hook-call-freezes-captured-memberexpr.expect.md
+1 -1
View File
@@ -32,7 +32,7 @@ export const FIXTURE_ENTRYPOINT = {
11 | });
12 |
> 13 | x.value += count;
| ^ InvalidReact: This mutates a variable that React considers immutable (13:13)
| ^ 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)
14 | return <Stringify x={x} cb={cb} />;
15 | }
16 |
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-non-imported-reanimated-shared-value-writes.expect.md
+1 -1
View File
@@ -27,7 +27,7 @@ function SomeComponent() {
9 | return (
10 | <Button
> 11 | onPress={() => (sharedVal.value = Math.random())}
| ^^^^^^^^^ InvalidReact: This mutates a variable that React considers immutable. Found mutation of `sharedVal` (11:11)
| ^^^^^^^^^ 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)
12 | title="Randomize"
13 | />
14 | );
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/retry-no-emit.expect.md
+1 -1
View File
@@ -52,7 +52,7 @@ export const FIXTURE_ENTRYPOINT = {
## Logs
```
{"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"}}}
{"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"}}}
{"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"}]}
{"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"}]}
{"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
View File
@@ -19,7 +19,7 @@ function Component({a, b}) {
3 | const x = {a};
4 | useFreeze(x);
> 5 | x.y = true;
| ^ InvalidReact: This mutates a variable that React considers immutable (5:5)
| ^ 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)
6 | return <div>error</div>;
7 | }
8 |
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/retry-no-emit.expect.md
+1 -1
View File
@@ -52,7 +52,7 @@ export const FIXTURE_ENTRYPOINT = {
## Logs
```
{"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"}}}
{"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"}}}
{"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"}]}
{"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"}]}
{"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}