Compare commits

...

2 Commits

Author SHA1 Message Date
Joe Savona
61b605f764 [compiler] Add hint to name variables with "Ref" suffix
If you have a ref that the compiler doesn't know is a ref (say, a value returned from a custom hook) and try to assign its `.current = ...`, we currently fail with a generic error that hook return values are not mutable. However, an assignment to `.current` specifically is a very strong hint that the value is likely to be a ref. So in this PR, we track the reason for the mutation and if it ends up being an error, we use it to show an additional hint to the user. See the fixture for an example of the message.
2025-08-07 00:10:41 -07:00
Joe Savona
fa992fdc10 [compiler] remove use of inspect module 2025-08-06 23:48:57 -07:00
5 changed files with 109 additions and 24 deletions

View File

@@ -9,6 +9,7 @@ import {
import * as t from '@babel/types';
import * as TypeErrors from './TypeErrors';
import {assertExhaustive} from '../Utils/utils';
import {FlowType} from './FlowTypes';
export const DEBUG = false;
@@ -196,8 +197,6 @@ export function makeVariableId(id: number): VariableId {
return id as VariableId;
}
import {inspect} from 'util';
import {FlowType} from './FlowTypes';
export function printConcrete<T>(
type: ConcreteType<T>,
printType: (_: T) => string,
@@ -241,7 +240,7 @@ export function printConcrete<T>(
case 'Generic':
return `T${type.id}`;
case 'Object': {
const name = `Object ${inspect([...type.members.keys()])}`;
const name = `Object [${[...type.members.keys()].map(key => JSON.stringify(key)).join(', ')}]`;
return `${name}`;
}
case 'Tuple': {

View File

@@ -50,7 +50,7 @@ export type AliasingEffect =
/**
* Mutate the value and any direct aliases (not captures). Errors if the value is not mutable.
*/
| {kind: 'Mutate'; value: Place}
| {kind: 'Mutate'; value: Place; reason?: MutationReason | null}
/**
* Mutate the value and any direct aliases (not captures), but only if the value is known mutable.
* This should be rare.
@@ -174,6 +174,8 @@ export type AliasingEffect =
place: Place;
};
export type MutationReason = {kind: 'AssignCurrentProperty'};
export function hashEffect(effect: AliasingEffect): string {
switch (effect.kind) {
case 'Apply': {

View File

@@ -68,7 +68,12 @@ import {FunctionSignature} from '../HIR/ObjectShape';
import {getWriteErrorReason} from './InferFunctionEffects';
import prettyFormat from 'pretty-format';
import {createTemporaryPlace} from '../HIR/HIRBuilder';
import {AliasingEffect, AliasingSignature, hashEffect} from './AliasingEffects';
import {
AliasingEffect,
AliasingSignature,
hashEffect,
MutationReason,
} from './AliasingEffects';
const DEBUG = false;
@@ -452,18 +457,29 @@ function applySignature(
effect.value.identifier.name.kind === 'named'
? `\`${effect.value.identifier.name.value}\``
: 'value';
const diagnostic = CompilerDiagnostic.create({
severity: ErrorSeverity.InvalidReact,
category: 'This value cannot be modified',
description: `${reason}.`,
}).withDetail({
kind: 'error',
loc: effect.value.loc,
message: `${variable} cannot be modified`,
});
if (
effect.kind === 'Mutate' &&
effect.reason?.kind === 'AssignCurrentProperty'
) {
diagnostic.withDetail({
kind: 'error',
loc: effect.value.loc,
message: `Hint: If this value is a Ref (value returned by \`useRef()\`), rename the variable to end in "Ref".`,
});
}
effects.push({
kind: 'MutateFrozen',
place: effect.value,
error: CompilerDiagnostic.create({
severity: ErrorSeverity.InvalidReact,
category: 'This value cannot be modified',
description: `${reason}.`,
}).withDetail({
kind: 'error',
loc: effect.value.loc,
message: `${variable} cannot be modified`,
}),
error: diagnostic,
});
}
}
@@ -1066,6 +1082,25 @@ function applyEffect(
effect.value.identifier.name.kind === 'named'
? `\`${effect.value.identifier.name.value}\``
: 'value';
const diagnostic = CompilerDiagnostic.create({
severity: ErrorSeverity.InvalidReact,
category: 'This value cannot be modified',
description: `${reason}.`,
}).withDetail({
kind: 'error',
loc: effect.value.loc,
message: `${variable} cannot be modified`,
});
if (
effect.kind === 'Mutate' &&
effect.reason?.kind === 'AssignCurrentProperty'
) {
diagnostic.withDetail({
kind: 'error',
loc: effect.value.loc,
message: `Hint: If this value is a Ref (value returned by \`useRef()\`), rename the variable to end in "Ref".`,
});
}
applyEffect(
context,
state,
@@ -1075,15 +1110,7 @@ function applyEffect(
? 'MutateFrozen'
: 'MutateGlobal',
place: effect.value,
error: CompilerDiagnostic.create({
severity: ErrorSeverity.InvalidReact,
category: 'This value cannot be modified',
description: `${reason}.`,
}).withDetail({
kind: 'error',
loc: effect.value.loc,
message: `${variable} cannot be modified`,
}),
error: diagnostic,
},
initialized,
effects,
@@ -1680,7 +1707,15 @@ function computeSignatureForInstruction(
}
case 'PropertyStore':
case 'ComputedStore': {
effects.push({kind: 'Mutate', value: value.object});
const mutationReason: MutationReason | null =
value.kind === 'PropertyStore' && value.property === 'current'
? {kind: 'AssignCurrentProperty'}
: null;
effects.push({
kind: 'Mutate',
value: value.object,
reason: mutationReason,
});
effects.push({
kind: 'Capture',
from: value.value,

View File

@@ -0,0 +1,42 @@
## Input
```javascript
// @flow
component Foo() {
const foo = useFoo();
foo.current = true;
return <div />;
}
```
## Error
```
Found 1 error:
Error: This value cannot be modified
Modifying a value returned from a hook is not allowed. Consider moving the modification into the hook where the value is constructed.
3 | component Foo() {
4 | const foo = useFoo();
> 5 | foo.current = true;
| ^^^ value cannot be modified
6 | return <div />;
7 | }
8 |
3 | component Foo() {
4 | const foo = useFoo();
> 5 | foo.current = true;
| ^^^ Hint: If this value is a Ref (value returned by `useRef()`), rename the variable to end in "Ref".
6 | return <div />;
7 | }
8 |
```

View File

@@ -0,0 +1,7 @@
// @flow
component Foo() {
const foo = useFoo();
foo.current = true;
return <div />;
}