From 7b67dc92b0339062ce8b6a1d64a458d7c8f04561 Mon Sep 17 00:00:00 2001
From: Joseph Savona <6425824+josephsavona@users.noreply.github.com>
Date: Wed, 18 Jun 2025 13:02:32 -0700
Subject: [PATCH] [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
---
.../Inference/InferMutationAliasingEffects.ts | 41 +++++++++++++-----
.../error.invalid-hoisting-setstate.expect.md | 2 +-
...rozen-hoisted-storecontext-const.expect.md | 43 +++++++++++++++++++
...ncing-frozen-hoisted-storecontext-const.js | 22 ++++++++++
4 files changed, 97 insertions(+), 11 deletions(-)
create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.invalid-referencing-frozen-hoisted-storecontext-const.expect.md
create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.invalid-referencing-frozen-hoisted-storecontext-const.js
diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts
index 19f0d84b9a..f0ecb67546 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts
+++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts
@@ -901,11 +901,36 @@ 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',
@@ -913,11 +938,7 @@ function applyEffect(
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,
},
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-hoisting-setstate.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-hoisting-setstate.expect.md
index fcd5dcc698..3fcc84c9a4 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-hoisting-setstate.expect.md
+++ b/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 ;
23 | }
24 |
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.invalid-referencing-frozen-hoisted-storecontext-const.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.invalid-referencing-frozen-hoisted-storecontext-const.expect.md
new file mode 100644
index 0000000000..7bf3cd0cd3
--- /dev/null
+++ b/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 ;
+}
+
+```
+
+
+## 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 ;
+ 22 | }
+```
+
+
\ No newline at end of file
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.invalid-referencing-frozen-hoisted-storecontext-const.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.invalid-referencing-frozen-hoisted-storecontext-const.js
new file mode 100644
index 0000000000..30d1e0e35e
--- /dev/null
+++ b/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 ;
+}