diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects.ts index 8ec7542a9d..44e65063ec 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects.ts @@ -5,21 +5,31 @@ * LICENSE file in the root directory of this source tree. */ -import {CompilerError, SourceLocation} from '..'; +import {CompilerError, Effect} from '..'; import {ErrorCategory} from '../CompilerError'; import { - ArrayExpression, BlockId, FunctionExpression, HIRFunction, IdentifierId, isSetStateType, isUseEffectHookType, + Place, + CallExpression, + Instruction, + isUseStateType, } from '../HIR'; -import { - eachInstructionValueOperand, - eachTerminalOperand, -} from '../HIR/visitors'; +import {eachInstructionLValue, eachInstructionOperand} from '../HIR/visitors'; +import {isMutable} from '../ReactiveScopes/InferReactiveScopeVariables'; +import {assertExhaustive} from '../Utils/utils'; + +type TypeOfValue = 'ignored' | 'fromProps' | 'fromState' | 'fromPropsAndState'; + +type DerivationMetadata = { + typeOfValue: TypeOfValue; + place: Place; + sourcesIds: Set; +}; /** * Validates that useEffect is not used for derived computations which could/should @@ -45,102 +55,226 @@ import { * ``` */ export function validateNoDerivedComputationsInEffects(fn: HIRFunction): void { - const candidateDependencies: Map = new Map(); const functions: Map = new Map(); - const locals: Map = new Map(); + + const derivationCache: Map = new Map(); + if (fn.fnType === 'Hook') { + for (const param of fn.params) { + if (param.kind === 'Identifier') { + derivationCache.set(param.identifier.id, { + place: param, + sourcesIds: new Set([param.identifier.id]), + typeOfValue: 'fromProps', + }); + } + } + } else if (fn.fnType === 'Component') { + const props = fn.params[0]; + if (props != null && props.kind === 'Identifier') { + derivationCache.set(props.identifier.id, { + place: props, + sourcesIds: new Set([props.identifier.id]), + typeOfValue: 'fromProps', + }); + } + } const errors = new CompilerError(); for (const block of fn.body.blocks.values()) { - for (const instr of block.instructions) { - const {lvalue, value} = instr; - if (value.kind === 'LoadLocal') { - locals.set(lvalue.identifier.id, value.place.identifier.id); - } else if (value.kind === 'ArrayExpression') { - candidateDependencies.set(lvalue.identifier.id, value); - } else if (value.kind === 'FunctionExpression') { - functions.set(lvalue.identifier.id, value); - } else if ( - value.kind === 'CallExpression' || - value.kind === 'MethodCall' - ) { - const callee = - value.kind === 'CallExpression' ? value.callee : value.property; - if ( - isUseEffectHookType(callee.identifier) && - value.args.length === 2 && - value.args[0].kind === 'Identifier' && - value.args[1].kind === 'Identifier' + for (const phi of block.phis) { + let typeOfValue: TypeOfValue = 'ignored'; + let sourcesIds: Set = new Set(); + for (const operand of phi.operands.values()) { + const operandMetadata = derivationCache.get(operand.identifier.id); + + if (operandMetadata === undefined) { + continue; + } + + typeOfValue = joinValue(typeOfValue, operandMetadata.typeOfValue); + sourcesIds.add(operand.identifier.id); + } + + if (typeOfValue !== 'ignored') { + addDerivationEntry(phi.place, sourcesIds, typeOfValue, derivationCache); + } + } + for (const i of block.instructions) { + function recordInstructiorDerivations(instr: Instruction): void { + let typeOfValue: TypeOfValue = 'ignored'; + const sources: Set = new Set(); + const {lvalue, value} = instr; + if (value.kind === 'FunctionExpression') { + functions.set(lvalue.identifier.id, value); + for (const [, block] of value.loweredFunc.func.body.blocks) { + for (const instr of block.instructions) { + recordInstructiorDerivations(instr); + } + } + } else if ( + value.kind === 'CallExpression' || + value.kind === 'MethodCall' ) { - const effectFunction = functions.get(value.args[0].identifier.id); - const deps = candidateDependencies.get(value.args[1].identifier.id); + const callee = + value.kind === 'CallExpression' ? value.callee : value.property; if ( - effectFunction != null && - deps != null && - deps.elements.length !== 0 && - deps.elements.every(element => element.kind === 'Identifier') + isUseEffectHookType(callee.identifier) && + value.args.length === 2 && + value.args[0].kind === 'Identifier' && + value.args[1].kind === 'Identifier' ) { - const dependencies: Array = deps.elements.map(dep => { - CompilerError.invariant(dep.kind === 'Identifier', { - reason: `Dependency is checked as a place above`, + const effectFunction = functions.get(value.args[0].identifier.id); + if (effectFunction != null) { + validateEffect( + effectFunction.loweredFunc.func, + errors, + derivationCache, + ); + } + } else if (isUseStateType(lvalue.identifier)) { + const stateValueSource = value.args[0]; + if (stateValueSource.kind === 'Identifier') { + sources.add(stateValueSource.identifier.id); + } + typeOfValue = joinValue(typeOfValue, 'fromState'); + } + } + + for (const operand of eachInstructionOperand(instr)) { + const operandMetadata = derivationCache.get(operand.identifier.id); + + if (operandMetadata === undefined) { + continue; + } + + typeOfValue = joinValue(typeOfValue, operandMetadata.typeOfValue); + for (const id of operandMetadata.sourcesIds) { + sources.add(id); + } + } + + if (typeOfValue === 'ignored') { + return; + } + + for (const lvalue of eachInstructionLValue(instr)) { + addDerivationEntry(lvalue, sources, typeOfValue, derivationCache); + } + + for (const operand of eachInstructionOperand(instr)) { + switch (operand.effect) { + case Effect.Capture: + case Effect.Store: + case Effect.ConditionallyMutate: + case Effect.ConditionallyMutateIterator: + case Effect.Mutate: { + if (isMutable(instr, operand)) { + addDerivationEntry( + operand, + sources, + typeOfValue, + derivationCache, + ); + } + break; + } + case Effect.Freeze: + case Effect.Read: { + // no-op + break; + } + case Effect.Unknown: { + CompilerError.invariant(false, { + reason: 'Unexpected unknown effect', description: null, details: [ { kind: 'error', - loc: value.loc, - message: 'this is checked as a place above', + loc: operand.loc, + message: 'Unexpected unknown effect', }, ], }); - return locals.get(dep.identifier.id) ?? dep.identifier.id; - }); - validateEffect( - effectFunction.loweredFunc.func, - dependencies, - errors, - ); + } + default: { + assertExhaustive( + operand.effect, + `Unexpected effect kind \`${operand.effect}\``, + ); + } } } } + recordInstructiorDerivations(i); } } + if (errors.hasAnyErrors()) { throw errors; } } +function joinValue( + lvalueType: TypeOfValue, + valueType: TypeOfValue, +): TypeOfValue { + if (lvalueType === 'ignored') return valueType; + if (valueType === 'ignored') return lvalueType; + if (lvalueType === valueType) return lvalueType; + return 'fromPropsAndState'; +} + +function addDerivationEntry( + derivedVar: Place, + sourcesIds: Set, + typeOfValue: TypeOfValue, + derivationCache: Map, +): void { + let newValue: DerivationMetadata = { + place: derivedVar, + sourcesIds: new Set(), + typeOfValue: typeOfValue ?? 'ignored', + }; + + if (sourcesIds !== undefined) { + for (const id of sourcesIds) { + const sourcePlace = derivationCache.get(id)?.place; + + if (sourcePlace === undefined) { + continue; + } + + /* + * If the identifier of the source is a promoted identifier, then + * we should set the target as the source. + */ + if ( + sourcePlace.identifier.name === null || + sourcePlace.identifier.name?.kind === 'promoted' + ) { + newValue.sourcesIds.add(derivedVar.identifier.id); + } else { + newValue.sourcesIds.add(sourcePlace.identifier.id); + } + } + } + + derivationCache.set(derivedVar.identifier.id, newValue); +} + function validateEffect( effectFunction: HIRFunction, - effectDeps: Array, errors: CompilerError, + derivationCache: Map, ): void { - for (const operand of effectFunction.context) { - if (isSetStateType(operand.identifier)) { - continue; - } else if (effectDeps.find(dep => dep === operand.identifier.id) != null) { - continue; - } else { - // Captured something other than the effect dep or setState - return; - } - } - for (const dep of effectDeps) { - if ( - effectFunction.context.find(operand => operand.identifier.id === dep) == - null - ) { - // effect dep wasn't actually used in the function - return; - } - } - const seenBlocks: Set = new Set(); - const values: Map> = new Map(); - for (const dep of effectDeps) { - values.set(dep, [dep]); - } - const setStateLocations: Array = []; + const effectDerivedSetStateCalls: Array<{ + value: CallExpression; + sourceIds: Set; + }> = []; + for (const block of effectFunction.body.blocks.values()) { for (const pred of block.preds) { if (!seenBlocks.has(pred)) { @@ -148,90 +282,36 @@ function validateEffect( return; } } - for (const phi of block.phis) { - const aggregateDeps: Set = new Set(); - for (const operand of phi.operands.values()) { - const deps = values.get(operand.identifier.id); - if (deps != null) { - for (const dep of deps) { - aggregateDeps.add(dep); - } - } - } - if (aggregateDeps.size !== 0) { - values.set(phi.place.identifier.id, Array.from(aggregateDeps)); - } - } - for (const instr of block.instructions) { - switch (instr.value.kind) { - case 'Primitive': - case 'JSXText': - case 'LoadGlobal': { - break; - } - case 'LoadLocal': { - const deps = values.get(instr.value.place.identifier.id); - if (deps != null) { - values.set(instr.lvalue.identifier.id, deps); - } - break; - } - case 'ComputedLoad': - case 'PropertyLoad': - case 'BinaryExpression': - case 'TemplateLiteral': - case 'CallExpression': - case 'MethodCall': { - const aggregateDeps: Set = new Set(); - for (const operand of eachInstructionValueOperand(instr.value)) { - const deps = values.get(operand.identifier.id); - if (deps != null) { - for (const dep of deps) { - aggregateDeps.add(dep); - } - } - } - if (aggregateDeps.size !== 0) { - values.set(instr.lvalue.identifier.id, Array.from(aggregateDeps)); - } - if ( - instr.value.kind === 'CallExpression' && - isSetStateType(instr.value.callee.identifier) && - instr.value.args.length === 1 && - instr.value.args[0].kind === 'Identifier' - ) { - const deps = values.get(instr.value.args[0].identifier.id); - if (deps != null && new Set(deps).size === effectDeps.length) { - setStateLocations.push(instr.value.callee.loc); - } else { - // doesn't depend on any deps - return; - } - } - break; + for (const instr of block.instructions) { + if ( + instr.value.kind === 'CallExpression' && + isSetStateType(instr.value.callee.identifier) && + instr.value.args.length === 1 && + instr.value.args[0].kind === 'Identifier' + ) { + const argMetadata = derivationCache.get( + instr.value.args[0].identifier.id, + ); + + if (argMetadata !== undefined) { + effectDerivedSetStateCalls.push({ + value: instr.value, + sourceIds: argMetadata.sourcesIds, + }); } - default: { - return; - } - } - } - for (const operand of eachTerminalOperand(block.terminal)) { - if (values.has(operand.identifier.id)) { - // - return; } } seenBlocks.add(block.id); } - for (const loc of setStateLocations) { + for (const derivedSetStateCall of effectDerivedSetStateCalls) { errors.push({ category: ErrorCategory.EffectDerivationsOfState, reason: 'Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)', description: null, - loc, + loc: derivedSetStateCall.value.loc, suggestions: null, }); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-conditionally-in-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-conditionally-in-effect.expect.md deleted file mode 100644 index b7a1c85d52..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-conditionally-in-effect.expect.md +++ /dev/null @@ -1,79 +0,0 @@ - -## Input - -```javascript -// @validateNoDerivedComputationsInEffects -import {useEffect, useState} from 'react'; - -function Component({value, enabled}) { - const [localValue, setLocalValue] = useState(''); - - useEffect(() => { - if (enabled) { - setLocalValue(value); - } else { - setLocalValue('disabled'); - } - }, [value, enabled]); - - return
{localValue}
; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{value: 'test', enabled: true}], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects -import { useEffect, useState } from "react"; - -function Component(t0) { - const $ = _c(6); - const { value, enabled } = t0; - const [localValue, setLocalValue] = useState(""); - let t1; - let t2; - if ($[0] !== enabled || $[1] !== value) { - t1 = () => { - if (enabled) { - setLocalValue(value); - } else { - setLocalValue("disabled"); - } - }; - - t2 = [value, enabled]; - $[0] = enabled; - $[1] = value; - $[2] = t1; - $[3] = t2; - } else { - t1 = $[2]; - t2 = $[3]; - } - useEffect(t1, t2); - let t3; - if ($[4] !== localValue) { - t3 =
{localValue}
; - $[4] = localValue; - $[5] = t3; - } else { - t3 = $[5]; - } - return t3; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{ value: "test", enabled: true }], -}; - -``` - -### Eval output -(kind: ok)
test
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-default-props.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-default-props.expect.md deleted file mode 100644 index d4024eab5c..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-default-props.expect.md +++ /dev/null @@ -1,68 +0,0 @@ - -## Input - -```javascript -// @validateNoDerivedComputationsInEffects - -export default function Component(input = 'empty') { - const [currInput, setCurrInput] = useState(input); - const localConst = 'local const'; - - useEffect(() => { - setCurrInput(input + localConst); - }, [input, localConst]); - - return
{currInput}
; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{input: 'test'}], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects - -export default function Component(t0) { - const $ = _c(5); - const input = t0 === undefined ? "empty" : t0; - const [currInput, setCurrInput] = useState(input); - let t1; - let t2; - if ($[0] !== input) { - t1 = () => { - setCurrInput(input + "local const"); - }; - t2 = [input, "local const"]; - $[0] = input; - $[1] = t1; - $[2] = t2; - } else { - t1 = $[1]; - t2 = $[2]; - } - useEffect(t1, t2); - let t3; - if ($[3] !== currInput) { - t3 =
{currInput}
; - $[3] = currInput; - $[4] = t3; - } else { - t3 = $[4]; - } - return t3; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{ input: "test" }], -}; - -``` - -### Eval output -(kind: exception) useState is not defined \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-local-state-and-component-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-local-state-and-component-scope.expect.md deleted file mode 100644 index 932e4fc9cc..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-local-state-and-component-scope.expect.md +++ /dev/null @@ -1,108 +0,0 @@ - -## Input - -```javascript -// @validateNoDerivedComputationsInEffects -import {useEffect, useState} from 'react'; - -function Component({firstName}) { - const [lastName, setLastName] = useState('Doe'); - const [fullName, setFullName] = useState('John'); - - const middleName = 'D.'; - - useEffect(() => { - setFullName(firstName + ' ' + middleName + ' ' + lastName); - }, [firstName, middleName, lastName]); - - return ( -
- setLastName(e.target.value)} /> -
{fullName}
-
- ); -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{firstName: 'John'}], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects -import { useEffect, useState } from "react"; - -function Component(t0) { - const $ = _c(12); - const { firstName } = t0; - const [lastName, setLastName] = useState("Doe"); - const [fullName, setFullName] = useState("John"); - let t1; - let t2; - if ($[0] !== firstName || $[1] !== lastName) { - t1 = () => { - setFullName(firstName + " " + "D." + " " + lastName); - }; - t2 = [firstName, "D.", lastName]; - $[0] = firstName; - $[1] = lastName; - $[2] = t1; - $[3] = t2; - } else { - t1 = $[2]; - t2 = $[3]; - } - useEffect(t1, t2); - let t3; - if ($[4] === Symbol.for("react.memo_cache_sentinel")) { - t3 = (e) => setLastName(e.target.value); - $[4] = t3; - } else { - t3 = $[4]; - } - let t4; - if ($[5] !== lastName) { - t4 = ; - $[5] = lastName; - $[6] = t4; - } else { - t4 = $[6]; - } - let t5; - if ($[7] !== fullName) { - t5 =
{fullName}
; - $[7] = fullName; - $[8] = t5; - } else { - t5 = $[8]; - } - let t6; - if ($[9] !== t4 || $[10] !== t5) { - t6 = ( -
- {t4} - {t5} -
- ); - $[9] = t4; - $[10] = t5; - $[11] = t6; - } else { - t6 = $[11]; - } - return t6; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{ firstName: "John" }], -}; - -``` - -### Eval output -(kind: ok)
John D. Doe
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-with-side-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-with-side-effect.expect.md deleted file mode 100644 index cc98724820..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-with-side-effect.expect.md +++ /dev/null @@ -1,69 +0,0 @@ - -## Input - -```javascript -// @validateNoDerivedComputationsInEffects - -function Component({value}) { - const [localValue, setLocalValue] = useState(''); - - useEffect(() => { - setLocalValue(value); - document.title = `Value: ${value}`; - }, [value]); - - return
{localValue}
; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{value: 'test'}], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects - -function Component(t0) { - const $ = _c(5); - const { value } = t0; - const [localValue, setLocalValue] = useState(""); - let t1; - let t2; - if ($[0] !== value) { - t1 = () => { - setLocalValue(value); - document.title = `Value: ${value}`; - }; - t2 = [value]; - $[0] = value; - $[1] = t1; - $[2] = t2; - } else { - t1 = $[1]; - t2 = $[2]; - } - useEffect(t1, t2); - let t3; - if ($[3] !== localValue) { - t3 =
{localValue}
; - $[3] = localValue; - $[4] = t3; - } else { - t3 = $[4]; - } - return t3; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{ value: "test" }], -}; - -``` - -### Eval output -(kind: exception) useState is not defined \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-contains-local-function-call.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-contains-local-function-call.expect.md deleted file mode 100644 index 36aa1ce15d..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-contains-local-function-call.expect.md +++ /dev/null @@ -1,86 +0,0 @@ - -## Input - -```javascript -// @validateNoDerivedComputationsInEffects -import {useEffect, useState} from 'react'; - -function Component({propValue}) { - const [value, setValue] = useState(null); - - function localFunction() { - console.log('local function'); - } - - useEffect(() => { - setValue(propValue); - localFunction(); - }, [propValue]); - - return
{value}
; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{propValue: 'test'}], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects -import { useEffect, useState } from "react"; - -function Component(t0) { - const $ = _c(6); - const { propValue } = t0; - const [value, setValue] = useState(null); - let t1; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - t1 = function localFunction() { - console.log("local function"); - }; - $[0] = t1; - } else { - t1 = $[0]; - } - const localFunction = t1; - let t2; - let t3; - if ($[1] !== propValue) { - t2 = () => { - setValue(propValue); - localFunction(); - }; - t3 = [propValue]; - $[1] = propValue; - $[2] = t2; - $[3] = t3; - } else { - t2 = $[2]; - t3 = $[3]; - } - useEffect(t2, t3); - let t4; - if ($[4] !== value) { - t4 =
{value}
; - $[4] = value; - $[5] = t4; - } else { - t4 = $[5]; - } - return t4; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{ propValue: "test" }], -}; - -``` - -### Eval output -(kind: ok)
test
-logs: ['local function'] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-conditionally-in-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-conditionally-in-effect.expect.md new file mode 100644 index 0000000000..24df0001ab --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-conditionally-in-effect.expect.md @@ -0,0 +1,47 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + +function Component({value, enabled}) { + const [localValue, setLocalValue] = useState(''); + + useEffect(() => { + if (enabled) { + setLocalValue(value); + } else { + setLocalValue('disabled'); + } + }, [value, enabled]); + + return
{localValue}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 'test', enabled: true}], +}; + +``` + + +## Error + +``` +Found 1 error: + +Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + +error.derived-state-conditionally-in-effect.ts:9:6 + 7 | useEffect(() => { + 8 | if (enabled) { +> 9 | setLocalValue(value); + | ^^^^^^^^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + 10 | } else { + 11 | setLocalValue('disabled'); + 12 | } +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-conditionally-in-effect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-conditionally-in-effect.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-conditionally-in-effect.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-conditionally-in-effect.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-default-props.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-default-props.expect.md new file mode 100644 index 0000000000..d374888897 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-default-props.expect.md @@ -0,0 +1,43 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects + +export default function Component(input = 'empty') { + const [currInput, setCurrInput] = useState(input); + const localConst = 'local const'; + + useEffect(() => { + setCurrInput(input + localConst); + }, [input, localConst]); + + return
{currInput}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{input: 'test'}], +}; + +``` + + +## Error + +``` +Found 1 error: + +Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + +error.derived-state-from-default-props.ts:8:4 + 6 | + 7 | useEffect(() => { +> 8 | setCurrInput(input + localConst); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + 9 | }, [input, localConst]); + 10 | + 11 | return
{currInput}
; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-default-props.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-default-props.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-default-props.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-default-props.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-local-state-and-component-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-local-state-and-component-scope.expect.md new file mode 100644 index 0000000000..ca7695e2ea --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-local-state-and-component-scope.expect.md @@ -0,0 +1,51 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + +function Component({firstName}) { + const [lastName, setLastName] = useState('Doe'); + const [fullName, setFullName] = useState('John'); + + const middleName = 'D.'; + + useEffect(() => { + setFullName(firstName + ' ' + middleName + ' ' + lastName); + }, [firstName, middleName, lastName]); + + return ( +
+ setLastName(e.target.value)} /> +
{fullName}
+
+ ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{firstName: 'John'}], +}; + +``` + + +## Error + +``` +Found 1 error: + +Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + +error.derived-state-from-prop-local-state-and-component-scope.ts:11:4 + 9 | + 10 | useEffect(() => { +> 11 | setFullName(firstName + ' ' + middleName + ' ' + lastName); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + 12 | }, [firstName, middleName, lastName]); + 13 | + 14 | return ( +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-local-state-and-component-scope.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-local-state-and-component-scope.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-local-state-and-component-scope.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-local-state-and-component-scope.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-setter-call-outside-effect-no-error.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-setter-call-outside-effect-no-error.expect.md index 44ca7ffa5a..5cc82027de 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-setter-call-outside-effect-no-error.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-setter-call-outside-effect-no-error.expect.md @@ -40,7 +40,7 @@ error.derived-state-from-prop-setter-call-outside-effect-no-error.ts:8:4 6 | 7 | useEffect(() => { > 8 | setName(initialName); - | ^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + | ^^^^^^^^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) 9 | }, [initialName]); 10 | 11 | return ( diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-setter-used-outside-effect-no-error.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-setter-used-outside-effect-no-error.expect.md index a271a4c54b..34f2384f03 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-setter-used-outside-effect-no-error.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-setter-used-outside-effect-no-error.expect.md @@ -37,7 +37,7 @@ error.derived-state-from-prop-setter-used-outside-effect-no-error.ts:11:4 9 | const [value, setValue] = useState(null); 10 | useEffect(() => { > 11 | setValue(propValue); - | ^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + | ^^^^^^^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) 12 | }, [propValue]); 13 | 14 | return ; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-with-side-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-with-side-effect.expect.md new file mode 100644 index 0000000000..fc06bdeb37 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-with-side-effect.expect.md @@ -0,0 +1,43 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects + +function Component({value}) { + const [localValue, setLocalValue] = useState(''); + + useEffect(() => { + setLocalValue(value); + document.title = `Value: ${value}`; + }, [value]); + + return
{localValue}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 'test'}], +}; + +``` + + +## Error + +``` +Found 1 error: + +Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + +error.derived-state-from-prop-with-side-effect.ts:7:4 + 5 | + 6 | useEffect(() => { +> 7 | setLocalValue(value); + | ^^^^^^^^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + 8 | document.title = `Value: ${value}`; + 9 | }, [value]); + 10 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-with-side-effect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-with-side-effect.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-with-side-effect.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-with-side-effect.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-ref-and-state-no-error.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-ref-and-state-no-error.expect.md new file mode 100644 index 0000000000..9f5783d41a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-ref-and-state-no-error.expect.md @@ -0,0 +1,45 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects +import {useEffect, useState, useRef} from 'react'; + +export default function Component({test}) { + const [local, setLocal] = useState(''); + + const myRef = useRef(null); + + useEffect(() => { + setLocal(myRef.current + test); + }, [test]); + + return <>{local}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{test: 'testString'}], +}; + +``` + + +## Error + +``` +Found 1 error: + +Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + +error.derived-state-from-ref-and-state-no-error.ts:10:4 + 8 | + 9 | useEffect(() => { +> 10 | setLocal(myRef.current + test); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + 11 | }, [test]); + 12 | + 13 | return <>{local}; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-ref-and-state-no-error.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-ref-and-state-no-error.js new file mode 100644 index 0000000000..dec6ae6daa --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-ref-and-state-no-error.js @@ -0,0 +1,19 @@ +// @validateNoDerivedComputationsInEffects +import {useEffect, useState, useRef} from 'react'; + +export default function Component({test}) { + const [local, setLocal] = useState(''); + + const myRef = useRef(null); + + useEffect(() => { + setLocal(myRef.current + test); + }, [test]); + + return <>{local}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{test: 'testString'}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.effect-contains-local-function-call.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.effect-contains-local-function-call.expect.md new file mode 100644 index 0000000000..7dc6a0ffcd --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.effect-contains-local-function-call.expect.md @@ -0,0 +1,48 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + +function Component({propValue}) { + const [value, setValue] = useState(null); + + function localFunction() { + console.log('local function'); + } + + useEffect(() => { + setValue(propValue); + localFunction(); + }, [propValue]); + + return
{value}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{propValue: 'test'}], +}; + +``` + + +## Error + +``` +Found 1 error: + +Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + +error.effect-contains-local-function-call.ts:12:4 + 10 | + 11 | useEffect(() => { +> 12 | setValue(propValue); + | ^^^^^^^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + 13 | localFunction(); + 14 | }, [propValue]); + 15 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-contains-local-function-call.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.effect-contains-local-function-call.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-contains-local-function-call.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.effect-contains-local-function-call.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.effect-contains-prop-function-call-no-error.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.effect-contains-prop-function-call-no-error.expect.md new file mode 100644 index 0000000000..31b7114af5 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.effect-contains-prop-function-call-no-error.expect.md @@ -0,0 +1,43 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + +function Component({propValue, onChange}) { + const [value, setValue] = useState(null); + useEffect(() => { + setValue(propValue); + onChange(); + }, [propValue]); + + return
{value}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{propValue: 'test', onChange: () => {}}], +}; + +``` + + +## Error + +``` +Found 1 error: + +Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + +error.effect-contains-prop-function-call-no-error.ts:7:4 + 5 | const [value, setValue] = useState(null); + 6 | useEffect(() => { +> 7 | setValue(propValue); + | ^^^^^^^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + 8 | onChange(); + 9 | }, [propValue]); + 10 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.effect-contains-prop-function-call-no-error.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.effect-contains-prop-function-call-no-error.js new file mode 100644 index 0000000000..aba975e899 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.effect-contains-prop-function-call-no-error.js @@ -0,0 +1,17 @@ +// @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + +function Component({propValue, onChange}) { + const [value, setValue] = useState(null); + useEffect(() => { + setValue(propValue); + onChange(); + }, [propValue]); + + return
{value}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{propValue: 'test', onChange: () => {}}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.effect-with-global-function-call-no-error.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.effect-with-global-function-call-no-error.expect.md index 90bef0c2e7..8115bb162b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.effect-with-global-function-call-no-error.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.effect-with-global-function-call-no-error.expect.md @@ -34,7 +34,7 @@ error.effect-with-global-function-call-no-error.ts:7:4 5 | const [value, setValue] = useState(null); 6 | useEffect(() => { > 7 | setValue(propValue); - | ^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + | ^^^^^^^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) 8 | globalCall(); 9 | }, [propValue]); 10 | diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-computation-in-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-computation-in-effect.expect.md new file mode 100644 index 0000000000..5e9efc3a33 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-computation-in-effect.expect.md @@ -0,0 +1,44 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects +function Component() { + const [firstName, setFirstName] = useState('Taylor'); + const lastName = 'Swift'; + + // 🔴 Avoid: redundant state and unnecessary Effect + const [fullName, setFullName] = useState(''); + useEffect(() => { + setFullName(firstName + ' ' + lastName); + }, [firstName, lastName]); + + return
{fullName}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [], +}; + +``` + + +## Error + +``` +Found 1 error: + +Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + +error.invalid-derived-computation-in-effect.ts:9:4 + 7 | const [fullName, setFullName] = useState(''); + 8 | useEffect(() => { +> 9 | setFullName(firstName + ' ' + lastName); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + 10 | }, [firstName, lastName]); + 11 | + 12 | return
{fullName}
; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-computation-in-effect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-computation-in-effect.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-computation-in-effect.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-computation-in-effect.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-state-from-computed-props.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-state-from-computed-props.expect.md new file mode 100644 index 0000000000..39d71798aa --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-state-from-computed-props.expect.md @@ -0,0 +1,44 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + +export default function Component(props) { + const [displayValue, setDisplayValue] = useState(''); + + useEffect(() => { + const computed = props.prefix + props.value + props.suffix; + setDisplayValue(computed); + }, [props.prefix, props.value, props.suffix]); + + return
{displayValue}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{prefix: '[', value: 'test', suffix: ']'}], +}; + +``` + + +## Error + +``` +Found 1 error: + +Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + +error.invalid-derived-state-from-computed-props.ts:9:4 + 7 | useEffect(() => { + 8 | const computed = props.prefix + props.value + props.suffix; +> 9 | setDisplayValue(computed); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + 10 | }, [props.prefix, props.value, props.suffix]); + 11 | + 12 | return
{displayValue}
; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-state-from-computed-props.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-state-from-computed-props.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-state-from-computed-props.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-state-from-computed-props.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-state-from-destructured-props.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-state-from-destructured-props.expect.md new file mode 100644 index 0000000000..7f3f807568 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-state-from-destructured-props.expect.md @@ -0,0 +1,45 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + +export default function Component({props}) { + const [fullName, setFullName] = useState( + props.firstName + ' ' + props.lastName + ); + + useEffect(() => { + setFullName(props.firstName + ' ' + props.lastName); + }, [props.firstName, props.lastName]); + + return
{fullName}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{props: {firstName: 'John', lastName: 'Doe'}}], +}; + +``` + + +## Error + +``` +Found 1 error: + +Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + +error.invalid-derived-state-from-destructured-props.ts:10:4 + 8 | + 9 | useEffect(() => { +> 10 | setFullName(props.firstName + ' ' + props.lastName); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + 11 | }, [props.firstName, props.lastName]); + 12 | + 13 | return
{fullName}
; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-state-from-destructured-props.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-state-from-destructured-props.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-state-from-destructured-props.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-state-from-destructured-props.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.ref-conditional-in-effect-no-error.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.ref-conditional-in-effect-no-error.expect.md new file mode 100644 index 0000000000..5aad420e8e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.ref-conditional-in-effect-no-error.expect.md @@ -0,0 +1,60 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects +import {useEffect, useState, useRef} from 'react'; + +export default function Component({test}) { + const [local, setLocal] = useState(0); + + const myRef = useRef(null); + + useEffect(() => { + if (myRef.current) { + setLocal(test); + } else { + setLocal(test + test); + } + }, [test]); + + return <>{local}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{test: 4}], +}; + +``` + + +## Error + +``` +Found 2 errors: + +Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + +error.ref-conditional-in-effect-no-error.ts:11:6 + 9 | useEffect(() => { + 10 | if (myRef.current) { +> 11 | setLocal(test); + | ^^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + 12 | } else { + 13 | setLocal(test + test); + 14 | } + +Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + +error.ref-conditional-in-effect-no-error.ts:13:6 + 11 | setLocal(test); + 12 | } else { +> 13 | setLocal(test + test); + | ^^^^^^^^^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + 14 | } + 15 | }, [test]); + 16 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.ref-conditional-in-effect-no-error.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.ref-conditional-in-effect-no-error.js new file mode 100644 index 0000000000..3d0e27126a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.ref-conditional-in-effect-no-error.js @@ -0,0 +1,23 @@ +// @validateNoDerivedComputationsInEffects +import {useEffect, useState, useRef} from 'react'; + +export default function Component({test}) { + const [local, setLocal] = useState(0); + + const myRef = useRef(null); + + useEffect(() => { + if (myRef.current) { + setLocal(test); + } else { + setLocal(test + test); + } + }, [test]); + + return <>{local}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{test: 4}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-computation-in-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-computation-in-effect.expect.md deleted file mode 100644 index b26a51832e..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-computation-in-effect.expect.md +++ /dev/null @@ -1,69 +0,0 @@ - -## Input - -```javascript -// @validateNoDerivedComputationsInEffects -function Component() { - const [firstName, setFirstName] = useState('Taylor'); - const lastName = 'Swift'; - - // 🔴 Avoid: redundant state and unnecessary Effect - const [fullName, setFullName] = useState(''); - useEffect(() => { - setFullName(firstName + ' ' + lastName); - }, [firstName, lastName]); - - return
{fullName}
; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects -function Component() { - const $ = _c(5); - const [firstName] = useState("Taylor"); - - const [fullName, setFullName] = useState(""); - let t0; - let t1; - if ($[0] !== firstName) { - t0 = () => { - setFullName(firstName + " " + "Swift"); - }; - t1 = [firstName, "Swift"]; - $[0] = firstName; - $[1] = t0; - $[2] = t1; - } else { - t0 = $[1]; - t1 = $[2]; - } - useEffect(t0, t1); - let t2; - if ($[3] !== fullName) { - t2 =
{fullName}
; - $[3] = fullName; - $[4] = t2; - } else { - t2 = $[4]; - } - return t2; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [], -}; - -``` - -### Eval output -(kind: exception) useState is not defined \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-state-from-computed-props.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-state-from-computed-props.expect.md deleted file mode 100644 index c8d60f981b..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-state-from-computed-props.expect.md +++ /dev/null @@ -1,72 +0,0 @@ - -## Input - -```javascript -// @validateNoDerivedComputationsInEffects -import {useEffect, useState} from 'react'; - -export default function Component(props) { - const [displayValue, setDisplayValue] = useState(''); - - useEffect(() => { - const computed = props.prefix + props.value + props.suffix; - setDisplayValue(computed); - }, [props.prefix, props.value, props.suffix]); - - return
{displayValue}
; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{prefix: '[', value: 'test', suffix: ']'}], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects -import { useEffect, useState } from "react"; - -export default function Component(props) { - const $ = _c(7); - const [displayValue, setDisplayValue] = useState(""); - let t0; - let t1; - if ($[0] !== props.prefix || $[1] !== props.suffix || $[2] !== props.value) { - t0 = () => { - const computed = props.prefix + props.value + props.suffix; - setDisplayValue(computed); - }; - t1 = [props.prefix, props.value, props.suffix]; - $[0] = props.prefix; - $[1] = props.suffix; - $[2] = props.value; - $[3] = t0; - $[4] = t1; - } else { - t0 = $[3]; - t1 = $[4]; - } - useEffect(t0, t1); - let t2; - if ($[5] !== displayValue) { - t2 =
{displayValue}
; - $[5] = displayValue; - $[6] = t2; - } else { - t2 = $[6]; - } - return t2; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{ prefix: "[", value: "test", suffix: "]" }], -}; - -``` - -### Eval output -(kind: ok)
[test]
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-state-from-destructured-props.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-state-from-destructured-props.expect.md deleted file mode 100644 index c06b8772e4..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-state-from-destructured-props.expect.md +++ /dev/null @@ -1,74 +0,0 @@ - -## Input - -```javascript -// @validateNoDerivedComputationsInEffects -import {useEffect, useState} from 'react'; - -export default function Component({props}) { - const [fullName, setFullName] = useState( - props.firstName + ' ' + props.lastName - ); - - useEffect(() => { - setFullName(props.firstName + ' ' + props.lastName); - }, [props.firstName, props.lastName]); - - return
{fullName}
; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{props: {firstName: 'John', lastName: 'Doe'}}], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects -import { useEffect, useState } from "react"; - -export default function Component(t0) { - const $ = _c(6); - const { props } = t0; - const [fullName, setFullName] = useState( - props.firstName + " " + props.lastName, - ); - let t1; - let t2; - if ($[0] !== props.firstName || $[1] !== props.lastName) { - t1 = () => { - setFullName(props.firstName + " " + props.lastName); - }; - t2 = [props.firstName, props.lastName]; - $[0] = props.firstName; - $[1] = props.lastName; - $[2] = t1; - $[3] = t2; - } else { - t1 = $[2]; - t2 = $[3]; - } - useEffect(t1, t2); - let t3; - if ($[4] !== fullName) { - t3 =
{fullName}
; - $[4] = fullName; - $[5] = t3; - } else { - t3 = $[5]; - } - return t3; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{ props: { firstName: "John", lastName: "Doe" } }], -}; - -``` - -### Eval output -(kind: ok)
John Doe
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-derived-computation-in-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-derived-computation-in-effect.expect.md index 3d53941f44..4915e08a4a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-derived-computation-in-effect.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-derived-computation-in-effect.expect.md @@ -30,7 +30,7 @@ error.invalid-derived-computation-in-effect.ts:9:4 7 | const [fullName, setFullName] = useState(''); 8 | useEffect(() => { > 9 | setFullName(firstName + ' ' + lastName); - | ^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) 10 | }, [firstName, lastName]); 11 | 12 | return
{fullName}
;