[Compiler] Improve error for calculate in render useEffect validation

Summary:
Change error and update snapshots

The error now mentions what values are causing the issue which should provide better context on how to fix the issue

Author: jorge-cab

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects.ts

@@ -5,7 +5,7 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import {CompilerError, Effect} from '..';
+import {CompilerDiagnostic, CompilerError, Effect} from '..';
 import {ErrorCategory} from '../CompilerError';
 import {
   BlockId,
@@ -111,8 +111,10 @@ export function validateNoDerivedComputationsInEffects(fn: HIRFunction): void {
       const modifiedContext = recordInstructionDerivations(instr, context);
 
       if (needsFixPointItr && modifiedContext) {
-        // If the derivation cache was modified, we need to re-run the fixpoint
-        // iteration to ensure that all derived values are properly recorded.
+        /*
+         * If the derivation cache was modified, we need to re-run the fixpoint
+         * iteration to ensure that all derived values are properly recorded.
+         */
         recordPhiDerivations(block, context);
       }
     }
@@ -140,8 +142,10 @@ function recordPhiDerivations(
         operand.identifier.id,
       );
 
-      // if we don't have metadata for the operand, we need to re-run the fixpoint
-      // if a future instruction modifies the derivation cache
+      /*
+       * if we don't have metadata for the operand, we need to re-run the fixpoint
+       * if a future instruction modifies the derivation cache
+       */
       if (operandMetadata === undefined) {
         needsFixPointItr = true;
         continue;
@@ -343,6 +347,7 @@ function validateEffect(
     value: CallExpression;
     loc: SourceLocation;
     sourceIds: Set<IdentifierId>;
+    typeOfValue: TypeOfValue;
   }> = [];
 
   const globals: Set<IdentifierId> = new Set();
@@ -388,6 +393,7 @@ function validateEffect(
             value: instr.value,
             loc: instr.value.callee.loc,
             sourceIds: argMetadata.sourcesIds,
+            typeOfValue: argMetadata.typeOfValue,
           });
         }
       } else if (instr.value.kind === 'CallExpression') {
@@ -429,14 +435,36 @@ function validateEffect(
           .length -
           1
     ) {
-      context.errors.push({
-        category: ErrorCategory.EffectDerivationsOfState,
-        reason:
-          'Values derived from props and 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: null,
-        loc: derivedSetStateCall.value.callee.loc,
-        suggestions: null,
-      });
+      const derivedDepsStr = Array.from(derivedSetStateCall.sourceIds)
+        .map(sourceId => {
+          const sourceMetadata = context.derivationCache.get(sourceId);
+          return sourceMetadata?.place.identifier.name?.value;
+        })
+        .filter(Boolean)
+        .join(', ');
+
+      let description;
+
+      if (derivedSetStateCall.typeOfValue === 'fromProps') {
+        description = `From props: [${derivedDepsStr}]`;
+      } else if (derivedSetStateCall.typeOfValue === 'fromState') {
+        description = `From local state: [${derivedDepsStr}]`;
+      } else {
+        description = `From props and local state: [${derivedDepsStr}]`;
+      }
+
+      context.errors.pushDiagnostic(
+        CompilerDiagnostic.create({
+          description: `Derived values (${description}) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user`,
+          category: ErrorCategory.EffectDerivationsOfState,
+          reason:
+            'You might not need an effect. Derive values in render, not effects.',
+        }).withDetails({
+          kind: 'error',
+          loc: derivedSetStateCall.value.callee.loc,
+          message: 'This should be computed during render, not in an effect',
+        }),
+      );
     }
   }
 }

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-conditionally-in-effect.expect.md

@@ -32,13 +32,15 @@ export const FIXTURE_ENTRYPOINT = {
 ```
 Found 1 error:
 
-Error: Values derived from props and 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)
+Error: You might not need an effect. Derive values in render, not effects.
+
+Derived values (From props: [value]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
 
 error.derived-state-conditionally-in-effect.ts:9:6
    7 |   useEffect(() => {
    8 |     if (enabled) {
 >  9 |       setLocalValue(value);
-     |       ^^^^^^^^^^^^^ Values derived from props and 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)
+     |       ^^^^^^^^^^^^^ This should be computed during render, not in an effect
   10 |     } else {
   11 |       setLocalValue('disabled');
   12 |     }

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-default-props.expect.md

@@ -29,13 +29,15 @@ export const FIXTURE_ENTRYPOINT = {
 ```
 Found 1 error:
 
-Error: Values derived from props and 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)
+Error: You might not need an effect. Derive values in render, not effects.
+
+Derived values (From props: [input]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
 
 error.derived-state-from-default-props.ts:9:4
    7 |
    8 |   useEffect(() => {
 >  9 |     setCurrInput(input + localConst);
-     |     ^^^^^^^^^^^^ Values derived from props and 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)
+     |     ^^^^^^^^^^^^ This should be computed during render, not in an effect
   10 |   }, [input, localConst]);
   11 |
   12 |   return <div>{currInput}</div>;

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-local-state-in-effect.expect.md

@@ -26,13 +26,15 @@ function Component({shouldChange}) {
 ```
 Found 1 error:
 
-Error: Values derived from props and 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)
+Error: You might not need an effect. Derive values in render, not effects.
+
+Derived values (From local state: [count]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
 
 error.derived-state-from-local-state-in-effect.ts:10:6
    8 |   useEffect(() => {
    9 |     if (shouldChange) {
 > 10 |       setCount(count + 1);
-     |       ^^^^^^^^ Values derived from props and 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)
+     |       ^^^^^^^^ This should be computed during render, not in an effect
   11 |     }
   12 |   }, [count]);
   13 |

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-local-state-and-component-scope.expect.md

@@ -36,13 +36,15 @@ export const FIXTURE_ENTRYPOINT = {
 ```
 Found 1 error:
 
-Error: Values derived from props and 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)
+Error: You might not need an effect. Derive values in render, not effects.
+
+Derived values (From props and local state: [firstName, lastName]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
 
 error.derived-state-from-prop-local-state-and-component-scope.ts:11:4
    9 |
   10 |   useEffect(() => {
 > 11 |     setFullName(firstName + ' ' + middleName + ' ' + lastName);
-     |     ^^^^^^^^^^^ Values derived from props and 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)
+     |     ^^^^^^^^^^^ This should be computed during render, not in an effect
   12 |   }, [firstName, middleName, lastName]);
   13 |
   14 |   return (

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-with-side-effect.expect.md

@@ -29,13 +29,15 @@ export const FIXTURE_ENTRYPOINT = {
 ```
 Found 1 error:
 
-Error: Values derived from props and 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)
+Error: You might not need an effect. Derive values in render, not effects.
+
+Derived values (From props: [value]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
 
 error.derived-state-from-prop-with-side-effect.ts:8:4
    6 |
    7 |   useEffect(() => {
 >  8 |     setLocalValue(value);
-     |     ^^^^^^^^^^^^^ Values derived from props and 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)
+     |     ^^^^^^^^^^^^^ This should be computed during render, not in an effect
    9 |     document.title = `Value: ${value}`;
   10 |   }, [value]);
   11 |

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.effect-contains-local-function-call.expect.md

@@ -33,13 +33,15 @@ export const FIXTURE_ENTRYPOINT = {
 ```
 Found 1 error:
 
-Error: Values derived from props and 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)
+Error: You might not need an effect. Derive values in render, not effects.
+
+Derived values (From props: [propValue]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
 
 error.effect-contains-local-function-call.ts:12:4
   10 |
   11 |   useEffect(() => {
 > 12 |     setValue(propValue);
-     |     ^^^^^^^^ Values derived from props and 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)
+     |     ^^^^^^^^ This should be computed during render, not in an effect
   13 |     localFunction();
   14 |   }, [propValue]);
   15 |

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-computation-in-effect.expect.md

@@ -31,13 +31,15 @@ export const FIXTURE_ENTRYPOINT = {
 ```
 Found 1 error:
 
-Error: Values derived from props and 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)
+Error: You might not need an effect. Derive values in render, not effects.
+
+Derived values (From local state: [firstName]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
 
 error.invalid-derived-computation-in-effect.ts:11:4
    9 |   const [fullName, setFullName] = useState('');
   10 |   useEffect(() => {
 > 11 |     setFullName(firstName + ' ' + lastName);
-     |     ^^^^^^^^^^^ Values derived from props and 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)
+     |     ^^^^^^^^^^^ This should be computed during render, not in an effect
   12 |   }, [firstName, lastName]);
   13 |
   14 |   return <div>{fullName}</div>;

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-state-from-computed-props.expect.md

@@ -29,13 +29,15 @@ export const FIXTURE_ENTRYPOINT = {
 ```
 Found 1 error:
 
-Error: Values derived from props and 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)
+Error: You might not need an effect. Derive values in render, not effects.
+
+Derived values (From props: [props]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
 
 error.invalid-derived-state-from-computed-props.ts:9:4
    7 |   useEffect(() => {
    8 |     const computed = props.prefix + props.value + props.suffix;
 >  9 |     setDisplayValue(computed);
-     |     ^^^^^^^^^^^^^^^ Values derived from props and 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)
+     |     ^^^^^^^^^^^^^^^ This should be computed during render, not in an effect
   10 |   }, [props.prefix, props.value, props.suffix]);
   11 |
   12 |   return <div>{displayValue}</div>;

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-state-from-destructured-props.expect.md

@@ -30,13 +30,15 @@ export const FIXTURE_ENTRYPOINT = {
 ```
 Found 1 error:
 
-Error: Values derived from props and 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)
+Error: You might not need an effect. Derive values in render, not effects.
+
+Derived values (From props: [props]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
 
 error.invalid-derived-state-from-destructured-props.ts:10:4
    8 |
    9 |   useEffect(() => {
 > 10 |     setFullName(props.firstName + ' ' + props.lastName);
-     |     ^^^^^^^^^^^ Values derived from props and 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)
+     |     ^^^^^^^^^^^ This should be computed during render, not in an effect
   11 |   }, [props.firstName, props.lastName]);
   12 |
   13 |   return <div>{fullName}</div>;