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 a83b22651e741..0c777f87707da 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -277,7 +277,7 @@ function runWithEnvironment( } if (env.config.validateNoDerivedComputationsInEffects_exp) { - validateNoDerivedComputationsInEffects_exp(hir); + env.logErrors(validateNoDerivedComputationsInEffects_exp(hir)); } if (env.config.validateNoSetStateInEffects) { 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 index a755d0e2c65fb..28d19fb1dbfbc 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects_exp.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects_exp.ts @@ -5,6 +5,7 @@ * LICENSE file in the root directory of this source tree. */ +import {Result} from '../Utils/Result'; import {CompilerDiagnostic, CompilerError, Effect} from '..'; import {ErrorCategory} from '../CompilerError'; import { @@ -47,6 +48,43 @@ type ValidationContext = { class DerivationCache { hasChanges: boolean = false; cache: Map = new Map(); + private previousCache: Map | null = null; + + takeSnapshot(): void { + this.previousCache = new Map(); + for (const [key, value] of this.cache.entries()) { + this.previousCache.set(key, { + place: value.place, + sourcesIds: new Set(value.sourcesIds), + typeOfValue: value.typeOfValue, + }); + } + } + + checkForChanges(): void { + if (this.previousCache === null) { + this.hasChanges = true; + return; + } + + for (const [key, value] of this.cache.entries()) { + const previousValue = this.previousCache.get(key); + if ( + previousValue === undefined || + !this.isDerivationEqual(previousValue, value) + ) { + this.hasChanges = true; + return; + } + } + + if (this.cache.size !== this.previousCache.size) { + this.hasChanges = true; + return; + } + + this.hasChanges = false; + } snapshot(): boolean { const hasChanges = this.hasChanges; @@ -65,41 +103,27 @@ class DerivationCache { typeOfValue: typeOfValue ?? 'ignored', }; - if (sourcesIds !== undefined) { - for (const id of sourcesIds) { - const sourcePlace = this.cache.get(id)?.place; + if (isNamedIdentifier(derivedVar)) { + newValue.sourcesIds.add(derivedVar.identifier.id); + } - if (sourcePlace === undefined) { - continue; - } + for (const id of sourcesIds) { + const sourceMetadata = this.cache.get(id); - /* - * 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 (sourceMetadata === undefined) { + continue; } - } - if (newValue.sourcesIds.size === 0) { - newValue.sourcesIds.add(derivedVar.identifier.id); + if (isNamedIdentifier(sourceMetadata.place)) { + newValue.sourcesIds.add(sourceMetadata.place.identifier.id); + } else { + for (const sourcesSourceId of sourceMetadata.sourcesIds) { + newValue.sourcesIds.add(sourcesSourceId); + } + } } - 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; - } + this.cache.set(derivedVar.identifier.id, newValue); } private isDerivationEqual( @@ -121,6 +145,12 @@ class DerivationCache { } } +function isNamedIdentifier(place: Place): boolean { + return ( + place.identifier.name !== null && place.identifier.name?.kind === 'named' + ); +} + /** * Validates that useEffect is not used for derived computations which could/should * be performed in render. @@ -146,7 +176,7 @@ class DerivationCache { */ export function validateNoDerivedComputationsInEffects_exp( fn: HIRFunction, -): void { +): Result { const functions: Map = new Map(); const derivationCache = new DerivationCache(); const errors = new CompilerError(); @@ -172,10 +202,11 @@ export function validateNoDerivedComputationsInEffects_exp( if (param.kind === 'Identifier') { context.derivationCache.cache.set(param.identifier.id, { place: param, - sourcesIds: new Set([param.identifier.id]), + sourcesIds: new Set( + isNamedIdentifier(param) ? [param.identifier.id] : [], + ), typeOfValue: 'fromProps', }); - context.derivationCache.hasChanges = true; } } } else if (fn.fnType === 'Component') { @@ -183,15 +214,18 @@ export function validateNoDerivedComputationsInEffects_exp( if (props != null && props.kind === 'Identifier') { context.derivationCache.cache.set(props.identifier.id, { place: props, - sourcesIds: new Set([props.identifier.id]), + sourcesIds: new Set( + isNamedIdentifier(props) ? [props.identifier.id] : [], + ), typeOfValue: 'fromProps', }); - context.derivationCache.hasChanges = true; } } let isFirstPass = true; do { + context.derivationCache.takeSnapshot(); + for (const block of fn.body.blocks.values()) { recordPhiDerivations(block, context); for (const instr of block.instructions) { @@ -199,6 +233,7 @@ export function validateNoDerivedComputationsInEffects_exp( } } + context.derivationCache.checkForChanges(); isFirstPass = false; } while (context.derivationCache.snapshot()); @@ -206,9 +241,7 @@ export function validateNoDerivedComputationsInEffects_exp( validateEffect(effect, context); } - if (errors.hasAnyErrors()) { - throw errors; - } + return errors.asResult(); } function recordPhiDerivations( @@ -262,6 +295,7 @@ function recordInstructionDerivations( if (value.kind === 'FunctionExpression') { context.functions.set(lvalue.identifier.id, value); for (const [, block] of value.loweredFunc.func.body.blocks) { + recordPhiDerivations(block, context); for (const instr of block.instructions) { recordInstructionDerivations(instr, context, isFirstPass); } @@ -310,9 +344,7 @@ function recordInstructionDerivations( } typeOfValue = joinValue(typeOfValue, operandMetadata.typeOfValue); - for (const id of operandMetadata.sourcesIds) { - sources.add(id); - } + sources.add(operand.identifier.id); } if (typeOfValue === 'ignored') { @@ -331,11 +363,24 @@ function recordInstructionDerivations( case Effect.ConditionallyMutateIterator: case Effect.Mutate: { if (isMutable(instr, operand)) { - context.derivationCache.addDerivationEntry( - operand, - sources, - typeOfValue, - ); + if (context.derivationCache.cache.has(operand.identifier.id)) { + const operandMetadata = context.derivationCache.cache.get( + operand.identifier.id, + ); + + if (operandMetadata !== undefined) { + operandMetadata.typeOfValue = joinValue( + typeOfValue, + operandMetadata.typeOfValue, + ); + } + } else { + context.derivationCache.addDerivationEntry( + operand, + sources, + typeOfValue, + ); + } } break; } @@ -367,6 +412,68 @@ function recordInstructionDerivations( } } +function buildDataFlowTree( + sourceId: IdentifierId, + indent: string = '', + isLast: boolean = true, + context: ValidationContext, + propsSet: Set, + stateSet: Set, +): string { + const sourceMetadata = context.derivationCache.cache.get(sourceId); + if (!sourceMetadata || !sourceMetadata.place.identifier.name?.value) { + return ''; + } + + const sourceName = sourceMetadata.place.identifier.name.value; + const prefix = indent + (isLast ? '└── ' : '├── '); + const childIndent = indent + (isLast ? ' ' : '│ '); + + const childSourceIds = Array.from(sourceMetadata.sourcesIds).filter( + id => id !== sourceId, + ); + + const isOriginal = childSourceIds.length === 0; + + let result = `${prefix}${sourceName}`; + + if (isOriginal) { + let typeLabel: string; + if (sourceMetadata.typeOfValue === 'fromProps') { + propsSet.add(sourceMetadata.place.identifier.name?.value); + typeLabel = 'Prop'; + } else if (sourceMetadata.typeOfValue === 'fromState') { + stateSet.add(sourceMetadata.place.identifier.name?.value); + typeLabel = 'State'; + } else { + propsSet.add(sourceMetadata.place.identifier.name?.value); + stateSet.add(sourceMetadata.place.identifier.name?.value); + typeLabel = 'Prop and State'; + } + result += ` (${typeLabel})`; + } + + if (childSourceIds.length > 0) { + result += '\n'; + childSourceIds.forEach((childId, index) => { + const childTree = buildDataFlowTree( + childId, + childIndent, + index === childSourceIds.length - 1, + context, + propsSet, + stateSet, + ); + if (childTree) { + result += childTree + '\n'; + } + }); + result = result.slice(0, -1); + } + + return result; +} + function validateEffect( effectFunction: HIRFunction, context: ValidationContext, @@ -469,27 +576,49 @@ function validateEffect( .length - 1 ) { - const derivedDepsStr = Array.from(derivedSetStateCall.sourceIds) - .map(sourceId => { - const sourceMetadata = context.derivationCache.cache.get(sourceId); - return sourceMetadata?.place.identifier.name?.value; - }) - .filter(Boolean) - .join(', '); - - let description; - - if (derivedSetStateCall.typeOfValue === 'fromProps') { - description = `From props: [${derivedDepsStr}]`; - } else if (derivedSetStateCall.typeOfValue === 'fromState') { - description = `From local state: [${derivedDepsStr}]`; - } else { - description = `From props and local state: [${derivedDepsStr}]`; + const allSourceIds = Array.from(derivedSetStateCall.sourceIds); + const propsSet = new Set(); + const stateSet = new Set(); + + const trees = allSourceIds + .map((id, index) => + buildDataFlowTree( + id, + '', + index === allSourceIds.length - 1, + context, + propsSet, + stateSet, + ), + ) + .filter(Boolean); + + const propsArr = Array.from(propsSet); + const stateArr = Array.from(stateSet); + + let rootSources = ''; + if (propsArr.length > 0) { + rootSources += `Props: [${propsArr.join(', ')}]`; + } + if (stateArr.length > 0) { + if (rootSources) rootSources += '\n'; + rootSources += `State: [${stateArr.join(', ')}]`; } + const description = `Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user + +This setState call is setting a derived value that depends on the following reactive sources: + +${rootSources} + +Data Flow Tree: +${trees.join('\n')} + +See: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state`; + context.errors.pushDiagnostic( CompilerDiagnostic.create({ - description: `Derived values (${description}) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user`, + description: description, category: ErrorCategory.EffectDerivationsOfState, reason: 'You might not need an effect. Derive values in render, not effects.', 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 new file mode 100644 index 0000000000000..756a219e645a1 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-conditionally-in-effect.expect.md @@ -0,0 +1,86 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly +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_exp @loggerTestOnly +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 }], +}; + +``` + +## Logs + +``` +{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nProps: [value]\n\nData Flow Tree:\n└── value (Prop)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":9,"column":6,"index":244},"end":{"line":9,"column":19,"index":257},"filename":"derived-state-conditionally-in-effect.ts","identifierName":"setLocalValue"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null} +{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":0,"index":107},"end":{"line":16,"column":1,"index":378},"filename":"derived-state-conditionally-in-effect.ts"},"fnName":"Component","memoSlots":6,"memoBlocks":2,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0} +``` + +### 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/error.derived-state-conditionally-in-effect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-conditionally-in-effect.js similarity index 86% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-conditionally-in-effect.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-conditionally-in-effect.js index 2ccd52500c773..fb65cbfeb82bd 100644 --- 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/derived-state-conditionally-in-effect.js @@ -1,4 +1,4 @@ -// @validateNoDerivedComputationsInEffects_exp +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import {useEffect, useState} from 'react'; function Component({value, enabled}) { 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 new file mode 100644 index 0000000000000..2f3a3d0e61606 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-default-props.expect.md @@ -0,0 +1,78 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly +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'}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp @loggerTestOnly +import { useEffect, useState } from "react"; + +export default function Component(t0) { + const $ = _c(5); + const { input: t1 } = t0; + const input = t1 === undefined ? "empty" : t1; + const [currInput, setCurrInput] = useState(input); + let t2; + let t3; + if ($[0] !== input) { + t2 = () => { + setCurrInput(input + "local const"); + }; + t3 = [input, "local const"]; + $[0] = input; + $[1] = t2; + $[2] = t3; + } else { + t2 = $[1]; + t3 = $[2]; + } + useEffect(t2, t3); + let t4; + if ($[3] !== currInput) { + t4 =
{currInput}
; + $[3] = currInput; + $[4] = t4; + } else { + t4 = $[4]; + } + return t4; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ input: "test" }], +}; + +``` + +## Logs + +``` +{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nProps: [input]\n\nData Flow Tree:\n└── input (Prop)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":9,"column":4,"index":276},"end":{"line":9,"column":16,"index":288},"filename":"derived-state-from-default-props.ts","identifierName":"setCurrInput"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null} +{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":15,"index":122},"end":{"line":13,"column":1,"index":372},"filename":"derived-state-from-default-props.ts"},"fnName":"Component","memoSlots":5,"memoBlocks":2,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0} +``` + +### Eval output +(kind: ok)
testlocal const
\ 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/derived-state-from-default-props.js similarity index 86% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-default-props.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-default-props.js index 1a0f5126e7a0b..1de911c9b3791 100644 --- 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/derived-state-from-default-props.js @@ -1,4 +1,4 @@ -// @validateNoDerivedComputationsInEffects_exp +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import {useEffect, useState} from 'react'; export default function Component({input = 'empty'}) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-local-state-in-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-local-state-in-effect.expect.md new file mode 100644 index 0000000000000..37458dcea06c5 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-local-state-in-effect.expect.md @@ -0,0 +1,77 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly + +import {useEffect, useState} from 'react'; + +function Component({shouldChange}) { + const [count, setCount] = useState(0); + + useEffect(() => { + if (shouldChange) { + setCount(count + 1); + } + }, [count]); + + return
{count}
; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp @loggerTestOnly + +import { useEffect, useState } from "react"; + +function Component(t0) { + const $ = _c(7); + const { shouldChange } = t0; + const [count, setCount] = useState(0); + let t1; + if ($[0] !== count || $[1] !== shouldChange) { + t1 = () => { + if (shouldChange) { + setCount(count + 1); + } + }; + $[0] = count; + $[1] = shouldChange; + $[2] = t1; + } else { + t1 = $[2]; + } + let t2; + if ($[3] !== count) { + t2 = [count]; + $[3] = count; + $[4] = t2; + } else { + t2 = $[4]; + } + useEffect(t1, t2); + let t3; + if ($[5] !== count) { + t3 =
{count}
; + $[5] = count; + $[6] = t3; + } else { + t3 = $[6]; + } + return t3; +} + +``` + +## Logs + +``` +{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nState: [count]\n\nData Flow Tree:\n└── count (State)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":10,"column":6,"index":237},"end":{"line":10,"column":14,"index":245},"filename":"derived-state-from-local-state-in-effect.ts","identifierName":"setCount"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null} +{"kind":"CompileSuccess","fnLoc":{"start":{"line":5,"column":0,"index":108},"end":{"line":15,"column":1,"index":310},"filename":"derived-state-from-local-state-in-effect.ts"},"fnName":"Component","memoSlots":7,"memoBlocks":3,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0} +``` + +### Eval output +(kind: exception) Fixture not implemented \ 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/derived-state-from-local-state-in-effect.js similarity index 79% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-local-state-in-effect.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-local-state-in-effect.js index 9568e4900296e..79e65e4849a90 100644 --- 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/derived-state-from-local-state-in-effect.js @@ -1,4 +1,4 @@ -// @validateNoDerivedComputationsInEffects_exp +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import {useEffect, useState} from 'react'; 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 new file mode 100644 index 0000000000000..fdcbccd3de5be --- /dev/null +++ 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 @@ -0,0 +1,115 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly +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_exp @loggerTestOnly +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" }], +}; + +``` + +## Logs + +``` +{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nProps: [firstName]\nState: [lastName]\n\nData Flow Tree:\n├── firstName (Prop)\n└── lastName (State)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":11,"column":4,"index":297},"end":{"line":11,"column":15,"index":308},"filename":"derived-state-from-prop-local-state-and-component-scope.ts","identifierName":"setFullName"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null} +{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":0,"index":107},"end":{"line":20,"column":1,"index":542},"filename":"derived-state-from-prop-local-state-and-component-scope.ts"},"fnName":"Component","memoSlots":12,"memoBlocks":5,"memoValues":6,"prunedMemoBlocks":0,"prunedMemoValues":0} +``` + +### 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/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/derived-state-from-prop-local-state-and-component-scope.js similarity index 90% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.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/derived-state-from-prop-local-state-and-component-scope.js index 3090ef00412d8..f25e20863d339 100644 --- 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/derived-state-from-prop-local-state-and-component-scope.js @@ -1,4 +1,4 @@ -// @validateNoDerivedComputationsInEffects_exp +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import {useEffect, useState} from 'react'; function Component({firstName}) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/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/derived-state-from-prop-setter-call-outside-effect-no-error.expect.md index ef817a3ebf24b..6981482545009 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/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/derived-state-from-prop-setter-call-outside-effect-no-error.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// @validateNoDerivedComputationsInEffects_exp +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import {useEffect, useState} from 'react'; function Component({initialName}) { @@ -29,7 +29,7 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import { useEffect, useState } from "react"; function Component(t0) { @@ -79,6 +79,12 @@ export const FIXTURE_ENTRYPOINT = { }; ``` + +## Logs + +``` +{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":0,"index":107},"end":{"line":16,"column":1,"index":359},"filename":"derived-state-from-prop-setter-call-outside-effect-no-error.ts"},"fnName":"Component","memoSlots":6,"memoBlocks":3,"memoValues":4,"prunedMemoBlocks":0,"prunedMemoValues":0} +``` ### Eval output (kind: ok)
\ 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-setter-call-outside-effect-no-error.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-setter-call-outside-effect-no-error.js index 502402be51a3f..6df5f2eed6f92 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/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/derived-state-from-prop-setter-call-outside-effect-no-error.js @@ -1,4 +1,4 @@ -// @validateNoDerivedComputationsInEffects_exp +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import {useEffect, useState} from 'react'; function Component({initialName}) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-setter-ternary.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-setter-ternary.expect.md new file mode 100644 index 0000000000000..48811aa5a9433 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-setter-ternary.expect.md @@ -0,0 +1,57 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp + +function Component({value}) { + const [checked, setChecked] = useState(''); + + useEffect(() => { + setChecked(value === '' ? [] : value.split(',')); + }, [value]); + + return
{checked}
; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp + +function Component(t0) { + const $ = _c(5); + const { value } = t0; + const [checked, setChecked] = useState(""); + let t1; + let t2; + if ($[0] !== value) { + t1 = () => { + setChecked(value === "" ? [] : value.split(",")); + }; + t2 = [value]; + $[0] = value; + $[1] = t1; + $[2] = t2; + } else { + t1 = $[1]; + t2 = $[2]; + } + useEffect(t1, t2); + let t3; + if ($[3] !== checked) { + t3 =
{checked}
; + $[3] = checked; + $[4] = t3; + } else { + t3 = $[4]; + } + return t3; +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ 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-setter-ternary.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-setter-ternary.js new file mode 100644 index 0000000000000..afd198caa262e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-setter-ternary.js @@ -0,0 +1,11 @@ +// @validateNoDerivedComputationsInEffects_exp + +function Component({value}) { + const [checked, setChecked] = useState(''); + + useEffect(() => { + setChecked(value === '' ? [] : value.split(',')); + }, [value]); + + return
{checked}
; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/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/derived-state-from-prop-setter-used-outside-effect-no-error.expect.md index 2924de0da6132..b5100dc2a6334 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/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/derived-state-from-prop-setter-used-outside-effect-no-error.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// @validateNoDerivedComputationsInEffects_exp +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import {useEffect, useState} from 'react'; function MockComponent({onSet}) { @@ -28,7 +28,7 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import { useEffect, useState } from "react"; function MockComponent(t0) { @@ -80,6 +80,13 @@ export const FIXTURE_ENTRYPOINT = { }; ``` + +## Logs + +``` +{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":0,"index":107},"end":{"line":6,"column":1,"index":211},"filename":"derived-state-from-prop-setter-used-outside-effect-no-error.ts"},"fnName":"MockComponent","memoSlots":2,"memoBlocks":1,"memoValues":1,"prunedMemoBlocks":0,"prunedMemoValues":0} +{"kind":"CompileSuccess","fnLoc":{"start":{"line":8,"column":0,"index":213},"end":{"line":15,"column":1,"index":402},"filename":"derived-state-from-prop-setter-used-outside-effect-no-error.ts"},"fnName":"Component","memoSlots":4,"memoBlocks":2,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0} +``` ### Eval output (kind: ok)
Mock Component
\ 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-setter-used-outside-effect-no-error.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-setter-used-outside-effect-no-error.js index d33af16ec5901..43b5a8c52ada8 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/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/derived-state-from-prop-setter-used-outside-effect-no-error.js @@ -1,4 +1,4 @@ -// @validateNoDerivedComputationsInEffects_exp +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import {useEffect, useState} from 'react'; function MockComponent({onSet}) { 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 new file mode 100644 index 0000000000000..0160fbbb4a712 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-with-side-effect.expect.md @@ -0,0 +1,78 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly +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'}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp @loggerTestOnly +import { useEffect, useState } from "react"; + +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" }], +}; + +``` + +## Logs + +``` +{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nProps: [value]\n\nData Flow Tree:\n└── value (Prop)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":8,"column":4,"index":214},"end":{"line":8,"column":17,"index":227},"filename":"derived-state-from-prop-with-side-effect.ts","identifierName":"setLocalValue"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null} +{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":0,"index":107},"end":{"line":13,"column":1,"index":327},"filename":"derived-state-from-prop-with-side-effect.ts"},"fnName":"Component","memoSlots":5,"memoBlocks":2,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0} +``` + +### 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/error.derived-state-from-prop-with-side-effect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-with-side-effect.js similarity index 84% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-with-side-effect.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-with-side-effect.js index 88c66ce1ef650..5bb963daac320 100644 --- 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/derived-state-from-prop-with-side-effect.js @@ -1,4 +1,4 @@ -// @validateNoDerivedComputationsInEffects_exp +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import {useEffect, useState} from 'react'; function Component({value}) { 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 index 4d0b6663e325a..e88d5833a9a1b 100644 --- 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 @@ -2,7 +2,7 @@ ## Input ```javascript -// @validateNoDerivedComputationsInEffects_exp +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import {useEffect, useState, useRef} from 'react'; export default function Component({test}) { @@ -27,7 +27,7 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import { useEffect, useState, useRef } from "react"; export default function Component(t0) { @@ -68,6 +68,12 @@ export const FIXTURE_ENTRYPOINT = { }; ``` + +## Logs + +``` +{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":15,"index":130},"end":{"line":14,"column":1,"index":328},"filename":"derived-state-from-ref-and-state-no-error.ts"},"fnName":"Component","memoSlots":5,"memoBlocks":2,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0} +``` ### 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 index 6b24f73ac7400..18ad2bdca19fd 100644 --- 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 @@ -1,4 +1,4 @@ -// @validateNoDerivedComputationsInEffects_exp +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import {useEffect, useState, useRef} from 'react'; export default function Component({test}) { 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 new file mode 100644 index 0000000000000..fdc7081f3758a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-contains-local-function-call.expect.md @@ -0,0 +1,93 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly +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_exp @loggerTestOnly +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" }], +}; + +``` + +## Logs + +``` +{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nProps: [propValue]\n\nData Flow Tree:\n└── propValue (Prop)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":12,"column":4,"index":279},"end":{"line":12,"column":12,"index":287},"filename":"effect-contains-local-function-call.ts","identifierName":"setValue"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null} +{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":0,"index":107},"end":{"line":17,"column":1,"index":371},"filename":"effect-contains-local-function-call.ts"},"fnName":"Component","memoSlots":6,"memoBlocks":3,"memoValues":4,"prunedMemoBlocks":0,"prunedMemoValues":0} +``` + +### 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.effect-contains-local-function-call.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-contains-local-function-call.js similarity index 86% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.effect-contains-local-function-call.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-contains-local-function-call.js index 1efb3177e51e5..a6442b36477f9 100644 --- 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/effect-contains-local-function-call.js @@ -1,4 +1,4 @@ -// @validateNoDerivedComputationsInEffects_exp +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import {useEffect, useState} from 'react'; function Component({propValue}) { 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 index c83ea552a6a78..74391e86ad375 100644 --- 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 @@ -2,7 +2,7 @@ ## Input ```javascript -// @validateNoDerivedComputationsInEffects_exp +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import {useEffect, useState} from 'react'; function Component({propValue, onChange}) { @@ -25,7 +25,7 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import { useEffect, useState } from "react"; function Component(t0) { @@ -70,6 +70,13 @@ export const FIXTURE_ENTRYPOINT = { }; ``` + +## Logs + +``` +{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":0,"index":107},"end":{"line":12,"column":1,"index":306},"filename":"effect-contains-prop-function-call-no-error.ts"},"fnName":"Component","memoSlots":7,"memoBlocks":3,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0} +{"kind":"CompileSuccess","fnLoc":{"start":{"line":16,"column":41,"index":402},"end":{"line":16,"column":49,"index":410},"filename":"effect-contains-prop-function-call-no-error.ts"},"fnName":null,"memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0} +``` ### 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 index 512df7cb36ebd..f19e9518d6fa4 100644 --- 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 @@ -1,4 +1,4 @@ -// @validateNoDerivedComputationsInEffects_exp +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import {useEffect, useState} from 'react'; function Component({propValue, 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 index e17f1e26f6c20..e26643723d980 100644 --- 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 @@ -2,7 +2,7 @@ ## Input ```javascript -// @validateNoDerivedComputationsInEffects_exp +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import {useEffect, useState} from 'react'; function Component({propValue}) { @@ -25,7 +25,7 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import { useEffect, useState } from "react"; function Component(t0) { @@ -65,6 +65,12 @@ export const FIXTURE_ENTRYPOINT = { }; ``` + +## Logs + +``` +{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":0,"index":107},"end":{"line":12,"column":1,"index":298},"filename":"effect-with-global-function-call-no-error.ts"},"fnName":"Component","memoSlots":5,"memoBlocks":2,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0} +``` ### 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 index 4cded6dcc8ef3..ae7622d4d0642 100644 --- 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 @@ -1,4 +1,4 @@ -// @validateNoDerivedComputationsInEffects_exp +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import {useEffect, useState} from 'react'; function Component({propValue}) { 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 deleted file mode 100644 index 1fa7f7d7950e6..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-conditionally-in-effect.expect.md +++ /dev/null @@ -1,49 +0,0 @@ - -## 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: You might not need an effect. Derive values in render, not effects. - -Derived values (From props: [value]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user. - -error.derived-state-conditionally-in-effect.ts:9:6 - 7 | useEffect(() => { - 8 | if (enabled) { -> 9 | setLocalValue(value); - | ^^^^^^^^^^^^^ This should be computed during render, not in an effect - 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-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 deleted file mode 100644 index f30235a064a94..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-default-props.expect.md +++ /dev/null @@ -1,46 +0,0 @@ - -## 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: You might not need an effect. Derive values in render, not effects. - -Derived values (From props: [input]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user. - -error.derived-state-from-default-props.ts:9:4 - 7 | - 8 | useEffect(() => { -> 9 | setCurrInput(input + localConst); - | ^^^^^^^^^^^^ This should be computed during render, not in an effect - 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-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 deleted file mode 100644 index 779ddafc40184..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-local-state-in-effect.expect.md +++ /dev/null @@ -1,43 +0,0 @@ - -## 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: You might not need an effect. Derive values in render, not effects. - -Derived values (From local state: [count]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user. - -error.derived-state-from-local-state-in-effect.ts:10:6 - 8 | useEffect(() => { - 9 | if (shouldChange) { -> 10 | setCount(count + 1); - | ^^^^^^^^ This should be computed during render, not in an effect - 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-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 deleted file mode 100644 index 7b27b556b3e98..0000000000000 --- 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 +++ /dev/null @@ -1,53 +0,0 @@ - -## 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: You might not need an effect. Derive values in render, not effects. - -Derived values (From props and local state: [firstName, lastName]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user. - -error.derived-state-from-prop-local-state-and-component-scope.ts:11:4 - 9 | - 10 | useEffect(() => { -> 11 | setFullName(firstName + ' ' + middleName + ' ' + lastName); - | ^^^^^^^^^^^ This should be computed during render, not in an effect - 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-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 deleted file mode 100644 index 7fadae5667fdb..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-with-side-effect.expect.md +++ /dev/null @@ -1,46 +0,0 @@ - -## 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: You might not need an effect. Derive values in render, not effects. - -Derived values (From props: [value]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user. - -error.derived-state-from-prop-with-side-effect.ts:8:4 - 6 | - 7 | useEffect(() => { -> 8 | setLocalValue(value); - | ^^^^^^^^^^^^^ This should be computed during render, not in an effect - 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.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 deleted file mode 100644 index aec543fcbf48c..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.effect-contains-local-function-call.expect.md +++ /dev/null @@ -1,50 +0,0 @@ - -## 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: You might not need an effect. Derive values in render, not effects. - -Derived values (From props: [propValue]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user. - -error.effect-contains-local-function-call.ts:12:4 - 10 | - 11 | useEffect(() => { -> 12 | setValue(propValue); - | ^^^^^^^^ This should be computed during render, not in an effect - 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.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 deleted file mode 100644 index f1f755adfa803..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-computation-in-effect.expect.md +++ /dev/null @@ -1,48 +0,0 @@ - -## 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: You might not need an effect. Derive values in render, not effects. - -Derived values (From local state: [firstName]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user. - -error.invalid-derived-computation-in-effect.ts:11:4 - 9 | const [fullName, setFullName] = useState(''); - 10 | useEffect(() => { -> 11 | setFullName(firstName + ' ' + lastName); - | ^^^^^^^^^^^ This should be computed during render, not in an effect - 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-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 deleted file mode 100644 index 3a0788969397a..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-state-from-computed-props.expect.md +++ /dev/null @@ -1,46 +0,0 @@ - -## 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: You might not need an effect. Derive values in render, not effects. - -Derived values (From props: [props]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user. - -error.invalid-derived-state-from-computed-props.ts:9:4 - 7 | useEffect(() => { - 8 | const computed = props.prefix + props.value + props.suffix; -> 9 | setDisplayValue(computed); - | ^^^^^^^^^^^^^^^ This should be computed during render, not in an effect - 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-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 deleted file mode 100644 index b28692c67b307..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-state-from-destructured-props.expect.md +++ /dev/null @@ -1,47 +0,0 @@ - -## 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: You might not need an effect. Derive values in render, not effects. - -Derived values (From props: [props]) should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user. - -error.invalid-derived-state-from-destructured-props.ts:10:4 - 8 | - 9 | useEffect(() => { -> 10 | setFullName(props.firstName + ' ' + props.lastName); - | ^^^^^^^^^^^ This should be computed during render, not in an effect - 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-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 new file mode 100644 index 0000000000000..29dea440b463d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-computation-in-effect.expect.md @@ -0,0 +1,80 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly +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: [], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp @loggerTestOnly +import { useEffect, useState } from "react"; + +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: [], +}; + +``` + +## Logs + +``` +{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nState: [firstName]\n\nData Flow Tree:\n└── firstName (State)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":11,"column":4,"index":341},"end":{"line":11,"column":15,"index":352},"filename":"invalid-derived-computation-in-effect.ts","identifierName":"setFullName"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null} +{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":0,"index":107},"end":{"line":15,"column":1,"index":445},"filename":"invalid-derived-computation-in-effect.ts"},"fnName":"Component","memoSlots":5,"memoBlocks":2,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0} +``` + +### Eval output +(kind: ok)
Taylor Swift
\ 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/invalid-derived-computation-in-effect.js similarity index 87% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-computation-in-effect.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-computation-in-effect.js index 17779a5b4c576..e29ece67bd57f 100644 --- 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/invalid-derived-computation-in-effect.js @@ -1,4 +1,4 @@ -// @validateNoDerivedComputationsInEffects_exp +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import {useEffect, useState} from 'react'; function Component() { 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 new file mode 100644 index 0000000000000..c7199d95480fe --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-state-from-computed-props.expect.md @@ -0,0 +1,79 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly +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_exp @loggerTestOnly +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: "]" }], +}; + +``` + +## Logs + +``` +{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nProps: [props]\n\nData Flow Tree:\n└── computed\n └── props (Prop)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":9,"column":4,"index":295},"end":{"line":9,"column":19,"index":310},"filename":"invalid-derived-state-from-computed-props.ts","identifierName":"setDisplayValue"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null} +{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":15,"index":122},"end":{"line":13,"column":1,"index":409},"filename":"invalid-derived-state-from-computed-props.ts"},"fnName":"Component","memoSlots":7,"memoBlocks":2,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0} +``` + +### 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/error.invalid-derived-state-from-computed-props.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-state-from-computed-props.js similarity index 87% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-state-from-computed-props.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-state-from-computed-props.js index 24afa944fc566..39648ef8d5eed 100644 --- 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/invalid-derived-state-from-computed-props.js @@ -1,4 +1,4 @@ -// @validateNoDerivedComputationsInEffects_exp +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import {useEffect, useState} from 'react'; export default function Component(props) { 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 new file mode 100644 index 0000000000000..5a6af555408e1 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-state-from-destructured-props.expect.md @@ -0,0 +1,81 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly +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_exp @loggerTestOnly +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" } }], +}; + +``` + +## Logs + +``` +{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nProps: [props]\n\nData Flow Tree:\n└── props (Prop)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":10,"column":4,"index":269},"end":{"line":10,"column":15,"index":280},"filename":"invalid-derived-state-from-destructured-props.ts","identifierName":"setFullName"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null} +{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":15,"index":122},"end":{"line":14,"column":1,"index":397},"filename":"invalid-derived-state-from-destructured-props.ts"},"fnName":"Component","memoSlots":6,"memoBlocks":2,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0} +``` + +### 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/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/invalid-derived-state-from-destructured-props.js similarity index 87% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-state-from-destructured-props.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-state-from-destructured-props.js index bdfb47a2c6aad..3f662f13f71cc 100644 --- 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/invalid-derived-state-from-destructured-props.js @@ -1,4 +1,4 @@ -// @validateNoDerivedComputationsInEffects_exp +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import {useEffect, useState} from 'react'; export default function Component({props}) { 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 index 365ee1fef4548..9a843d1883c31 100644 --- 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 @@ -2,7 +2,7 @@ ## Input ```javascript -// @validateNoDerivedComputationsInEffects_exp +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import {useEffect, useState, useRef} from 'react'; export default function Component({test}) { @@ -31,7 +31,7 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import { useEffect, useState, useRef } from "react"; export default function Component(t0) { @@ -77,6 +77,12 @@ export const FIXTURE_ENTRYPOINT = { }; ``` + +## Logs + +``` +{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":15,"index":130},"end":{"line":18,"column":1,"index":386},"filename":"ref-conditional-in-effect-no-error.ts"},"fnName":"Component","memoSlots":5,"memoBlocks":2,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0} +``` ### 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 index ee59ccb78f037..3594deaa02e98 100644 --- 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 @@ -1,4 +1,4 @@ -// @validateNoDerivedComputationsInEffects_exp +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly import {useEffect, useState, useRef} from 'react'; export default function Component({test}) {