Skip to content
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 @@ -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;

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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': {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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': {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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,
});
}
}
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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 |
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// @flow

component Foo() {
const foo = useFoo();
foo.current = true;
return <div />;
}
Loading