diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts index f250361e3b2..356bc8af08b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts @@ -26,6 +26,7 @@ import { Type, ValueKind, ValueReason, + getHookKind, isArrayType, isMutableEffect, isObjectType, @@ -48,7 +49,6 @@ import { eachTerminalSuccessor, } from '../HIR/visitors'; import {assertExhaustive} from '../Utils/utils'; -import {isEffectHook} from '../Validation/ValidateMemoizedEffectDependencies'; const UndefinedValue: InstructionValue = { kind: 'Primitive', @@ -1151,7 +1151,7 @@ function inferBlock( ); functionEffects.push( ...propEffects.filter( - propEffect => propEffect.kind !== 'GlobalMutation', + effect => !isEffectSafeOutsideRender(effect), ), ); } @@ -1330,7 +1330,7 @@ function inferBlock( context: new Set(), }; let hasCaptureArgument = false; - let isUseEffect = isEffectHook(instrValue.callee.identifier); + let isHook = getHookKind(env, instrValue.callee.identifier) != null; for (let i = 0; i < instrValue.args.length; i++) { const argumentEffects: Array = []; const arg = instrValue.args[i]; @@ -1356,8 +1356,7 @@ function inferBlock( */ functionEffects.push( ...argumentEffects.filter( - argEffect => - !isUseEffect || i !== 0 || argEffect.kind !== 'GlobalMutation', + argEffect => !isHook || !isEffectSafeOutsideRender(argEffect), ), ); hasCaptureArgument ||= place.effect === Effect.Capture; @@ -1455,7 +1454,7 @@ function inferBlock( const effects = signature !== null ? getFunctionEffects(instrValue, signature) : null; let hasCaptureArgument = false; - let isUseEffect = isEffectHook(instrValue.property.identifier); + let isHook = getHookKind(env, instrValue.property.identifier) != null; for (let i = 0; i < instrValue.args.length; i++) { const argumentEffects: Array = []; const arg = instrValue.args[i]; @@ -1485,8 +1484,7 @@ function inferBlock( */ functionEffects.push( ...argumentEffects.filter( - argEffect => - !isUseEffect || i !== 0 || argEffect.kind !== 'GlobalMutation', + argEffect => !isHook || !isEffectSafeOutsideRender(argEffect), ), ); hasCaptureArgument ||= place.effect === Effect.Capture; @@ -2010,11 +2008,15 @@ function inferBlock( } else { effect = Effect.Read; } + const propEffects: Array = []; state.referenceAndRecordEffects( operand, effect, ValueReason.Other, - functionEffects, + propEffects, + ); + functionEffects.push( + ...propEffects.filter(effect => !isEffectSafeOutsideRender(effect)), ); } } @@ -2128,6 +2130,10 @@ function areArgumentsImmutableAndNonMutating( return true; } +function isEffectSafeOutsideRender(effect: FunctionEffect): boolean { + return effect.kind === 'GlobalMutation'; +} + function getWriteErrorReason(abstractValue: AbstractValue): string { if (abstractValue.reason.has(ValueReason.Global)) { return 'Writing to a variable defined outside a component or hook is not allowed. Consider using an effect'; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.reassign-global-fn-arg.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.reassign-global-fn-arg.expect.md new file mode 100644 index 00000000000..235e663be97 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.reassign-global-fn-arg.expect.md @@ -0,0 +1,36 @@ + +## Input + +```javascript +let b = 1; + +export default function MyApp() { + const fn = () => { + b = 2; + }; + return foo(fn); +} + +function foo(fn) {} + +export const FIXTURE_ENTRYPOINT = { + fn: MyApp, + params: [], +}; + +``` + + +## Error + +``` + 3 | export default function MyApp() { + 4 | const fn = () => { +> 5 | b = 2; + | ^ InvalidReact: Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (5:5) + 6 | }; + 7 | return foo(fn); + 8 | } +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.reassign-global-fn-arg.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.reassign-global-fn-arg.js new file mode 100644 index 00000000000..2ef634b470d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.reassign-global-fn-arg.js @@ -0,0 +1,15 @@ +let b = 1; + +export default function MyApp() { + const fn = () => { + b = 2; + }; + return foo(fn); +} + +function foo(fn) {} + +export const FIXTURE_ENTRYPOINT = { + fn: MyApp, + params: [], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-global-hook-arg.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-global-hook-arg.expect.md new file mode 100644 index 00000000000..70ff08be60f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-global-hook-arg.expect.md @@ -0,0 +1,46 @@ + +## Input + +```javascript +let b = 1; + +export default function MyApp() { + const fn = () => { + b = 2; + }; + return useFoo(fn); +} + +function useFoo(fn) {} + +export const FIXTURE_ENTRYPOINT = { + fn: MyApp, + params: [], +}; + +``` + +## Code + +```javascript +let b = 1; + +export default function MyApp() { + const fn = _temp; + return useFoo(fn); +} +function _temp() { + b = 2; +} + +function useFoo(fn) {} + +export const FIXTURE_ENTRYPOINT = { + fn: MyApp, + params: [], +}; + +``` + +### Eval output +(kind: ok) \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-global-hook-arg.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-global-hook-arg.js new file mode 100644 index 00000000000..f584792febf --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-global-hook-arg.js @@ -0,0 +1,15 @@ +let b = 1; + +export default function MyApp() { + const fn = () => { + b = 2; + }; + return useFoo(fn); +} + +function useFoo(fn) {} + +export const FIXTURE_ENTRYPOINT = { + fn: MyApp, + params: [], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-global-return.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-global-return.expect.md new file mode 100644 index 00000000000..9e7ba639e2c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-global-return.expect.md @@ -0,0 +1,42 @@ + +## Input + +```javascript +let b = 1; + +export default function useMyHook() { + const fn = () => { + b = 2; + }; + return fn; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useMyHook, + params: [], +}; + +``` + +## Code + +```javascript +let b = 1; + +export default function useMyHook() { + const fn = _temp; + return fn; +} +function _temp() { + b = 2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useMyHook, + params: [], +}; + +``` + +### Eval output +(kind: ok) "[[ function params=0 ]]" \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-global-return.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-global-return.js new file mode 100644 index 00000000000..abbf1550792 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-global-return.js @@ -0,0 +1,13 @@ +let b = 1; + +export default function useMyHook() { + const fn = () => { + b = 2; + }; + return fn; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useMyHook, + params: [], +};