Skip to content

[compiler] Use new diagnostics for core inference errors #33760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -995,13 +995,13 @@ export function printAliasingEffect(effect: AliasingEffect): string {
return `${effect.kind} ${printPlaceForAliasEffect(effect.value)}`;
}
case 'MutateFrozen': {
return `MutateFrozen ${printPlaceForAliasEffect(effect.place)} reason=${JSON.stringify(effect.error.reason)}`;
return `MutateFrozen ${printPlaceForAliasEffect(effect.place)} reason=${JSON.stringify(effect.error.category)}`;
}
case 'MutateGlobal': {
return `MutateGlobal ${printPlaceForAliasEffect(effect.place)} reason=${JSON.stringify(effect.error.reason)}`;
return `MutateGlobal ${printPlaceForAliasEffect(effect.place)} reason=${JSON.stringify(effect.error.category)}`;
}
case 'Impure': {
return `Impure ${printPlaceForAliasEffect(effect.place)} reason=${JSON.stringify(effect.error.reason)}`;
return `Impure ${printPlaceForAliasEffect(effect.place)} reason=${JSON.stringify(effect.error.category)}`;
}
case 'Render': {
return `Render ${printPlaceForAliasEffect(effect.place)}`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

import {CompilerErrorDetailOptions} from '../CompilerError';
import {CompilerDiagnostic} from '../CompilerError';
import {
FunctionExpression,
GeneratedSource,
Expand Down Expand Up @@ -133,19 +133,19 @@ export type AliasingEffect =
/**
* Mutation of a value known to be immutable
*/
| {kind: 'MutateFrozen'; place: Place; error: CompilerErrorDetailOptions}
| {kind: 'MutateFrozen'; place: Place; error: CompilerDiagnostic}
/**
* Mutation of a global
*/
| {
kind: 'MutateGlobal';
place: Place;
error: CompilerErrorDetailOptions;
error: CompilerDiagnostic;
}
/**
* Indicates a side-effect that is not safe during render
*/
| {kind: 'Impure'; place: Place; error: CompilerErrorDetailOptions}
| {kind: 'Impure'; place: Place; error: CompilerDiagnostic}
/**
* Indicates that a given place is accessed during render. Used to distingush
* hook arguments that are known to be called immediately vs those used for
Expand Down Expand Up @@ -211,9 +211,9 @@ export function hashEffect(effect: AliasingEffect): string {
effect.kind,
effect.place.identifier.id,
effect.error.severity,
effect.error.reason,
effect.error.category,
effect.error.description,
printSourceLocation(effect.error.loc ?? GeneratedSource),
printSourceLocation(effect.error.primaryLocation() ?? GeneratedSource),
].join(':');
}
case 'Mutate':
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -326,26 +326,26 @@ function isEffectSafeOutsideRender(effect: FunctionEffect): boolean {

export function getWriteErrorReason(abstractValue: AbstractValue): string {
if (abstractValue.reason.has(ValueReason.Global)) {
return 'Writing to a variable defined outside a component or hook is not allowed. Consider using an effect';
return 'Modifying a variable defined outside a component or hook is not allowed. Consider using an effect';
} else if (abstractValue.reason.has(ValueReason.JsxCaptured)) {
return 'Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX';
return 'Modifying a value used previously in JSX is not allowed. Consider moving the modification before the JSX';
} else if (abstractValue.reason.has(ValueReason.Context)) {
return `Mutating a value returned from 'useContext()', which should not be mutated`;
return `Modifying a value returned from 'useContext()' is not allowed.`;
} else if (abstractValue.reason.has(ValueReason.KnownReturnSignature)) {
return 'Mutating a value returned from a function whose return value should not be mutated';
return 'Modifying a value returned from a function whose return value should not be mutated';
} else if (abstractValue.reason.has(ValueReason.ReactiveFunctionArgument)) {
return 'Mutating component props or hook arguments is not allowed. Consider using a local variable instead';
return 'Modifying component props or hook arguments is not allowed. Consider using a local variable instead';
} else if (abstractValue.reason.has(ValueReason.State)) {
return "Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead";
return "Modifying a value returned from 'useState()', which should not be modified directly. Use the setter function to update instead";
} else if (abstractValue.reason.has(ValueReason.ReducerState)) {
return "Mutating a value returned from 'useReducer()', which should not be mutated. Use the dispatch function to update instead";
return "Modifying a value returned from 'useReducer()', which should not be modified directly. Use the dispatch function to update instead";
} else if (abstractValue.reason.has(ValueReason.Effect)) {
return 'Updating a value used previously in an effect function or as an effect dependency is not allowed. Consider moving the mutation before calling useEffect()';
return 'Modifying a value used previously in an effect function or as an effect dependency is not allowed. Consider moving the modification before calling useEffect()';
} else if (abstractValue.reason.has(ValueReason.HookCaptured)) {
return 'Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook';
return 'Modifying a value previously passed as an argument to a hook is not allowed. Consider moving the modification before calling the hook';
} else if (abstractValue.reason.has(ValueReason.HookReturn)) {
return 'Updating a value returned from a hook is not allowed. Consider moving the mutation into the hook where the value is constructed';
return 'Modifying a value returned from a hook is not allowed. Consider moving the modification into the hook where the value is constructed';
} else {
return 'This mutates a variable that React considers immutable';
return 'This modifies a variable that React considers immutable';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import {
CompilerDiagnostic,
CompilerError,
Effect,
ErrorSeverity,
Expand Down Expand Up @@ -446,20 +447,24 @@ function applySignature(
reason: value.reason,
context: new Set(),
});
const message =
effect.value.identifier.name !== null &&
effect.value.identifier.name.kind === 'named'
? `\`${effect.value.identifier.name.value}\` cannot be modified`
: 'This value cannot be modified';
effects.push({
kind: 'MutateFrozen',
place: effect.value,
error: {
error: CompilerDiagnostic.create({
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,
loc: effect.value.loc,
category: 'This value cannot be modified',
description: reason,
suggestions: null,
},
}).withDetail({
kind: 'error',
loc: effect.value.loc,
message,
}),
});
}
}
Expand Down Expand Up @@ -1016,44 +1021,36 @@ function applyEffect(
const description =
effect.value.identifier.name !== null &&
effect.value.identifier.name.kind === 'named'
? `Variable \`${effect.value.identifier.name.value}\` is accessed before it is declared`
: null;
? `Variable \`${effect.value.identifier.name.value}\``
: 'This variable';
const hoistedAccess = context.hoistedContextDeclarations.get(
effect.value.identifier.declarationId,
);
const diagnostic = CompilerDiagnostic.create({
severity: ErrorSeverity.InvalidReact,
category: 'Cannot access variable before it is declared',
description: `${description} is accessed before it is declared, which prevents the earlier access from updating when this value changes over time`,
});
if (hoistedAccess != null && hoistedAccess.loc != effect.value.loc) {
applyEffect(
context,
state,
{
kind: 'MutateFrozen',
place: effect.value,
error: {
severity: ErrorSeverity.InvalidReact,
reason: `This variable is accessed before it is declared, which may prevent it from updating as the assigned value changes over time`,
description,
loc: hoistedAccess.loc,
suggestions: null,
},
},
initialized,
effects,
);
diagnostic.withDetail({
kind: 'error',
loc: hoistedAccess.loc,
message: 'Variable accessed before it is declared',
});
}
diagnostic.withDetail({
kind: 'error',
loc: effect.value.loc,
message: 'The variable is declared here',
});

applyEffect(
context,
state,
{
kind: 'MutateFrozen',
place: effect.value,
error: {
severity: ErrorSeverity.InvalidReact,
reason: `This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time`,
description,
loc: effect.value.loc,
suggestions: null,
},
error: diagnostic,
},
initialized,
effects,
Expand All @@ -1064,11 +1061,11 @@ function applyEffect(
reason: value.reason,
context: new Set(),
});
const description =
const message =
effect.value.identifier.name !== null &&
effect.value.identifier.name.kind === 'named'
? `Found mutation of \`${effect.value.identifier.name.value}\``
: null;
? `\`${effect.value.identifier.name.value}\` cannot be modified`
: 'This value cannot be modified';
applyEffect(
context,
state,
Expand All @@ -1078,13 +1075,15 @@ function applyEffect(
? 'MutateFrozen'
: 'MutateGlobal',
place: effect.value,
error: {
error: CompilerDiagnostic.create({
severity: ErrorSeverity.InvalidReact,
reason,
description,
category: 'This value cannot be modified',
description: reason,
}).withDetail({
kind: 'error',
loc: effect.value.loc,
suggestions: null,
},
message,
}),
},
initialized,
effects,
Expand Down Expand Up @@ -2006,13 +2005,18 @@ function computeSignatureForInstruction(
effects.push({
kind: 'MutateGlobal',
place: value.value,
error: {
reason:
'Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render)',
loc: instr.loc,
suggestions: null,
error: CompilerDiagnostic.create({
severity: ErrorSeverity.InvalidReact,
},
category:
'Cannot reassign variables declared outside of the component/hook',
description:
'Reassigning a variable declared outside of the component/hook is a form of side effect, which can cause unpredictable behavior depending on when the component happens to re-render. If this variable is used in rendering, use useState instead. Otherwise, consider updating it in an effect (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render)',
suggestions: null,
}).withDetail({
kind: 'error',
loc: instr.loc,
message: 'Cannot reassign variable',
}),
});
effects.push({kind: 'Assign', from: value.value, into: lvalue});
break;
Expand Down Expand Up @@ -2102,17 +2106,20 @@ function computeEffectsForLegacySignature(
effects.push({
kind: 'Impure',
place: receiver,
error: {
reason:
'Calling an impure function can produce unstable results. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent)',
description:
signature.canonicalName != null
? `\`${signature.canonicalName}\` is an impure function whose results may change on every call`
: null,
error: CompilerDiagnostic.create({
severity: ErrorSeverity.InvalidReact,
loc,
category: 'Cannot call impure function during render',
description:
(signature.canonicalName != null
? `\`${signature.canonicalName}\` is an impure function. `
: '') +
'Calling an impure function can produce unstable results that update unpredictably when the component happens to re-render. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent)',
suggestions: null,
},
}).withDetail({
kind: 'error',
loc,
message: 'Cannot call impure function',
}),
});
}
const stores: Array<Place> = [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ export function inferMutationAliasingRanges(
effect.kind === 'MutateGlobal' ||
effect.kind === 'Impure'
) {
errors.push(effect.error);
errors.pushDiagnostic(effect.error);
functionEffects.push(effect);
} else if (effect.kind === 'Render') {
renders.push({index: index++, place: effect.place});
Expand Down Expand Up @@ -549,7 +549,7 @@ function appendFunctionErrors(errors: CompilerError, fn: HIRFunction): void {
case 'Impure':
case 'MutateFrozen':
case 'MutateGlobal': {
errors.push(effect.error);
errors.pushDiagnostic(effect.error);
break;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,18 @@ function Component() {

```
Found 1 error:
Error: Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render)
Error: Cannot reassign variables declared outside of the component/hook

Reassigning a variable declared outside of the component/hook is a form of side effect, which can cause unpredictable behavior depending on when the component happens to re-render. If this variable is used in rendering, use useState instead. Otherwise, consider updating it in an effect (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render)

error.assign-global-in-component-tag-function.ts:3:4
1 | function Component() {
2 | const Foo = () => {
> 3 | someGlobal = true;
| ^^^^^^^^^^ Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render)
| ^^^^^^^^^^ Cannot reassign variable
4 | };
5 | return <Foo />;
6 | }


```


Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,18 @@ function Component() {

```
Found 1 error:
Error: Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render)
Error: Cannot reassign variables declared outside of the component/hook

Reassigning a variable declared outside of the component/hook is a form of side effect, which can cause unpredictable behavior depending on when the component happens to re-render. If this variable is used in rendering, use useState instead. Otherwise, consider updating it in an effect (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render)

error.assign-global-in-jsx-children.ts:3:4
1 | function Component() {
2 | const foo = () => {
> 3 | someGlobal = true;
| ^^^^^^^^^^ Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render)
| ^^^^^^^^^^ Cannot reassign variable
4 | };
5 | // Children are generally access/called during render, so
6 | // modifying a global in a children function is almost


```


Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,18 @@ export const FIXTURE_ENTRYPOINT = {

```
Found 1 error:
Error: Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook
Error: This value cannot be modified

Modifying a value previously passed as an argument to a hook is not allowed. Consider moving the modification before calling the hook

error.hook-call-freezes-captured-identifier.ts:13:2
11 | });
12 |
> 13 | x.value += count;
| ^ Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook
| ^ This value cannot be modified
14 | return <Stringify x={x} cb={cb} />;
15 | }
16 |


```


Loading
Loading