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 c492e28354..80694a7a78 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 ef5c9524b2..982ab36b75 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -67,7 +67,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; @@ -451,18 +456,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, }); } } @@ -1030,6 +1046,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, @@ -1039,15 +1074,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, @@ -1637,7 +1664,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 0000000000..ef79ed1b4a --- /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,34 @@ + +## Input + +```javascript +// @flow + +component Foo() { + const foo = useFoo(); + foo.curren = 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.curren = true; + | ^^^ value cannot be modified + 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 0000000000..efe92bc034 --- /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
; +}