diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts index 252721765ae..33c978c1459 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -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 */ diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts index e47d561231b..eb80e5c59d2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts @@ -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', ); diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferFunctionEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferFunctionEffects.ts index 4a278850956..9b347ebb6c4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferFunctionEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferFunctionEffects.ts @@ -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'; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts index f0ecb675460..521f55026ec 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -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(); const substitutions: Map> = 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; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hook-call-freezes-captured-identifier.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hook-call-freezes-captured-identifier.expect.md index 7babe57b000..5e0a9886272 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hook-call-freezes-captured-identifier.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hook-call-freezes-captured-identifier.expect.md @@ -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 ; 15 | } 16 | diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hook-call-freezes-captured-memberexpr.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hook-call-freezes-captured-memberexpr.expect.md index fcc47ddc2b1..c5af59d6424 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hook-call-freezes-captured-memberexpr.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hook-call-freezes-captured-memberexpr.expect.md @@ -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 ; 15 | } 16 | diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-non-imported-reanimated-shared-value-writes.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-non-imported-reanimated-shared-value-writes.expect.md index d3bb7f41362..ce278dda621 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-non-imported-reanimated-shared-value-writes.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-non-imported-reanimated-shared-value-writes.expect.md @@ -27,7 +27,7 @@ function SomeComponent() { 9 | return ( 10 |