diff --git a/compiler/packages/babel-plugin-react-compiler/src/Flood/Types.ts b/compiler/packages/babel-plugin-react-compiler/src/Flood/Types.ts index b4f91ab478e..21391b197b7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Flood/Types.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Flood/Types.ts @@ -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( type: ConcreteType, printType: (_: T) => string, @@ -241,7 +240,7 @@ export function printConcrete( 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': { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/AliasingEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/AliasingEffects.ts index c492e283546..80694a7a784 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/AliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/AliasingEffects.ts @@ -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': { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts index 2adf78fe058..44bbaa463d6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -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, diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assing-to-ref-current-in-render.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assing-to-ref-current-in-render.expect.md new file mode 100644 index 00000000000..af94ba06973 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assing-to-ref-current-in-render.expect.md @@ -0,0 +1,42 @@ + +## Input + +```javascript +// @flow + +component Foo() { + const foo = useFoo(); + foo.current = true; + return
; +} + +``` + + +## 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
; + 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
; + 7 | } + 8 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assing-to-ref-current-in-render.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assing-to-ref-current-in-render.js new file mode 100644 index 00000000000..efe92bc034e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assing-to-ref-current-in-render.js @@ -0,0 +1,7 @@ +// @flow + +component Foo() { + const foo = useFoo(); + foo.current = true; + return
; +}