The library for web and native user interfaces.
Mirrored from https://github.com/247TaxiDriver/QOS-React.git
Synced Nov 8, 10:00 UTC
[commit] Improve error for hoisting violations (#33514)
The previous error for hoisting violations pointed only to the variable declaration, but didn't show where the value was accessed before that declaration. We now track where each hoisted variable is first accessed and report two errors, one for the reference and one for the declaration. When we improve our diagnostic infra to support reporting errors at multiple locations we can merge these into a single conceptual error. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33514). * #33571 * #33558 * #33547 * #33543 * #33533 * #33532 * #33530 * #33526 * #33522 * #33518 * __->__ #33514 * #33573
Joseph Savona committed
Jun 18, 2025 at 15:24 UTC
8f4ce72f0bfe02e51e9a7c704dc33122d909f292
3 files changed
+86
-42
compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts
+70
-30
@@ -38,6 +38,7 @@ import {
38
import {
39
eachInstructionValueLValue,
40
eachInstructionValueOperand,
41
+ eachTerminalOperand,
42
eachTerminalSuccessor,
43
} from '../HIR/visitors';
44
import {Ok, Result} from '../Utils/Result';
@@ -221,8 +222,19 @@ export function inferMutationAliasingEffects(
222
return Ok(undefined);
223
}
224
224
-function findHoistedContextDeclarations(fn: HIRFunction): Set<DeclarationId> {
225
- const hoisted = new Set<DeclarationId>();
225
+function findHoistedContextDeclarations(
226
+ fn: HIRFunction,
227
+): Map<DeclarationId, Place | null> {
228
+ const hoisted = new Map<DeclarationId, Place | null>();
229
+ function visit(place: Place): void {
230
+ if (
231
+ hoisted.has(place.identifier.declarationId) &&
232
+ hoisted.get(place.identifier.declarationId) == null
233
+ ) {
234
+ // If this is the first load of the value, store the location
235
+ hoisted.set(place.identifier.declarationId, place);
236
+ }
237
+ }
238
for (const block of fn.body.blocks.values()) {
239
for (const instr of block.instructions) {
240
if (instr.value.kind === 'DeclareContext') {
@@ -232,10 +244,17 @@ function findHoistedContextDeclarations(fn: HIRFunction): Set<DeclarationId> {
244
kind == InstructionKind.HoistedFunction ||
245
kind == InstructionKind.HoistedLet
246
) {
235
- hoisted.add(instr.value.lvalue.place.identifier.declarationId);
247
+ hoisted.set(instr.value.lvalue.place.identifier.declarationId, null);
248
+ }
249
+ } else {
250
+ for (const operand of eachInstructionValueOperand(instr.value)) {
251
+ visit(operand);
252
}
253
}
254
}
255
+ for (const operand of eachTerminalOperand(block.terminal)) {
256
+ visit(operand);
257
+ }
258
}
259
return hoisted;
260
}
@@ -248,12 +267,12 @@ class Context {
267
catchHandlers: Map<BlockId, Place> = new Map();
268
isFuctionExpression: boolean;
269
fn: HIRFunction;
251
- hoistedContextDeclarations: Set<DeclarationId>;
270
+ hoistedContextDeclarations: Map<DeclarationId, Place | null>;
271
272
constructor(
273
isFunctionExpression: boolean,
274
fn: HIRFunction,
256
- hoistedContextDeclarations: Set<DeclarationId>,
275
+ hoistedContextDeclarations: Map<DeclarationId, Place | null>,
276
) {
277
this.isFuctionExpression = isFunctionExpression;
278
this.fn = fn;
@@ -901,48 +920,69 @@ function applyEffect(
920
console.log(prettyFormat(state.debugAbstractValue(value)));
921
}
922
904
- let reason: string;
905
- let description: string | null = null;
906
-
923
if (
924
mutationKind === 'mutate-frozen' &&
925
context.hoistedContextDeclarations.has(
926
effect.value.identifier.declarationId,
927
)
928
) {
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 (
929
+ const description =
930
effect.value.identifier.name !== null &&
931
effect.value.identifier.name.kind === 'named'
917
- ) {
918
- description = `Move the declaration of \`${effect.value.identifier.name.value}\` to before it is first referenced`;
932
+ ? `Variable \`${effect.value.identifier.name.value}\` is accessed before it is declared`
933
+ : null;
934
+ const hoistedAccess = context.hoistedContextDeclarations.get(
935
+ effect.value.identifier.declarationId,
936
+ );
937
+ if (hoistedAccess != null && hoistedAccess.loc != effect.value.loc) {
938
+ effects.push({
939
+ kind: 'MutateFrozen',
940
+ place: effect.value,
941
+ error: {
942
+ severity: ErrorSeverity.InvalidReact,
943
+ reason: `This variable is accessed before it is declared, which may prevent it from updating as the assigned value changes over time`,
944
+ description,
945
+ loc: hoistedAccess.loc,
946
+ suggestions: null,
947
+ },
948
+ });
949
}
950
+
951
+ effects.push({
952
+ kind: 'MutateFrozen',
953
+ place: effect.value,
954
+ error: {
955
+ severity: ErrorSeverity.InvalidReact,
956
+ reason: `This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time`,
957
+ description,
958
+ loc: effect.value.loc,
959
+ suggestions: null,
960
+ },
961
+ });
962
} else {
921
- reason = getWriteErrorReason({
963
+ const reason = getWriteErrorReason({
964
kind: value.kind,
965
reason: value.reason,
966
context: new Set(),
967
});
926
- if (
968
+ const description =
969
effect.value.identifier.name !== null &&
970
effect.value.identifier.name.kind === 'named'
929
- ) {
930
- description = `Found mutation of \`${effect.value.identifier.name.value}\``;
931
- }
971
+ ? `Found mutation of \`${effect.value.identifier.name.value}\``
972
+ : null;
973
+ effects.push({
974
+ kind:
975
+ value.kind === ValueKind.Frozen ? 'MutateFrozen' : 'MutateGlobal',
976
+ place: effect.value,
977
+ error: {
978
+ severity: ErrorSeverity.InvalidReact,
979
+ reason,
980
+ description,
981
+ loc: effect.value.loc,
982
+ suggestions: null,
983
+ },
984
+ });
985
}
933
-
934
- effects.push({
935
- kind:
936
- value.kind === ValueKind.Frozen ? 'MutateFrozen' : 'MutateGlobal',
937
- place: effect.value,
938
- error: {
939
- severity: ErrorSeverity.InvalidReact,
940
- reason,
941
- description,
942
- loc: effect.value.loc,
943
- suggestions: null,
944
- },
945
- });
986
}
987
break;
988
}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-hoisting-setstate.expect.md
+7
-5
@@ -38,13 +38,15 @@ export const FIXTURE_ENTRYPOINT = {
38
## Error
39
40
```
41
- 19 | useEffect(() => setState(2), []);
41
+ 17 | * $2 = Function context=setState
42
+ 18 | */
43
+> 19 | useEffect(() => setState(2), []);
44
+ | ^^^^^^^^ InvalidReact: This variable is accessed before it is declared, which may prevent it from updating as the assigned value changes over time. Variable `setState` is accessed before it is declared (19:19)
45
+
46
+InvalidReact: This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time. Variable `setState` is accessed before it is declared (21:21)
47
20 |
43
-> 21 | const [state, setState] = useState(0);
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)
48
+ 21 | const [state, setState] = useState(0);
49
22 | return <Stringify state={state} />;
46
- 23 | }
47
- 24 |
50
```
51
52
\ 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.expect.md
+9
-7
@@ -31,13 +31,15 @@ function Component({content, refetch}) {
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 | }
34
+ 9 | // TDZ violation!
35
+ 10 | const onRefetch = useCallback(() => {
36
+> 11 | refetch(data);
37
+ | ^^^^ InvalidReact: This variable is accessed before it is declared, which may prevent it from updating as the assigned value changes over time. Variable `data` is accessed before it is declared (11:11)
38
+
39
+InvalidReact: This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time. Variable `data` is accessed before it is declared (19:19)
40
+ 12 | }, [refetch]);
41
+ 13 |
42
+ 14 | // The context variable gets frozen here since it's passed to a hook
43
```
44
45
\ No newline at end of file