[commit] Better error message for invalid hoisting

We're already tracking which variables are hoisted context variables, so if we see a mutation of a frozen value we can emit a custom error message to help users identify the problem.

Author: josephsavona

compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts

@@ -901,23 +901,44 @@ function applyEffect(
           console.log(prettyFormat(state.debugAbstractValue(value)));
         }
 
-        const reason = getWriteErrorReason({
-          kind: value.kind,
-          reason: value.reason,
-          context: new Set(),
-        });
+        let reason: string;
+        let description: string | null = null;
+
+        if (
+          mutationKind === 'mutate-frozen' &&
+          context.hoistedContextDeclarations.has(
+            effect.value.identifier.declarationId,
+          )
+        ) {
+          reason = `This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time`;
+          if (
+            effect.value.identifier.name !== null &&
+            effect.value.identifier.name.kind === 'named'
+          ) {
+            description = `Move the declaration of \`${effect.value.identifier.name.value}\` to before it is first referenced`;
+          }
+        } else {
+          reason = getWriteErrorReason({
+            kind: value.kind,
+            reason: value.reason,
+            context: new Set(),
+          });
+          if (
+            effect.value.identifier.name !== null &&
+            effect.value.identifier.name.kind === 'named'
+          ) {
+            description = `Found mutation of \`${effect.value.identifier.name.value}\``;
+          }
+        }
+
         effects.push({
           kind:
             value.kind === ValueKind.Frozen ? 'MutateFrozen' : 'MutateGlobal',
           place: effect.value,
           error: {
             severity: ErrorSeverity.InvalidReact,
             reason,
-            description:
-              effect.value.identifier.name !== null &&
-              effect.value.identifier.name.kind === 'named'
-                ? `Found mutation of \`${effect.value.identifier.name.value}\``
-                : null,
+            description,
             loc: effect.value.loc,
             suggestions: null,
           },

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-hoisting-setstate.expect.md

@@ -41,7 +41,7 @@ export const FIXTURE_ENTRYPOINT = {
   19 |   useEffect(() => setState(2), []);
   20 |
 > 21 |   const [state, setState] = useState(0);
-     |                 ^^^^^^^^ InvalidReact: Updating a value used previously in an effect function or as an effect dependency is not allowed. Consider moving the mutation before calling useEffect(). Found mutation of `setState` (21:21)
+     |                 ^^^^^^^^ InvalidReact: This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time. Move the declaration of `setState` to before it is first referenced (21:21)
   22 |   return <Stringify state={state} />;
   23 | }
   24 |

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.invalid-referencing-frozen-hoisted-storecontext-const.expect.md

@@ -0,0 +1,43 @@
+
+## Input
+
+```javascript
+//@flow @validatePreserveExistingMemoizationGuarantees @enableNewMutationAliasingModel
+
+import {useCallback} from 'react';
+import {useIdentity} from 'shared-runtime';
+
+function Component({content, refetch}) {
+  // This callback function accesses a hoisted const as a dependency,
+  // but it cannot reference it as a dependency since that would be a
+  // TDZ violation!
+  const onRefetch = useCallback(() => {
+    refetch(data);
+  }, [refetch]);
+
+  // The context variable gets frozen here since it's passed to a hook
+  const onSubmit = useIdentity(onRefetch);
+
+  // This has to error: onRefetch needs to memoize with `content` as a
+  // dependency, but the dependency comes later
+  const {data = null} = content;
+
+  return <Foo data={data} onSubmit={onSubmit} />;
+}
+
+```
+
+
+## Error
+
+```
+  17 |   // This has to error: onRefetch needs to memoize with `content` as a
+  18 |   // dependency, but the dependency comes later
+> 19 |   const {data = null} = content;
+     |          ^^^^^^^^^^^ InvalidReact: This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time. Move the declaration of `data` to before it is first referenced (19:19)
+  20 |
+  21 |   return <Foo data={data} onSubmit={onSubmit} />;
+  22 | }
+```
+          
+      

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.invalid-referencing-frozen-hoisted-storecontext-const.js

@@ -0,0 +1,22 @@
+//@flow @validatePreserveExistingMemoizationGuarantees @enableNewMutationAliasingModel
+
+import {useCallback} from 'react';
+import {useIdentity} from 'shared-runtime';
+
+function Component({content, refetch}) {
+  // This callback function accesses a hoisted const as a dependency,
+  // but it cannot reference it as a dependency since that would be a
+  // TDZ violation!
+  const onRefetch = useCallback(() => {
+    refetch(data);
+  }, [refetch]);
+
+  // The context variable gets frozen here since it's passed to a hook
+  const onSubmit = useIdentity(onRefetch);
+
+  // This has to error: onRefetch needs to memoize with `content` as a
+  // dependency, but the dependency comes later
+  const {data = null} = content;
+
+  return <Foo data={data} onSubmit={onSubmit} />;
+}