diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts index 1085d4c69e..a83b22651e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -103,6 +103,7 @@ import {validateNoFreezingKnownMutableFunctions} from '../Validation/ValidateNoF import {inferMutationAliasingEffects} from '../Inference/InferMutationAliasingEffects'; import {inferMutationAliasingRanges} from '../Inference/InferMutationAliasingRanges'; import {validateNoDerivedComputationsInEffects} from '../Validation/ValidateNoDerivedComputationsInEffects'; +import {validateNoDerivedComputationsInEffects_exp} from '../Validation/ValidateNoDerivedComputationsInEffects_exp'; import {nameAnonymousFunctions} from '../Transform/NameAnonymousFunctions'; export type CompilerPipelineValue = @@ -275,6 +276,10 @@ function runWithEnvironment( validateNoDerivedComputationsInEffects(hir); } + if (env.config.validateNoDerivedComputationsInEffects_exp) { + validateNoDerivedComputationsInEffects_exp(hir); + } + if (env.config.validateNoSetStateInEffects) { env.logErrors(validateNoSetStateInEffects(hir, env)); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index 57567f325f..5712526a34 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -334,6 +334,12 @@ export const EnvironmentConfigSchema = z.object({ */ validateNoDerivedComputationsInEffects: z.boolean().default(false), + /** + * Experimental: Validates that effects are not used to calculate derived data which could instead be computed + * during render. Generates a custom error message for each type of violation. + */ + validateNoDerivedComputationsInEffects_exp: z.boolean().default(false), + /** * Validates against creating JSX within a try block and recommends using an error boundary * instead. diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects_exp.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects_exp.ts new file mode 100644 index 0000000000..5aafa19a2f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects_exp.ts @@ -0,0 +1,422 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import {CompilerError, Effect} from '..'; +import {ErrorCategory} from '../CompilerError'; +import { + BlockId, + FunctionExpression, + HIRFunction, + IdentifierId, + isSetStateType, + isUseEffectHookType, + Place, + CallExpression, + Instruction, + isUseStateType, + BasicBlock, + isUseRefType, +} from '../HIR'; +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; +}; + +type ValidationContext = { + readonly functions: Map; + readonly errors: CompilerError; + readonly derivationCache: DerivationCache; + readonly effects: Set; +}; + +class DerivationCache { + hasChanges: boolean = false; + cache: Map = new Map(); + + snapshot(): boolean { + const hasChanges = this.hasChanges; + this.hasChanges = false; + return hasChanges; + } + + addDerivationEntry( + derivedVar: Place, + sourcesIds: Set, + typeOfValue: TypeOfValue, + ): void { + let newValue: DerivationMetadata = { + place: derivedVar, + sourcesIds: new Set(), + typeOfValue: typeOfValue ?? 'ignored', + }; + + if (sourcesIds !== undefined) { + for (const id of sourcesIds) { + const sourcePlace = this.cache.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); + } + } + } + + if (newValue.sourcesIds.size === 0) { + newValue.sourcesIds.add(derivedVar.identifier.id); + } + + const existingValue = this.cache.get(derivedVar.identifier.id); + if ( + existingValue === undefined || + !this.isDerivationEqual(existingValue, newValue) + ) { + this.cache.set(derivedVar.identifier.id, newValue); + this.hasChanges = true; + } + } + + private isDerivationEqual( + a: DerivationMetadata, + b: DerivationMetadata, + ): boolean { + if (a.typeOfValue !== b.typeOfValue) { + return false; + } + if (a.sourcesIds.size !== b.sourcesIds.size) { + return false; + } + for (const id of a.sourcesIds) { + if (!b.sourcesIds.has(id)) { + return false; + } + } + return true; + } +} + +/** + * Validates that useEffect is not used for derived computations which could/should + * be performed in render. + * + * See https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state + * + * Example: + * + * ``` + * // 🔴 Avoid: redundant state and unnecessary Effect + * const [fullName, setFullName] = useState(''); + * useEffect(() => { + * setFullName(firstName + ' ' + lastName); + * }, [firstName, lastName]); + * ``` + * + * Instead use: + * + * ``` + * // ✅ Good: calculated during rendering + * const fullName = firstName + ' ' + lastName; + * ``` + */ +export function validateNoDerivedComputationsInEffects_exp( + fn: HIRFunction, +): void { + const functions: Map = new Map(); + const derivationCache = new DerivationCache(); + const errors = new CompilerError(); + const effects: Set = new Set(); + + const context: ValidationContext = { + functions, + errors, + derivationCache, + effects, + }; + + if (fn.fnType === 'Hook') { + for (const param of fn.params) { + if (param.kind === 'Identifier') { + context.derivationCache.cache.set(param.identifier.id, { + place: param, + sourcesIds: new Set([param.identifier.id]), + typeOfValue: 'fromProps', + }); + context.derivationCache.hasChanges = true; + } + } + } else if (fn.fnType === 'Component') { + const props = fn.params[0]; + if (props != null && props.kind === 'Identifier') { + context.derivationCache.cache.set(props.identifier.id, { + place: props, + sourcesIds: new Set([props.identifier.id]), + typeOfValue: 'fromProps', + }); + context.derivationCache.hasChanges = true; + } + } + + do { + for (const block of fn.body.blocks.values()) { + recordPhiDerivations(block, context); + for (const instr of block.instructions) { + recordInstructionDerivations(instr, context); + } + } + } while (context.derivationCache.snapshot()); + + for (const effect of effects) { + validateEffect(effect, context); + } + + if (errors.hasAnyErrors()) { + throw errors; + } +} + +function recordPhiDerivations( + block: BasicBlock, + context: ValidationContext, +): void { + for (const phi of block.phis) { + let typeOfValue: TypeOfValue = 'ignored'; + let sourcesIds: Set = new Set(); + for (const operand of phi.operands.values()) { + const operandMetadata = context.derivationCache.cache.get( + operand.identifier.id, + ); + + if (operandMetadata === undefined) { + continue; + } + + typeOfValue = joinValue(typeOfValue, operandMetadata.typeOfValue); + sourcesIds.add(operand.identifier.id); + } + + if (typeOfValue !== 'ignored') { + context.derivationCache.addDerivationEntry( + phi.place, + sourcesIds, + typeOfValue, + ); + } + } +} + +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 recordInstructionDerivations( + instr: Instruction, + context: ValidationContext, +): void { + let typeOfValue: TypeOfValue = 'ignored'; + const sources: Set = new Set(); + const {lvalue, value} = instr; + if (value.kind === 'FunctionExpression') { + context.functions.set(lvalue.identifier.id, value); + for (const [, block] of value.loweredFunc.func.body.blocks) { + for (const instr of block.instructions) { + recordInstructionDerivations(instr, context); + } + } + } 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' + ) { + const effectFunction = context.functions.get(value.args[0].identifier.id); + if (effectFunction != null) { + context.effects.add(effectFunction.loweredFunc.func); + } + } else if (isUseStateType(lvalue.identifier) && value.args.length > 0) { + 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 = context.derivationCache.cache.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)) { + context.derivationCache.addDerivationEntry(lvalue, sources, typeOfValue); + } + + 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)) { + context.derivationCache.addDerivationEntry( + operand, + sources, + typeOfValue, + ); + } + 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: operand.loc, + message: 'Unexpected unknown effect', + }, + ], + }); + } + default: { + assertExhaustive( + operand.effect, + `Unexpected effect kind \`${operand.effect}\``, + ); + } + } + } +} + +function validateEffect( + effectFunction: HIRFunction, + context: ValidationContext, +): void { + const seenBlocks: Set = new Set(); + + const effectDerivedSetStateCalls: Array<{ + value: CallExpression; + sourceIds: Set; + }> = []; + + const globals: Set = new Set(); + for (const block of effectFunction.body.blocks.values()) { + for (const pred of block.preds) { + if (!seenBlocks.has(pred)) { + // skip if block has a back edge + return; + } + } + + for (const instr of block.instructions) { + // Early return if any instruction is deriving a value from a ref + if (isUseRefType(instr.lvalue.identifier)) { + return; + } + + if ( + instr.value.kind === 'CallExpression' && + isSetStateType(instr.value.callee.identifier) && + instr.value.args.length === 1 && + instr.value.args[0].kind === 'Identifier' + ) { + const argMetadata = context.derivationCache.cache.get( + instr.value.args[0].identifier.id, + ); + + if (argMetadata !== undefined) { + effectDerivedSetStateCalls.push({ + value: instr.value, + sourceIds: argMetadata.sourcesIds, + }); + } + } else if (instr.value.kind === 'CallExpression') { + const calleeMetadata = context.derivationCache.cache.get( + instr.value.callee.identifier.id, + ); + + if ( + calleeMetadata !== undefined && + (calleeMetadata.typeOfValue === 'fromProps' || + calleeMetadata.typeOfValue === 'fromPropsAndState') + ) { + // If the callee is a prop we can't confidently say that it should be derived in render + return; + } + + if (globals.has(instr.value.callee.identifier.id)) { + // If the callee is a global we can't confidently say that it should be derived in render + return; + } + } else if (instr.value.kind === 'LoadGlobal') { + globals.add(instr.lvalue.identifier.id); + for (const operand of eachInstructionOperand(instr)) { + globals.add(operand.identifier.id); + } + } + } + seenBlocks.add(block.id); + } + + for (const derivedSetStateCall of effectDerivedSetStateCalls) { + context.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: derivedSetStateCall.value.callee.loc, + suggestions: null, + }); + } +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-ref-and-state-no-error.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-ref-and-state-no-error.expect.md new file mode 100644 index 0000000000..4d0b6663e3 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-ref-and-state-no-error.expect.md @@ -0,0 +1,73 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp +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'}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp +import { useEffect, useState, useRef } from "react"; + +export default function Component(t0) { + const $ = _c(5); + const { test } = t0; + const [local, setLocal] = useState(""); + + const myRef = useRef(null); + let t1; + let t2; + if ($[0] !== test) { + t1 = () => { + setLocal(myRef.current + test); + }; + t2 = [test]; + $[0] = test; + $[1] = t1; + $[2] = t2; + } else { + t1 = $[1]; + t2 = $[2]; + } + useEffect(t1, t2); + let t3; + if ($[3] !== local) { + t3 = <>{local}; + $[3] = local; + $[4] = t3; + } else { + t3 = $[4]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ test: "testString" }], +}; + +``` + +### Eval output +(kind: ok) nulltestString \ 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-ref-and-state-no-error.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-ref-and-state-no-error.js new file mode 100644 index 0000000000..6b24f73ac7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-ref-and-state-no-error.js @@ -0,0 +1,19 @@ +// @validateNoDerivedComputationsInEffects_exp +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/effect-contains-prop-function-call-no-error.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-contains-prop-function-call-no-error.expect.md new file mode 100644 index 0000000000..c83ea552a6 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-contains-prop-function-call-no-error.expect.md @@ -0,0 +1,75 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp +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: () => {}}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp +import { useEffect, useState } from "react"; + +function Component(t0) { + const $ = _c(7); + const { propValue, onChange } = t0; + const [value, setValue] = useState(null); + let t1; + if ($[0] !== onChange || $[1] !== propValue) { + t1 = () => { + setValue(propValue); + onChange(); + }; + $[0] = onChange; + $[1] = propValue; + $[2] = t1; + } else { + t1 = $[2]; + } + let t2; + if ($[3] !== propValue) { + t2 = [propValue]; + $[3] = propValue; + $[4] = t2; + } else { + t2 = $[4]; + } + useEffect(t1, t2); + let t3; + if ($[5] !== value) { + t3 =
{value}
; + $[5] = value; + $[6] = t3; + } else { + t3 = $[6]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ propValue: "test", onChange: () => {} }], +}; + +``` + +### 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/effect-contains-prop-function-call-no-error.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-contains-prop-function-call-no-error.js new file mode 100644 index 0000000000..512df7cb36 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-contains-prop-function-call-no-error.js @@ -0,0 +1,17 @@ +// @validateNoDerivedComputationsInEffects_exp +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/effect-with-global-function-call-no-error.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-with-global-function-call-no-error.expect.md new file mode 100644 index 0000000000..e17f1e26f6 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-with-global-function-call-no-error.expect.md @@ -0,0 +1,70 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp +import {useEffect, useState} from 'react'; + +function Component({propValue}) { + const [value, setValue] = useState(null); + useEffect(() => { + setValue(propValue); + globalCall(); + }, [propValue]); + + return
{value}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{propValue: 'test'}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp +import { useEffect, useState } from "react"; + +function Component(t0) { + const $ = _c(5); + const { propValue } = t0; + const [value, setValue] = useState(null); + let t1; + let t2; + if ($[0] !== propValue) { + t1 = () => { + setValue(propValue); + globalCall(); + }; + t2 = [propValue]; + $[0] = propValue; + $[1] = t1; + $[2] = t2; + } else { + t1 = $[1]; + t2 = $[2]; + } + useEffect(t1, t2); + let t3; + if ($[3] !== value) { + t3 =
{value}
; + $[3] = value; + $[4] = t3; + } else { + t3 = $[4]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ propValue: "test" }], +}; + +``` + +### Eval output +(kind: exception) globalCall 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-with-global-function-call-no-error.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-with-global-function-call-no-error.js new file mode 100644 index 0000000000..4cded6dcc8 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-with-global-function-call-no-error.js @@ -0,0 +1,17 @@ +// @validateNoDerivedComputationsInEffects_exp +import {useEffect, useState} from 'react'; + +function Component({propValue}) { + const [value, setValue] = useState(null); + useEffect(() => { + setValue(propValue); + globalCall(); + }, [propValue]); + + return
{value}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{propValue: 'test'}], +}; 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..8d378e4a39 --- /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_exp +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/error.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 new file mode 100644 index 0000000000..2ccd52500c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-conditionally-in-effect.js @@ -0,0 +1,21 @@ +// @validateNoDerivedComputationsInEffects_exp +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}], +}; 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..8e1ec8ae5f --- /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,44 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp +import {useEffect, useState} from 'react'; + +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:9:4 + 7 | + 8 | useEffect(() => { +> 9 | 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) + 10 | }, [input, localConst]); + 11 | + 12 | return
{currInput}
; +``` + + \ 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-default-props.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-default-props.js new file mode 100644 index 0000000000..1a0f5126e7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-default-props.js @@ -0,0 +1,18 @@ +// @validateNoDerivedComputationsInEffects_exp +import {useEffect, useState} from 'react'; + +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'}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-local-state-in-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-local-state-in-effect.expect.md new file mode 100644 index 0000000000..2723d7a799 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-local-state-in-effect.expect.md @@ -0,0 +1,41 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp + +import {useEffect, useState} from 'react'; + +function Component({shouldChange}) { + const [count, setCount] = useState(0); + + useEffect(() => { + if (shouldChange) { + setCount(count + 1); + } + }, [count]); + + return
{count}
; +} + +``` + + +## 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-local-state-in-effect.ts:10:6 + 8 | useEffect(() => { + 9 | if (shouldChange) { +> 10 | setCount(count + 1); + | ^^^^^^^^ 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 | } + 12 | }, [count]); + 13 | +``` + + \ 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-local-state-in-effect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-local-state-in-effect.js new file mode 100644 index 0000000000..9568e49002 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-local-state-in-effect.js @@ -0,0 +1,15 @@ +// @validateNoDerivedComputationsInEffects_exp + +import {useEffect, useState} from 'react'; + +function Component({shouldChange}) { + const [count, setCount] = useState(0); + + useEffect(() => { + if (shouldChange) { + setCount(count + 1); + } + }, [count]); + + return
{count}
; +} 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..48c173028e --- /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_exp +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/error.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 new file mode 100644 index 0000000000..3090ef0041 --- /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.js @@ -0,0 +1,25 @@ +// @validateNoDerivedComputationsInEffects_exp +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'}], +}; 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 new file mode 100644 index 0000000000..1b204093b0 --- /dev/null +++ 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 @@ -0,0 +1,47 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp +import {useEffect, useState} from 'react'; + +function Component({initialName}) { + const [name, setName] = useState(''); + + useEffect(() => { + setName(initialName); + }, [initialName]); + + return ( +
+ setName(e.target.value)} /> +
+ ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{initialName: '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-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) + 9 | }, [initialName]); + 10 | + 11 | return ( +``` + + \ 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-prop-setter-call-outside-effect-no-error.js 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.js new file mode 100644 index 0000000000..502402be51 --- /dev/null +++ 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.js @@ -0,0 +1,21 @@ +// @validateNoDerivedComputationsInEffects_exp +import {useEffect, useState} from 'react'; + +function Component({initialName}) { + const [name, setName] = useState(''); + + useEffect(() => { + setName(initialName); + }, [initialName]); + + return ( +
+ setName(e.target.value)} /> +
+ ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{initialName: 'John'}], +}; 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 new file mode 100644 index 0000000000..f3f45a24ef --- /dev/null +++ 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 @@ -0,0 +1,46 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp +import {useEffect, useState} from 'react'; + +function MockComponent({onSet}) { + return
onSet('clicked')}>Mock Component
; +} + +function Component({propValue}) { + const [value, setValue] = useState(null); + useEffect(() => { + setValue(propValue); + }, [propValue]); + + return ; +} + +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.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) + 12 | }, [propValue]); + 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/error.derived-state-from-prop-setter-used-outside-effect-no-error.js 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.js new file mode 100644 index 0000000000..d33af16ec5 --- /dev/null +++ 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.js @@ -0,0 +1,20 @@ +// @validateNoDerivedComputationsInEffects_exp +import {useEffect, useState} from 'react'; + +function MockComponent({onSet}) { + return
onSet('clicked')}>Mock Component
; +} + +function Component({propValue}) { + const [value, setValue] = useState(null); + useEffect(() => { + setValue(propValue); + }, [propValue]); + + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{propValue: 'test'}], +}; 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..2577db54be --- /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,44 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp +import {useEffect, useState} from 'react'; + +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:8:4 + 6 | + 7 | useEffect(() => { +> 8 | 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) + 9 | document.title = `Value: ${value}`; + 10 | }, [value]); + 11 | +``` + + \ 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-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 new file mode 100644 index 0000000000..88c66ce1ef --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-with-side-effect.js @@ -0,0 +1,18 @@ +// @validateNoDerivedComputationsInEffects_exp +import {useEffect, useState} from 'react'; + +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'}], +}; 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..d389851d7f --- /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_exp +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/error.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 new file mode 100644 index 0000000000..1efb3177e5 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.effect-contains-local-function-call.js @@ -0,0 +1,22 @@ +// @validateNoDerivedComputationsInEffects_exp +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'}], +}; 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..f06f726927 --- /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,46 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp +import {useEffect, useState} from 'react'; + +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:11:4 + 9 | const [fullName, setFullName] = useState(''); + 10 | useEffect(() => { +> 11 | 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) + 12 | }, [firstName, lastName]); + 13 | + 14 | return
{fullName}
; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.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 new file mode 100644 index 0000000000..17779a5b4c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-computation-in-effect.js @@ -0,0 +1,20 @@ +// @validateNoDerivedComputationsInEffects_exp +import {useEffect, useState} from 'react'; + +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: [], +}; 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..4f0469b791 --- /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_exp +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/error.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 new file mode 100644 index 0000000000..24afa944fc --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-state-from-computed-props.js @@ -0,0 +1,18 @@ +// @validateNoDerivedComputationsInEffects_exp +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: ']'}], +}; 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..309e9f73fa --- /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_exp +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/error.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 new file mode 100644 index 0000000000..bdfb47a2c6 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-state-from-destructured-props.js @@ -0,0 +1,19 @@ +// @validateNoDerivedComputationsInEffects_exp +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'}}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/ref-conditional-in-effect-no-error.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/ref-conditional-in-effect-no-error.expect.md new file mode 100644 index 0000000000..365ee1fef4 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/ref-conditional-in-effect-no-error.expect.md @@ -0,0 +1,82 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp +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}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp +import { useEffect, useState, useRef } from "react"; + +export default function Component(t0) { + const $ = _c(5); + const { test } = t0; + const [local, setLocal] = useState(0); + + const myRef = useRef(null); + let t1; + let t2; + if ($[0] !== test) { + t1 = () => { + if (myRef.current) { + setLocal(test); + } else { + setLocal(test + test); + } + }; + + t2 = [test]; + $[0] = test; + $[1] = t1; + $[2] = t2; + } else { + t1 = $[1]; + t2 = $[2]; + } + useEffect(t1, t2); + let t3; + if ($[3] !== local) { + t3 = <>{local}; + $[3] = local; + $[4] = t3; + } else { + t3 = $[4]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ test: 4 }], +}; + +``` + +### Eval output +(kind: ok) 8 \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/ref-conditional-in-effect-no-error.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/ref-conditional-in-effect-no-error.js new file mode 100644 index 0000000000..ee59ccb78f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/ref-conditional-in-effect-no-error.js @@ -0,0 +1,23 @@ +// @validateNoDerivedComputationsInEffects_exp +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/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 d97a665ae6..ccbcf68a57 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 @@ -3,6 +3,8 @@ ```javascript // @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + function BadExample() { const [firstName, setFirstName] = useState('Taylor'); const [lastName, setLastName] = useState('Swift'); @@ -10,7 +12,7 @@ function BadExample() { // 🔴 Avoid: redundant state and unnecessary Effect const [fullName, setFullName] = useState(''); useEffect(() => { - setFullName(capitalize(firstName + ' ' + lastName)); + setFullName(firstName + ' ' + lastName); }, [firstName, lastName]); return
{fullName}
; @@ -26,14 +28,14 @@ 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(capitalize(firstName + ' ' + lastName)); +error.invalid-derived-computation-in-effect.ts:11:4 + 9 | const [fullName, setFullName] = useState(''); + 10 | useEffect(() => { +> 11 | 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}
; + 12 | }, [firstName, lastName]); + 13 | + 14 | return
{fullName}
; ``` \ 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.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-derived-computation-in-effect.js index d803d3c4a3..0209b47ce3 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-derived-computation-in-effect.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-derived-computation-in-effect.js @@ -1,4 +1,6 @@ // @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + function BadExample() { const [firstName, setFirstName] = useState('Taylor'); const [lastName, setLastName] = useState('Swift'); @@ -6,7 +8,7 @@ function BadExample() { // 🔴 Avoid: redundant state and unnecessary Effect const [fullName, setFullName] = useState(''); useEffect(() => { - setFullName(capitalize(firstName + ' ' + lastName)); + setFullName(firstName + ' ' + lastName); }, [firstName, lastName]); return
{fullName}
;