Merge branch 'main' into pr34442

[compiler] Fix false positive hook return mutation error (#34424 )
This was fun. We previously added the MaybeAlias effect in #33984 in order to describe the semantic that an unknown function call _may_ alias its return value in its result, but that we don't know this for sure. We record mutations through MaybeAlias edges when walking backward in the data flow graph, but downgrade them to conditional mutations. See the original PR for full context. That change was sufficient for the original case like ```js const frozen = useContext(); useEffect(() => { frozen.method().property = true; }, [...]); ``` But it wasn't sufficient for cases where the aliasing occured between operands: ```js const dispatch = useDispatch(); <div onClick={(e) => { dispatch(...e.target.value) e.target.value = ...; }} /> ``` Here we would record a `Capture dispatch <- e.target` effect. Then during processing of the `event.target.value = ...` assignment we'd eventually _forward_ from `event` to `dispatch` (along a MaybeAlias edge). But in #33984 I missed that this forward walk also has to downgrade to conditional. In addition to that change, we also have to be a bit more precise about which set of effects we create for alias/capture/maybe-alias. The new logic is a bit clearer, I think: * If the value is frozen, it's an ImmutableCapture edge * If the values are mutable, it's a Capture * If it's a context->context, context->mutable, or mutable->context, count it as MaybeAlias.
2025-09-09 16:29:58 -07:00 · 2025-09-09 14:07:47 -07:00
5 changed files with 369 additions and 276 deletions
--- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts
+++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts
@@ -748,10 +748,14 @@ function applyEffect(
    case 'Alias':
    case 'Capture': {
      CompilerError.invariant(
-        effect.kind === 'Capture' || initialized.has(effect.into.identifier.id),
+        effect.kind === 'Capture' ||
+          effect.kind === 'MaybeAlias' ||
+          initialized.has(effect.into.identifier.id),
        {
-          reason: `Expected destination value to already be initialized within this instruction for Alias effect`,
-          description: `Destination ${printPlace(effect.into)} is not initialized in this instruction`,
+          reason: `Expected destination to already be initialized within this instruction`,
+          description:
+            `Destination ${printPlace(effect.into)} is not initialized in this ` +
+            `instruction for effect ${printAliasingEffect(effect)}`,
          details: [
            {
              kind: 'error',
@@ -767,49 +771,67 @@ function applyEffect(
       * copy-on-write semantics, then we can prune the effect
       */
      const intoKind = state.kind(effect.into).kind;
-      let isMutableDesination: boolean;
+      let destinationType: 'context' | 'mutable' | null = null;
      switch (intoKind) {
-        case ValueKind.Context:
-        case ValueKind.Mutable:
-        case ValueKind.MaybeFrozen: {
-          isMutableDesination = true;
+        case ValueKind.Context: {
+          destinationType = 'context';
          break;
        }
-        default: {
-          isMutableDesination = false;
+        case ValueKind.Mutable:
+        case ValueKind.MaybeFrozen: {
+          destinationType = 'mutable';
          break;
        }
      }
      const fromKind = state.kind(effect.from).kind;
-      let isMutableReferenceType: boolean;
+      let sourceType: 'context' | 'mutable' | 'frozen' | null = null;
      switch (fromKind) {
+        case ValueKind.Context: {
+          sourceType = 'context';
+          break;
+        }
        case ValueKind.Global:
        case ValueKind.Primitive: {
-          isMutableReferenceType = false;
          break;
        }
        case ValueKind.Frozen: {
-          isMutableReferenceType = false;
-          applyEffect(
-            context,
-            state,
-            {
-              kind: 'ImmutableCapture',
-              from: effect.from,
-              into: effect.into,
-            },
-            initialized,
-            effects,
-          );
+          sourceType = 'frozen';
          break;
        }
        default: {
-          isMutableReferenceType = true;
+          sourceType = 'mutable';
          break;
        }
      }
-      if (isMutableDesination && isMutableReferenceType) {
+
+      if (sourceType === 'frozen') {
+        applyEffect(
+          context,
+          state,
+          {
+            kind: 'ImmutableCapture',
+            from: effect.from,
+            into: effect.into,
+          },
+          initialized,
+          effects,
+        );
+      } else if (
+        (sourceType === 'mutable' && destinationType === 'mutable') ||
+        effect.kind === 'MaybeAlias'
+      ) {
        effects.push(effect);
+      } else if (
+        (sourceType === 'context' && destinationType != null) ||
+        (sourceType === 'mutable' && destinationType === 'context')
+      ) {
+        applyEffect(
+          context,
+          state,
+          {kind: 'MaybeAlias', from: effect.from, into: effect.into},
+          initialized,
+          effects,
+        );
      }
      break;
    }
--- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts
+++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts
@@ -779,7 +779,13 @@ class AliasingState {
        if (edge.index >= index) {
          break;
        }
-        queue.push({place: edge.node, transitive, direction: 'forwards', kind});
+        queue.push({
+          place: edge.node,
+          transitive,
+          direction: 'forwards',
+          // Traversing a maybeAlias edge always downgrades to conditional mutation
+          kind: edge.kind === 'maybeAlias' ? MutationKind.Conditional : kind,
+        });
      }
      for (const [alias, when] of node.createdFrom) {
        if (when >= index) {
@@ -807,7 +813,12 @@ class AliasingState {
          if (when >= index) {
            continue;
          }
-          queue.push({place: alias, transitive, direction: 'backwards', kind});
+          queue.push({
+            place: alias,
+            transitive,
+            direction: 'backwards',
+            kind,
+          });
        }
        /**
         * MaybeAlias indicates potential data flow from unknown function calls,
--- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects.ts
+++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects.ts
@@ -5,55 +5,40 @@
 * LICENSE file in the root directory of this source tree.
 */

-import {effect} from 'zod';
+import {TypeOf} from 'zod';
 import {CompilerError, Effect, ErrorSeverity, SourceLocation} from '..';
 import {ErrorCategory} from '../CompilerError';
 import {
  ArrayExpression,
-  BasicBlock,
  BlockId,
-  Identifier,
  FunctionExpression,
  HIRFunction,
  IdentifierId,
-  Instruction,
+  InstructionValue,
  Place,
  isSetStateType,
  isUseEffectHookType,
-  isUseStateType,
-  IdentifierName,
-  GeneratedSource,
 } from '../HIR';
-import {printInstruction} from '../HIR/PrintHIR';
+import {printInstruction, printPlace} from '../HIR/PrintHIR';
 import {
+  eachInstructionValueOperand,
  eachInstructionOperand,
  eachTerminalOperand,
  eachInstructionLValue,
-  eachPatternOperand,
 } from '../HIR/visitors';
 import {isMutable} from '../ReactiveScopes/InferReactiveScopeVariables';
 import {assertExhaustive} from '../Utils/utils';

 type SetStateCall = {
  loc: SourceLocation;
-  invalidDeps: Map<Identifier, Place[]> | undefined;
-  setStateId: IdentifierId;
+  propsSources: Place[] | undefined; // undefined means state-derived, defined means props-derived
 };
 type TypeOfValue = 'ignored' | 'fromProps' | 'fromState' | 'fromPropsOrState';

 type DerivationMetadata = {
-  typeOfValue: TypeOfValue;
-  // TODO: Rename to place
  identifierPlace: Place;
  sources: Place[];
-};
-
-// TODO: This needs refining
-type ErrorMetadata = {
-  errorType: 'HoistState' | 'CalculateInRender';
-  propInfo: string | undefined;
-  loc: SourceLocation;
-  setStateId: IdentifierId;
+  typeOfValue: TypeOfValue;
 };

 function joinValue(
@@ -66,6 +51,22 @@ function joinValue(
  return 'fromPropsOrState';
 }

+function propagateDerivation(
+  dest: Place,
+  source: Place | undefined,
+  derivedFromProps: Map<IdentifierId, Place>,
+) {
+  if (source === undefined) {
+    return;
+  }
+
+  if (source.identifier.name?.kind === 'promoted') {
+    derivedFromProps.set(dest.identifier.id, dest);
+  } else {
+    derivedFromProps.set(dest.identifier.id, source);
+  }
+}
+
 function updateDerivationMetadata(
  target: Place,
  sources: DerivationMetadata[],
@@ -80,7 +81,7 @@ function updateDerivationMetadata(

  for (const source of sources) {
    // If the identifier of the source is a promoted identifier, then
-    // we should set the target as the source.
+    // we should set the source as the first named identifier.
    if (source.identifierPlace.identifier.name?.kind === 'promoted') {
      newValue.sources.push(target);
    } else {
@@ -90,133 +91,6 @@ function updateDerivationMetadata(
  derivedTuple.set(target.identifier.id, newValue);
 }

-function parseInstr(
-  instr: Instruction,
-  derivedTuple: Map<IdentifierId, DerivationMetadata>,
-  setStateCalls: Map<string, Place>,
-) {
-  // console.log(printInstruction(instr));
-  // console.log(instr);
-  let typeOfValue: TypeOfValue = 'ignored';
-
-  // If the instruction is destructuring a useState hook call
-  if (
-    instr.value.kind === 'Destructure' &&
-    instr.value.lvalue.pattern.kind === 'ArrayPattern' &&
-    isUseStateType(instr.value.value.identifier)
-  ) {
-    const value = instr.value.lvalue.pattern.items[0];
-    if (value.kind === 'Identifier') {
-      derivedTuple.set(value.identifier.id, {
-        identifierPlace: value,
-        sources: [value],
-        typeOfValue: 'fromState',
-      });
-    }
-  }
-
-  // If the instruction is calling a setState
-  if (
-    instr.value.kind === 'CallExpression' &&
-    isSetStateType(instr.value.callee.identifier) &&
-    instr.value.args.length === 1 &&
-    instr.value.args[0].kind === 'Identifier' &&
-    instr.value.callee.loc !== GeneratedSource &&
-    instr.value.callee.loc.identifierName !== undefined &&
-    instr.value.callee.loc.identifierName !== null
-  ) {
-    setStateCalls.set(
-      instr.value.callee.loc.identifierName,
-      instr.value.callee,
-    );
-  }
-
-  let sources: DerivationMetadata[] = [];
-  for (const operand of eachInstructionOperand(instr)) {
-    const opSource = derivedTuple.get(operand.identifier.id);
-    if (opSource === undefined) {
-      continue;
-    }
-
-    typeOfValue = joinValue(typeOfValue, opSource.typeOfValue);
-    sources.push(opSource);
-  }
-
-  if (typeOfValue !== 'ignored') {
-    for (const lvalue of eachInstructionLValue(instr)) {
-      updateDerivationMetadata(lvalue, sources, typeOfValue, derivedTuple);
-    }
-
-    for (const operand of eachInstructionOperand(instr)) {
-      switch (operand.effect) {
-        case Effect.Capture:
-        case Effect.Store:
-        case Effect.ConditionallyMutate:
-        case Effect.ConditionallyMutateIterator:
-        case Effect.Mutate: {
-          if (isMutable(instr, operand)) {
-            updateDerivationMetadata(
-              operand,
-              sources,
-              typeOfValue,
-              derivedTuple,
-            );
-          }
-          break;
-        }
-        case Effect.Freeze:
-        case Effect.Read: {
-          // no-op
-          break;
-        }
-        case Effect.Unknown: {
-          CompilerError.invariant(false, {
-            reason: 'Unexpected unknown effect',
-            description: null,
-            loc: operand.loc,
-            suggestions: null,
-          });
-        }
-        default: {
-          assertExhaustive(
-            operand.effect,
-            `Unexpected effect kind \`${operand.effect}\``,
-          );
-        }
-      }
-    }
-  }
-}
-
-function parseBlockPhi(
-  block: BasicBlock,
-  derivedTuple: Map<IdentifierId, DerivationMetadata>,
-) {
-  for (const phi of block.phis) {
-    for (const operand of phi.operands.values()) {
-      const source = derivedTuple.get(operand.identifier.id);
-      if (source !== undefined && source.typeOfValue === 'fromProps') {
-        if (
-          source.identifierPlace.identifier.name === null ||
-          source.identifierPlace.identifier.name?.kind === 'promoted'
-        ) {
-          derivedTuple.set(phi.place.identifier.id, {
-            identifierPlace: phi.place,
-            sources: [phi.place],
-            typeOfValue: 'fromProps',
-          });
-        } else {
-          derivedTuple.set(phi.place.identifier.id, {
-            identifierPlace: phi.place,
-            sources: source.sources,
-            typeOfValue: 'fromProps',
-          });
-        }
-      }
-    }
-  }
-}
-
 /**
 * Validates that useEffect is not used for derived computations which could/should
 * be performed in render.
@@ -244,15 +118,17 @@ export function validateNoDerivedComputationsInEffects(fn: HIRFunction): void {
  const candidateDependencies: Map<IdentifierId, ArrayExpression> = new Map();
  const functions: Map<IdentifierId, FunctionExpression> = new Map();
  const locals: Map<IdentifierId, IdentifierId> = new Map();
+
+  // MY take on this
+  const valueToType: Map<IdentifierId, TypeOfValue> = new Map();
+  const valueToSourceProps: Map<IdentifierId, Set<Place>> = new Map();
+  const valueToSourceStates: Map<IdentifierId, Set<Place>> = new Map();
+  const valueToSources: Map<IdentifierId, Set<Place>> = new Map();
+
+  // Sources are still probably not correct
  const derivedTuple: Map<IdentifierId, DerivationMetadata> = new Map();

-  // Investigating
-  const effectSetStates: Map<string, Place> = new Map();
-  const setStateCalls: Map<string, Place> = new Map();
-
-  // let shouldCalculateInRender: boolean = true;
-
-  const errors: ErrorMetadata[] = [];
+  const errors = new CompilerError();

  if (fn.fnType === 'Hook') {
    for (const param of fn.params) {
@@ -276,26 +152,104 @@ export function validateNoDerivedComputationsInEffects(fn: HIRFunction): void {
  }

  for (const block of fn.body.blocks.values()) {
-    parseBlockPhi(block, derivedTuple);
+    for (const phi of block.phis) {
+      for (const operand of phi.operands.values()) {
+        const source = derivedTuple.get(operand.identifier.id);
+        if (source !== undefined && source.typeOfValue === 'fromProps') {
+          if (
+            source.identifierPlace.identifier.name === null ||
+            source.identifierPlace.identifier.name?.kind === 'promoted'
+          ) {
+            derivedTuple.set(phi.place.identifier.id, {
+              identifierPlace: phi.place,
+              sources: [phi.place],
+              typeOfValue: 'fromProps',
+            });
+          } else {
+            derivedTuple.set(phi.place.identifier.id, {
+              identifierPlace: phi.place,
+              sources: source.sources,
+              typeOfValue: 'fromProps',
+            });
+          }
+        }
+      }
+    }

    for (const instr of block.instructions) {
      const {lvalue, value} = instr;

-      parseInstr(instr, derivedTuple, setStateCalls);
+      // This needs to be repeated "recursively" on FunctionExpressions
+      // HERE >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
+      // DERIVATION LOGIC-----------------------------------------------------
+      console.log('instr', printInstruction(instr));
+      console.log('instr', instr);
+      // console.log('instr lValue', instr.lvalue);

-      /*
-       * Special case for function expressions, we need to parse nested instructions
-       * TODO: Can there be more recursive levels?
-       */
-      if (value.kind === 'FunctionExpression') {
-        for (const [, block] of value.loweredFunc.func.body.blocks) {
-          for (const instr of block.instructions) {
-            parseInstr(instr, derivedTuple, setStateCalls);
+      let typeOfValue: TypeOfValue = 'ignored';
+
+      // TODO: Add handling for state derived props
+      let sources: DerivationMetadata[] = [];
+      for (const operand of eachInstructionValueOperand(value)) {
+        const opSource = derivedTuple.get(operand.identifier.id);
+        if (opSource === undefined) {
+          continue;
+        }
+
+        typeOfValue = joinValue(typeOfValue, opSource.typeOfValue);
+        sources.push(opSource);
+      }
+
+      // TODO: Add handling for state derived props
+      if (typeOfValue !== 'ignored') {
+        for (const lvalue of eachInstructionLValue(instr)) {
+          updateDerivationMetadata(lvalue, sources, typeOfValue, derivedTuple);
+        }
+
+        for (const operand of eachInstructionValueOperand(value)) {
+          switch (operand.effect) {
+            case Effect.Capture:
+            case Effect.Store:
+            case Effect.ConditionallyMutate:
+            case Effect.ConditionallyMutateIterator:
+            case Effect.Mutate: {
+              if (isMutable(instr, operand)) {
+                updateDerivationMetadata(
+                  operand,
+                  sources,
+                  typeOfValue,
+                  derivedTuple,
+                );
+              }
+              break;
+            }
+            case Effect.Freeze:
+            case Effect.Read: {
+              // no-op
+              break;
+            }
+            case Effect.Unknown: {
+              CompilerError.invariant(false, {
+                reason: 'Unexpected unknown effect',
+                description: null,
+                loc: operand.loc,
+                suggestions: null,
+              });
+            }
+            default: {
+              assertExhaustive(
+                operand.effect,
+                `Unexpected effect kind \`${operand.effect}\``,
+              );
+            }
          }
        }
      }
+      console.log('derivedTuple', derivedTuple);
+      // HERE >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

-      // Maybe this should run for every instruction being parsed
+      // console.log('derivedTuple', derivedTuple);
+      // DERIVATION LOGIC-----------------------------------------------------
      if (value.kind === 'LoadLocal') {
        locals.set(lvalue.identifier.id, value.place.identifier.id);
      } else if (value.kind === 'ArrayExpression') {
@@ -309,6 +263,7 @@ export function validateNoDerivedComputationsInEffects(fn: HIRFunction): void {
        const callee =
          value.kind === 'CallExpression' ? value.callee : value.property;

+        // This is a useEffect hook
        if (
          isUseEffectHookType(callee.identifier) &&
          value.args.length === 2 &&
@@ -341,7 +296,6 @@ export function validateNoDerivedComputationsInEffects(fn: HIRFunction): void {
              effectFunction.loweredFunc.func,
              dependencies,
              derivedTuple,
-              effectSetStates,
              errors,
            );
          }
@@ -349,21 +303,8 @@ export function validateNoDerivedComputationsInEffects(fn: HIRFunction): void {
      }
    }
  }
-
-  console.log('setStateCalls: ', setStateCalls);
-  console.log('effectSetStates: ', effectSetStates);
-  const throwableErrors = new CompilerError();
-  for (const error of errors) {
-    throwableErrors.push({
-      reason: `You may not need an effect. Values derived from state should be calculated in render, not in an effect. `,
-      description: `You are using a value derived from props${error.propInfo} to update local state in an effect.`,
-      severity: ErrorSeverity.InvalidReact,
-      loc: error.loc,
-    });
-  }
-
-  if (throwableErrors.hasAnyErrors()) {
-    throw throwableErrors;
+  if (errors.hasAnyErrors()) {
+    throw errors;
  }
 }

@@ -371,13 +312,8 @@ function validateEffect(
  effectFunction: HIRFunction,
  effectDeps: Array<IdentifierId>,
  derivedTuple: Map<IdentifierId, DerivationMetadata>,
-  effectSetStates: Map<string, Place>,
-  errors: ErrorMetadata[],
+  errors: CompilerError,
 ): void {
-  /*
-   * TODO: This makes it so we only capture single line useEffects.
-   * We should be able to capture multiline as well
-   */
  for (const operand of effectFunction.context) {
    if (isSetStateType(operand.identifier)) {
      continue;
@@ -387,6 +323,7 @@ function validateEffect(
      continue;
    } else {
      // Captured something other than the effect dep or setState
+      console.log('early return 1');
      return;
    }
  }
@@ -413,18 +350,17 @@ function validateEffect(
  const seenBlocks: Set<BlockId> = new Set();
  // This variable is suspicious maybe we don't need it?
  const values: Map<IdentifierId, Array<IdentifierId>> = new Map();
-  const effectInvalidlyDerived: Map<IdentifierId, DerivationMetadata> =
-    new Map();
+  const effectInvalidlyDerived: Map<IdentifierId, Place[]> = new Map();

  for (const dep of effectDeps) {
    values.set(dep, [dep]);
    const depMetadata = derivedTuple.get(dep);
    if (depMetadata !== undefined) {
-      effectInvalidlyDerived.set(dep, depMetadata);
+      effectInvalidlyDerived.set(dep, depMetadata.sources);
    }
  }

-  const setStateCallsInEffect: Array<SetStateCall> = [];
+  const setStateCalls: Array<SetStateCall> = [];
  for (const block of effectFunction.body.blocks.values()) {
    for (const pred of block.preds) {
      if (!seenBlocks.has(pred)) {
@@ -433,23 +369,33 @@ function validateEffect(
      }
    }

-    parseBlockPhi(block, effectInvalidlyDerived);
+    // TODO: This might need editing
+    for (const phi of block.phis) {
+      const aggregateDeps: Set<IdentifierId> = new Set();
+      let propsSources: Place[] | null = null;
+
+      for (const operand of phi.operands.values()) {
+        const deps = values.get(operand.identifier.id);
+        if (deps != null) {
+          for (const dep of deps) {
+            aggregateDeps.add(dep);
+          }
+        }
+        const sources = effectInvalidlyDerived.get(operand.identifier.id);
+        if (sources != null) {
+          propsSources = sources;
+        }
+      }
+
+      if (aggregateDeps.size !== 0) {
+        values.set(phi.place.identifier.id, Array.from(aggregateDeps));
+      }
+      if (propsSources != null) {
+        effectInvalidlyDerived.set(phi.place.identifier.id, propsSources);
+      }
+    }

    for (const instr of block.instructions) {
-      if (
-        instr.value.kind === 'CallExpression' &&
-        isSetStateType(instr.value.callee.identifier) &&
-        instr.value.args.length === 1 &&
-        instr.value.args[0].kind === 'Identifier' &&
-        instr.value.callee.loc !== GeneratedSource &&
-        instr.value.callee.loc.identifierName !== undefined &&
-        instr.value.callee.loc.identifierName !== null
-      ) {
-        effectSetStates.set(
-          instr.value.callee.loc.identifierName,
-          instr.value.callee,
-        );
-      }
      switch (instr.value.kind) {
        case 'Primitive':
        case 'JSXText':
@@ -488,24 +434,32 @@ function validateEffect(
            instr.value.args.length === 1 &&
            instr.value.args[0].kind === 'Identifier'
          ) {
-            const propSources = derivedTuple.get(
-              instr.value.args[0].identifier.id,
-            );
+            const deps = values.get(instr.value.args[0].identifier.id);
+            console.log('deps', deps);
+            if (deps != null && new Set(deps).size === effectDeps.length) {
+              // console.log('setState arg', instr.value.args[0].identifier.id);
+              // console.log('effectInvalidlyDerived', effectInvalidlyDerived);
+              // console.log('derivedTuple', derivedTuple);
+              const propSources = derivedTuple.get(
+                instr.value.args[0].identifier.id,
+              );

-            if (propSources !== undefined) {
-              setStateCallsInEffect.push({
-                loc: instr.value.callee.loc,
-                setStateId: instr.value.callee.identifier.id,
-                invalidDeps: new Map([
-                  [instr.value.args[0].identifier, propSources.sources],
-                ]),
-              });
+              console.log('Final reference', propSources);
+              if (propSources !== undefined) {
+                setStateCalls.push({
+                  loc: instr.value.callee.loc,
+                  propsSources: propSources.sources,
+                });
+              } else {
+                setStateCalls.push({
+                  loc: instr.value.callee.loc,
+                  propsSources: undefined,
+                });
+              }
            } else {
-              setStateCallsInEffect.push({
-                loc: instr.value.callee.loc,
-                setStateId: instr.value.callee.identifier.id,
-                invalidDeps: undefined,
-              });
+              // doesn't depend on all deps
+              console.log('early return 3');
+              return;
            }
          }
          break;
@@ -516,7 +470,6 @@ function validateEffect(
        }
      }
    }
-
    for (const operand of eachTerminalOperand(block.terminal)) {
      if (values.has(operand.identifier.id)) {
        return;
@@ -525,36 +478,31 @@ function validateEffect(
    seenBlocks.add(block.id);
  }

-  // need to track if the setState call has been used elsewhere
-  // if it is then the solution should be to lift the state up to the parent component
-  // if not the solution should be to calculate the value in rende
-  //
-  // If the same setState is used both inside and outside the effect
-
-  for (const call of setStateCallsInEffect) {
-    if (call.invalidDeps != null) {
-      let propNames = '';
-      for (const [, places] of call.invalidDeps.entries()) {
-        const placeNames = places
-          .map(place => place.identifier.name?.value)
-          .join(', ');
-        propNames += `[${placeNames}], `;
-      }
-      propNames = propNames.slice(0, -2);
-      const propInfo = propNames ? ` (from props '${propNames}')` : '';
+  console.log('setStateCalls', setStateCalls);
+  for (const call of setStateCalls) {
+    if (call.propsSources != null) {
+      const propNames = call.propsSources
+        .map(place => place.identifier.name?.value)
+        .join(', ');
+      const propInfo = propNames != null ? ` (from props '${propNames}')` : '';

      errors.push({
-        errorType: 'HoistState',
-        propInfo: propInfo,
+        reason: `Consider lifting state up to the parent component to make this a controlled component. (https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes)`,
+        description: `You are using props${propInfo} to update local state in an effect.`,
+        severity: ErrorSeverity.InvalidReact,
        loc: call.loc,
-        setStateId: call.setStateId,
+        suggestions: null,
      });
    } else {
      errors.push({
-        errorType: 'CalculateInRender',
-        propInfo: undefined,
+        reason:
+          'You may not need this effect. Values derived from state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)',
+        description:
+          'This effect updates state based on other state values. ' +
+          'Consider calculating this value directly during render',
+        severity: ErrorSeverity.InvalidReact,
        loc: call.loc,
-        setStateId: call.setStateId,
+        suggestions: null,
      });
    }
  }
--- a/compiler/packages/babel-plugin-react-compiler/src/tests/fixtures/compiler/repro-dispatch-spread-event-marks-event-frozen.expect.md
+++ b/compiler/packages/babel-plugin-react-compiler/src/tests/fixtures/compiler/repro-dispatch-spread-event-marks-event-frozen.expect.md
@@ -0,0 +1,82 @@
+
+## Input
+
+```javascript
+// @compilationMode:"infer"
+function Component() {
+  const dispatch = useDispatch();
+  // const [state, setState] = useState(0);
+
+  return (
+    <div>
+      <input
+        type="file"
+        onChange={event => {
+          dispatch(...event.target);
+          event.target.value = '';
+        }}
+      />
+    </div>
+  );
+}
+
+function useDispatch() {
+  'use no memo';
+  // skip compilation to make it easier to debug the above function
+  return (...values) => {
+    console.log(...values);
+  };
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{}],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime"; // @compilationMode:"infer"
+function Component() {
+  const $ = _c(2);
+  const dispatch = useDispatch();
+  let t0;
+  if ($[0] !== dispatch) {
+    t0 = (
+      <div>
+        <input
+          type="file"
+          onChange={(event) => {
+            dispatch(...event.target);
+            event.target.value = "";
+          }}
+        />
+      </div>
+    );
+    $[0] = dispatch;
+    $[1] = t0;
+  } else {
+    t0 = $[1];
+  }
+  return t0;
+}
+
+function useDispatch() {
+  "use no memo";
+  // skip compilation to make it easier to debug the above function
+  return (...values) => {
+    console.log(...values);
+  };
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{}],
+};
+
+```
+      
+### Eval output
+(kind: ok) <div><input type="file"></div>
--- a/compiler/packages/babel-plugin-react-compiler/src/tests/fixtures/compiler/repro-dispatch-spread-event-marks-event-frozen.js
+++ b/compiler/packages/babel-plugin-react-compiler/src/tests/fixtures/compiler/repro-dispatch-spread-event-marks-event-frozen.js
@@ -0,0 +1,30 @@
+// @compilationMode:"infer"
+function Component() {
+  const dispatch = useDispatch();
+  // const [state, setState] = useState(0);
+
+  return (
+    <div>
+      <input
+        type="file"
+        onChange={event => {
+          dispatch(...event.target);
+          event.target.value = '';
+        }}
+      />
+    </div>
+  );
+}
+
+function useDispatch() {
+  'use no memo';
+  // skip compilation to make it easier to debug the above function
+  return (...values) => {
+    console.log(...values);
+  };
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{}],
+};