[commit] Better error message for invalid hoisting (#33504)
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. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33504). * #33571 * #33558 * #33547 * #33543 * #33533 * #33532 * #33530 * #33526 * #33522 * #33518 * #33514 * #33513 * #33512 * __->__ #33504 * #33500 * #33497 * #33496
Joseph Savona committed
Jun 18, 2025 at 13:02 UTC
7b67dc92b0339062ce8b6a1d64a458d7c8f04561
4 files changed
+97
-11
compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts
+31
-10
@@ -901,11 +901,36 @@ function applyEffect(
901
console.log(prettyFormat(state.debugAbstractValue(value)));
902
}
903
904
- const reason = getWriteErrorReason({
905
- kind: value.kind,
906
- reason: value.reason,
907
- context: new Set(),
908
- });
904
+ let reason: string;
905
+ let description: string | null = null;
906
+
907
+ if (
908
+ mutationKind === 'mutate-frozen' &&
909
+ context.hoistedContextDeclarations.has(
910
+ effect.value.identifier.declarationId,
911
+ )
912
+ ) {
913
+ reason = `This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time`;
914
+ if (
915
+ effect.value.identifier.name !== null &&
916
+ effect.value.identifier.name.kind === 'named'
917
+ ) {
918
+ description = `Move the declaration of \`${effect.value.identifier.name.value}\` to before it is first referenced`;
919
+ }
920
+ } else {
921
+ reason = getWriteErrorReason({
922
+ kind: value.kind,
923
+ reason: value.reason,
924
+ context: new Set(),
925
+ });
926
+ if (
927
+ effect.value.identifier.name !== null &&
928
+ effect.value.identifier.name.kind === 'named'
929
+ ) {
930
+ description = `Found mutation of \`${effect.value.identifier.name.value}\``;
931
+ }
932
+ }
933
+
934
effects.push({
935
kind:
936
value.kind === ValueKind.Frozen ? 'MutateFrozen' : 'MutateGlobal',
@@ -913,11 +938,7 @@ function applyEffect(
938
error: {
939
severity: ErrorSeverity.InvalidReact,
940
reason,
916
- description:
917
- effect.value.identifier.name !== null &&
918
- effect.value.identifier.name.kind === 'named'
919
- ? `Found mutation of \`${effect.value.identifier.name.value}\``
920
- : null,
941
+ description,
942
loc: effect.value.loc,
943
suggestions: null,
944
},
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-hoisting-setstate.expect.md
+1
-1
@@ -41,7 +41,7 @@ export const FIXTURE_ENTRYPOINT = {
41
19 | useEffect(() => setState(2), []);
42
20 |
43
> 21 | const [state, setState] = useState(0);
44
- | ^^^^^^^^ 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)
44
+ | ^^^^^^^^ 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)
45
22 | return <Stringify state={state} />;
46
23 | }
47
24 |
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.invalid-referencing-frozen-hoisted-storecontext-const.expect.md
new
+43
@@ -0,0 +1,43 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+//@flow @validatePreserveExistingMemoizationGuarantees @enableNewMutationAliasingModel
6
+
7
+import {useCallback} from 'react';
8
+import {useIdentity} from 'shared-runtime';
9
+
10
+function Component({content, refetch}) {
11
+ // This callback function accesses a hoisted const as a dependency,
12
+ // but it cannot reference it as a dependency since that would be a
13
+ // TDZ violation!
14
+ const onRefetch = useCallback(() => {
15
+ refetch(data);
16
+ }, [refetch]);
17
+
18
+ // The context variable gets frozen here since it's passed to a hook
19
+ const onSubmit = useIdentity(onRefetch);
20
+
21
+ // This has to error: onRefetch needs to memoize with `content` as a
22
+ // dependency, but the dependency comes later
23
+ const {data = null} = content;
24
+
25
+ return <Foo data={data} onSubmit={onSubmit} />;
26
+}
27
+
28
+```
29
+
30
+
31
+## Error
32
+
33
+```
34
+ 17 | // This has to error: onRefetch needs to memoize with `content` as a
35
+ 18 | // dependency, but the dependency comes later
36
+> 19 | const {data = null} = content;
37
+ | ^^^^^^^^^^^ 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)
38
+ 20 |
39
+ 21 | return <Foo data={data} onSubmit={onSubmit} />;
40
+ 22 | }
41
+```
42
+
43
+
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.invalid-referencing-frozen-hoisted-storecontext-const.js
new
+22
@@ -0,0 +1,22 @@
1
+//@flow @validatePreserveExistingMemoizationGuarantees @enableNewMutationAliasingModel
2
+
3
+import {useCallback} from 'react';
4
+import {useIdentity} from 'shared-runtime';
5
+
6
+function Component({content, refetch}) {
7
+ // This callback function accesses a hoisted const as a dependency,
8
+ // but it cannot reference it as a dependency since that would be a
9
+ // TDZ violation!
10
+ const onRefetch = useCallback(() => {
11
+ refetch(data);
12
+ }, [refetch]);
13
+
14
+ // The context variable gets frozen here since it's passed to a hook
15
+ const onSubmit = useIdentity(onRefetch);
16
+
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;
20
+
21
+ return <Foo data={data} onSubmit={onSubmit} />;
22
+}