Compare commits

..

1 Commits

Author SHA1 Message Date
Joe Savona
77f30d6f61 [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.
2025-06-18 13:00:42 -07:00
4 changed files with 97 additions and 11 deletions

View File

@@ -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,
},

View File

@@ -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 |

View File

@@ -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 | }
```

View File

@@ -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} />;
}