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 a83b22651e74..0c777f87707d 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 a755d0e2c65f..5c9462fd5510 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 { @@ -20,7 +21,6 @@ import { isUseStateType, BasicBlock, isUseRefType, - GeneratedSource, SourceLocation, } from '../HIR'; import {eachInstructionLValue, eachInstructionOperand} from '../HIR/visitors'; @@ -33,6 +33,7 @@ type DerivationMetadata = { typeOfValue: TypeOfValue; place: Place; sourcesIds: Set; + isStateSource: boolean; }; type ValidationContext = { @@ -40,13 +41,51 @@ type ValidationContext = { readonly errors: CompilerError; readonly derivationCache: DerivationCache; readonly effects: Set; - readonly setStateCache: Map>; - readonly effectSetStateCache: Map>; + readonly setStateLoads: Map; + readonly setStateUsages: Map>; }; 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, + isStateSource: value.isStateSource, + }); + } + } + + 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; @@ -58,48 +97,28 @@ class DerivationCache { derivedVar: Place, sourcesIds: Set, typeOfValue: TypeOfValue, + isStateSource: boolean, ): 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. - */ + let finalIsSource = isStateSource; + if (!finalIsSource) { + for (const sourceId of sourcesIds) { + const sourceMetadata = this.cache.get(sourceId); if ( - sourcePlace.identifier.name === null || - sourcePlace.identifier.name?.kind === 'promoted' + sourceMetadata?.isStateSource && + sourceMetadata.place.identifier.name?.kind !== 'named' ) { - newValue.sourcesIds.add(derivedVar.identifier.id); - } else { - newValue.sourcesIds.add(sourcePlace.identifier.id); + finalIsSource = true; + break; } } } - 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; - } + this.cache.set(derivedVar.identifier.id, { + place: derivedVar, + sourcesIds: sourcesIds, + typeOfValue: typeOfValue ?? 'ignored', + isStateSource: finalIsSource, + }); } private isDerivationEqual( @@ -121,6 +140,14 @@ class DerivationCache { } } +function isNamedIdentifier(place: Place): place is Place & { + identifier: {name: NonNullable}; +} { + 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,25 +173,22 @@ class DerivationCache { */ export function validateNoDerivedComputationsInEffects_exp( fn: HIRFunction, -): void { +): Result { const functions: Map = new Map(); const derivationCache = new DerivationCache(); const errors = new CompilerError(); const effects: Set = new Set(); - const setStateCache: Map> = new Map(); - const effectSetStateCache: Map< - string | undefined | null, - Array - > = new Map(); + const setStateLoads: Map = new Map(); + const setStateUsages: Map> = new Map(); const context: ValidationContext = { functions, errors, derivationCache, effects, - setStateCache, - effectSetStateCache, + setStateLoads, + setStateUsages, }; if (fn.fnType === 'Hook') { @@ -172,10 +196,10 @@ 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(), typeOfValue: 'fromProps', + isStateSource: true, }); - context.derivationCache.hasChanges = true; } } } else if (fn.fnType === 'Component') { @@ -183,15 +207,17 @@ 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(), typeOfValue: 'fromProps', + isStateSource: true, }); - 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 +225,7 @@ export function validateNoDerivedComputationsInEffects_exp( } } + context.derivationCache.checkForChanges(); isFirstPass = false; } while (context.derivationCache.snapshot()); @@ -206,9 +233,7 @@ export function validateNoDerivedComputationsInEffects_exp( validateEffect(effect, context); } - if (errors.hasAnyErrors()) { - throw errors; - } + return errors.asResult(); } function recordPhiDerivations( @@ -236,6 +261,7 @@ function recordPhiDerivations( phi.place, sourcesIds, typeOfValue, + false, ); } } @@ -251,17 +277,69 @@ function joinValue( return 'fromPropsAndState'; } +function getRootSetState( + key: IdentifierId, + loads: Map, + visited: Set = new Set(), +): IdentifierId | null { + if (visited.has(key)) { + return null; + } + visited.add(key); + + const parentId = loads.get(key); + + if (parentId === undefined) { + return null; + } + + if (parentId === null) { + return key; + } + + return getRootSetState(parentId, loads, visited); +} + +function maybeRecordSetState( + instr: Instruction, + loads: Map, + usages: Map>, +): void { + for (const operand of eachInstructionLValue(instr)) { + if ( + instr.value.kind === 'LoadLocal' && + loads.has(instr.value.place.identifier.id) + ) { + loads.set(operand.identifier.id, instr.value.place.identifier.id); + } else { + if (isSetStateType(operand.identifier)) { + // this is a root setState + loads.set(operand.identifier.id, null); + } + } + + const rootSetState = getRootSetState(operand.identifier.id, loads); + if (rootSetState !== null && usages.get(rootSetState) === undefined) { + usages.set(rootSetState, new Set([operand.loc])); + } + } +} + function recordInstructionDerivations( instr: Instruction, context: ValidationContext, isFirstPass: boolean, ): void { + maybeRecordSetState(instr, context.setStateLoads, context.setStateUsages); + let typeOfValue: TypeOfValue = 'ignored'; + let isSource: boolean = false; 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) { + recordPhiDerivations(block, context); for (const instr of block.instructions) { recordInstructionDerivations(instr, context, isFirstPass); } @@ -280,24 +358,25 @@ function recordInstructionDerivations( 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'); + typeOfValue = 'fromState'; + context.derivationCache.addDerivationEntry( + lvalue, + new Set(), + typeOfValue, + true, + ); + return; } } for (const operand of eachInstructionOperand(instr)) { - if ( - isSetStateType(operand.identifier) && - operand.loc !== GeneratedSource && - isFirstPass - ) { - if (context.setStateCache.has(operand.loc.identifierName)) { - context.setStateCache.get(operand.loc.identifierName)!.push(operand); - } else { - context.setStateCache.set(operand.loc.identifierName, [operand]); + if (context.setStateLoads.has(operand.identifier.id)) { + const rootSetStateId = getRootSetState( + operand.identifier.id, + context.setStateLoads, + ); + if (rootSetStateId !== null) { + context.setStateUsages.get(rootSetStateId)?.add(operand.loc); } } @@ -310,9 +389,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') { @@ -320,7 +397,12 @@ function recordInstructionDerivations( } for (const lvalue of eachInstructionLValue(instr)) { - context.derivationCache.addDerivationEntry(lvalue, sources, typeOfValue); + context.derivationCache.addDerivationEntry( + lvalue, + sources, + typeOfValue, + isSource, + ); } for (const operand of eachInstructionOperand(instr)) { @@ -331,11 +413,25 @@ 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, + false, + ); + } } break; } @@ -367,6 +463,137 @@ function recordInstructionDerivations( } } +type TreeNode = { + name: string; + typeOfValue: TypeOfValue; + isSource: boolean; + children: Array; +}; + +function buildTreeNode( + sourceId: IdentifierId, + context: ValidationContext, + visited: Set = new Set(), +): Array { + const sourceMetadata = context.derivationCache.cache.get(sourceId); + if (!sourceMetadata) { + return []; + } + + if (sourceMetadata.isStateSource && isNamedIdentifier(sourceMetadata.place)) { + return [ + { + name: sourceMetadata.place.identifier.name.value, + typeOfValue: sourceMetadata.typeOfValue, + isSource: sourceMetadata.isStateSource, + children: [], + }, + ]; + } + + const children: Array = []; + + const namedSiblings: Set = new Set(); + for (const childId of sourceMetadata.sourcesIds) { + const childNodes = buildTreeNode( + childId, + context, + new Set([ + ...visited, + ...(isNamedIdentifier(sourceMetadata.place) + ? [sourceMetadata.place.identifier.name.value] + : []), + ]), + ); + if (childNodes) { + for (const childNode of childNodes) { + if (!namedSiblings.has(childNode.name)) { + children.push(childNode); + namedSiblings.add(childNode.name); + } + } + } + } + + if ( + isNamedIdentifier(sourceMetadata.place) && + !visited.has(sourceMetadata.place.identifier.name.value) + ) { + return [ + { + name: sourceMetadata.place.identifier.name.value, + typeOfValue: sourceMetadata.typeOfValue, + isSource: sourceMetadata.isStateSource, + children: children, + }, + ]; + } + + return children; +} + +function renderTree( + node: TreeNode, + indent: string = '', + isLast: boolean = true, + propsSet: Set, + stateSet: Set, +): string { + const prefix = indent + (isLast ? '└── ' : '├── '); + const childIndent = indent + (isLast ? ' ' : '│ '); + + let result = `${prefix}${node.name}`; + + if (node.isSource) { + let typeLabel: string; + if (node.typeOfValue === 'fromProps') { + propsSet.add(node.name); + typeLabel = 'Prop'; + } else if (node.typeOfValue === 'fromState') { + stateSet.add(node.name); + typeLabel = 'State'; + } else { + propsSet.add(node.name); + stateSet.add(node.name); + typeLabel = 'Prop and State'; + } + result += ` (${typeLabel})`; + } + + if (node.children.length > 0) { + result += '\n'; + node.children.forEach((child, index) => { + const isLastChild = index === node.children.length - 1; + result += renderTree(child, childIndent, isLastChild, propsSet, stateSet); + if (index < node.children.length - 1) { + result += '\n'; + } + }); + } + + return result; +} + +function getFnLocalDeps( + fn: FunctionExpression | undefined, +): Set | undefined { + if (!fn) { + return undefined; + } + + const deps: Set = new Set(); + + for (const [, block] of fn.loweredFunc.func.body.blocks) { + for (const instr of block.instructions) { + if (instr.value.kind === 'LoadLocal') { + deps.add(instr.value.place.identifier.id); + } + } + } + + return deps; +} + function validateEffect( effectFunction: HIRFunction, context: ValidationContext, @@ -375,13 +602,33 @@ function validateEffect( const effectDerivedSetStateCalls: Array<{ value: CallExpression; - loc: SourceLocation; + id: IdentifierId; sourceIds: Set; typeOfValue: TypeOfValue; }> = []; + const effectSetStateUsages: Map< + IdentifierId, + Set + > = new Map(); + + let cleanUpFunctionDeps: Set | undefined; + const globals: Set = new Set(); for (const block of effectFunction.body.blocks.values()) { + /* + * if the block is in an effect and is of type return then its an effect's cleanup function + * if the cleanup function depends on a value from which effect-set state is derived then + * we can't validate + */ + if ( + block.terminal.kind === 'return' && + block.terminal.returnVariant === 'Explicit' + ) { + cleanUpFunctionDeps = getFnLocalDeps( + context.functions.get(block.terminal.value.identifier.id), + ); + } for (const pred of block.preds) { if (!seenBlocks.has(pred)) { // skip if block has a back edge @@ -395,19 +642,16 @@ function validateEffect( return; } + maybeRecordSetState(instr, context.setStateLoads, effectSetStateUsages); + for (const operand of eachInstructionOperand(instr)) { - if ( - isSetStateType(operand.identifier) && - operand.loc !== GeneratedSource - ) { - if (context.effectSetStateCache.has(operand.loc.identifierName)) { - context.effectSetStateCache - .get(operand.loc.identifierName)! - .push(operand); - } else { - context.effectSetStateCache.set(operand.loc.identifierName, [ - operand, - ]); + if (context.setStateLoads.has(operand.identifier.id)) { + const rootSetStateId = getRootSetState( + operand.identifier.id, + context.setStateLoads, + ); + if (rootSetStateId !== null) { + effectSetStateUsages.get(rootSetStateId)?.add(operand.loc); } } } @@ -425,7 +669,7 @@ function validateEffect( if (argMetadata !== undefined) { effectDerivedSetStateCalls.push({ value: instr.value, - loc: instr.value.callee.loc, + id: instr.value.callee.identifier.id, sourceIds: argMetadata.sourcesIds, typeOfValue: argMetadata.typeOfValue, }); @@ -459,37 +703,74 @@ function validateEffect( } for (const derivedSetStateCall of effectDerivedSetStateCalls) { + const rootSetStateCall = getRootSetState( + derivedSetStateCall.id, + context.setStateLoads, + ); + if ( - derivedSetStateCall.loc !== GeneratedSource && - context.effectSetStateCache.has(derivedSetStateCall.loc.identifierName) && - context.setStateCache.has(derivedSetStateCall.loc.identifierName) && - context.effectSetStateCache.get(derivedSetStateCall.loc.identifierName)! - .length === - context.setStateCache.get(derivedSetStateCall.loc.identifierName)! - .length - - 1 + rootSetStateCall !== null && + effectSetStateUsages.has(rootSetStateCall) && + context.setStateUsages.has(rootSetStateCall) && + effectSetStateUsages.get(rootSetStateCall)!.size === + context.setStateUsages.get(rootSetStateCall)!.size - 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 propsSet = new Set(); + const stateSet = new Set(); + + const rootNodesMap = new Map(); + for (const id of derivedSetStateCall.sourceIds) { + const nodes = buildTreeNode(id, context); + for (const node of nodes) { + if (!rootNodesMap.has(node.name)) { + rootNodesMap.set(node.name, node); + } + } } + const rootNodes = Array.from(rootNodesMap.values()); + + const trees = rootNodes.map((node, index) => + renderTree( + node, + '', + index === rootNodes.length - 1, + propsSet, + stateSet, + ), + ); + + for (const dep of derivedSetStateCall.sourceIds) { + if (cleanUpFunctionDeps !== undefined && cleanUpFunctionDeps.has(dep)) { + return; + } + } + + 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 000000000000..756a219e645a --- /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 2ccd52500c77..fb65cbfeb82b 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 000000000000..2f3a3d0e6160 --- /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 1a0f5126e7a0..1de911c9b379 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 000000000000..37458dcea06c --- /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 9568e4900296..79e65e4849a9 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 000000000000..fdcbccd3de5b --- /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 3090ef00412d..f25e20863d33 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 ef817a3ebf24..698148254500 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 502402be51a3..6df5f2eed6f9 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 000000000000..48811aa5a943 --- /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 000000000000..afd198caa262 --- /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 2924de0da613..b5100dc2a633 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 d33af16ec590..43b5a8c52ada 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 000000000000..0160fbbb4a71 --- /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 88c66ce1ef65..5bb963daac32 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 4d0b6663e325..e88d5833a9a1 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 6b24f73ac740..18ad2bdca19f 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 000000000000..fdc7081f3758 --- /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 1efb3177e51e..a6442b36477f 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 c83ea552a6a7..74391e86ad37 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 512df7cb36eb..f19e9518d6fa 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-cleanup-function-depending-on-derived-computation-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-with-cleanup-function-depending-on-derived-computation-value.expect.md new file mode 100644 index 000000000000..e84031591e07 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-with-cleanup-function-depending-on-derived-computation-value.expect.md @@ -0,0 +1,76 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly + +import {useEffect, useState} from 'react'; + +function Component(file: File) { + const [imageUrl, setImageUrl] = useState(null); + + /* + * Cleaning up the variable or a source of the variable used to setState + * inside the effect communicates that we always need to clean up something + * which is a valid use case for useEffect. In which case we want to + * avoid an throwing + */ + useEffect(() => { + const imageUrlPrepared = URL.createObjectURL(file); + setImageUrl(imageUrlPrepared); + return () => URL.revokeObjectURL(imageUrlPrepared); + }, [file]); + + return ; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp @loggerTestOnly + +import { useEffect, useState } from "react"; + +function Component(file) { + const $ = _c(5); + const [imageUrl, setImageUrl] = useState(null); + let t0; + let t1; + if ($[0] !== file) { + t0 = () => { + const imageUrlPrepared = URL.createObjectURL(file); + setImageUrl(imageUrlPrepared); + return () => URL.revokeObjectURL(imageUrlPrepared); + }; + t1 = [file]; + $[0] = file; + $[1] = t0; + $[2] = t1; + } else { + t0 = $[1]; + t1 = $[2]; + } + useEffect(t0, t1); + let t2; + if ($[3] !== imageUrl) { + t2 = ; + $[3] = imageUrl; + $[4] = t2; + } else { + t2 = $[4]; + } + return t2; +} + +``` + +## Logs + +``` +{"kind":"CompileSuccess","fnLoc":{"start":{"line":5,"column":0,"index":108},"end":{"line":21,"column":1,"index":700},"filename":"effect-with-cleanup-function-depending-on-derived-computation-value.ts"},"fnName":"Component","memoSlots":5,"memoBlocks":2,"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/effect-with-cleanup-function-depending-on-derived-computation-value.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-with-cleanup-function-depending-on-derived-computation-value.js new file mode 100644 index 000000000000..e419583cc6dd --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-with-cleanup-function-depending-on-derived-computation-value.js @@ -0,0 +1,21 @@ +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly + +import {useEffect, useState} from 'react'; + +function Component(file: File) { + const [imageUrl, setImageUrl] = useState(null); + + /* + * Cleaning up the variable or a source of the variable used to setState + * inside the effect communicates that we always need to clean up something + * which is a valid use case for useEffect. In which case we want to + * avoid an throwing + */ + useEffect(() => { + const imageUrlPrepared = URL.createObjectURL(file); + setImageUrl(imageUrlPrepared); + return () => URL.revokeObjectURL(imageUrlPrepared); + }, [file]); + + return ; +} 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 e17f1e26f6c2..e26643723d98 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 4cded6dcc8ef..ae7622d4d064 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 1fa7f7d7950e..000000000000 --- 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 f30235a064a9..000000000000 --- 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 779ddafc4018..000000000000 --- 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 7b27b556b3e9..000000000000 --- 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 7fadae5667fd..000000000000 --- 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 aec543fcbf48..000000000000 --- 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 f1f755adfa80..000000000000 --- 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 3a0788969397..000000000000 --- 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 b28692c67b30..000000000000 --- 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 000000000000..29dea440b463 --- /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 17779a5b4c57..e29ece67bd57 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 000000000000..c7199d95480f --- /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 24afa944fc56..39648ef8d5ee 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 000000000000..5a6af555408e --- /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 bdfb47a2c6aa..3f662f13f71c 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 365ee1fef454..9a843d1883c3 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 ee59ccb78f03..3594deaa02e9 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}) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/usestate-derived-from-prop-no-show-in-data-flow-tree.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/usestate-derived-from-prop-no-show-in-data-flow-tree.expect.md new file mode 100644 index 000000000000..7ab14466b256 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/usestate-derived-from-prop-no-show-in-data-flow-tree.expect.md @@ -0,0 +1,59 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp + +function Component({ prop }) { + const [s, setS] = useState(prop) + const [second, setSecond] = useState(prop) + + useEffect(() => { + setS(second) + }, [second]) + + return
{s}
+} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp + +function Component(t0) { + const $ = _c(5); + const { prop } = t0; + const [s, setS] = useState(prop); + const [second] = useState(prop); + let t1; + let t2; + if ($[0] !== second) { + t1 = () => { + setS(second); + }; + t2 = [second]; + $[0] = second; + $[1] = t1; + $[2] = t2; + } else { + t1 = $[1]; + t2 = $[2]; + } + useEffect(t1, t2); + let t3; + if ($[3] !== s) { + t3 =
{s}
; + $[3] = s; + $[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/usestate-derived-from-prop-no-show-in-data-flow-tree.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/usestate-derived-from-prop-no-show-in-data-flow-tree.js new file mode 100644 index 000000000000..5c62fa2e8f1f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/usestate-derived-from-prop-no-show-in-data-flow-tree.js @@ -0,0 +1,18 @@ +// @validateNoDerivedComputationsInEffects_exp + +function Component({prop}) { + const [s, setS] = useState(); + const [second, setSecond] = useState(prop); + + /* + * `second` is a source of state. It will inherit the value of `prop` in + * the first render, but after that it will no longer be updated when + * `prop` changes. So we shouldn't consider `second` as being derived from + * `prop` + */ + useEffect(() => { + setS(second); + }, [second]); + + return
{s}
; +} diff --git a/packages/react-devtools-shared/src/__tests__/store-test.js b/packages/react-devtools-shared/src/__tests__/store-test.js index 9b2b2ae54ffd..37272ea0579e 100644 --- a/packages/react-devtools-shared/src/__tests__/store-test.js +++ b/packages/react-devtools-shared/src/__tests__/store-test.js @@ -3298,4 +3298,100 @@ describe('Store', () => { `); }); + + // @reactVersion >= 19.0 + it('measures rects when reconnecting', async () => { + function Component({children, promise}) { + let content = ''; + if (promise) { + const value = readValue(promise); + if (typeof value === 'string') { + content += value; + } + } + return ( +
+ {content} + {children} +
+ ); + } + + function App({outer, inner}) { + return ( + loading outer}> + + outer content + + loading inner + }> + + inner content + + + + ); + } + + await actAsync(() => { + render(); + }); + + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + ▾ + + ▾ + + [suspense-root] rects={[{x:1,y:2,width:13,height:1}, {x:1,y:2,width:13,height:1}]} + + + `); + + let outerResolve; + const outerPromise = new Promise(resolve => { + outerResolve = resolve; + }); + + let innerResolve; + const innerPromise = new Promise(resolve => { + innerResolve = resolve; + }); + await actAsync(() => { + render(); + }); + + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + ▾ + + [suspense-root] rects={[{x:1,y:2,width:13,height:1}, {x:1,y:2,width:13,height:1}, {x:1,y:2,width:13,height:1}]} + + + `); + + await actAsync(() => { + outerResolve('..'); + innerResolve('.'); + }); + + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + ▾ + + ▾ + + [suspense-root] rects={[{x:1,y:2,width:15,height:1}, {x:1,y:2,width:14,height:1}]} + + + `); + }); }); diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 2cd037f78d47..8b7e20dcf09f 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -2586,6 +2586,17 @@ export function attach( } } } else { + const suspenseNode = fiberInstance.suspenseNode; + if (suspenseNode !== null && fiber.memoizedState === null) { + // We're reconnecting an unsuspended Suspense. Measure to see if anything changed. + const prevRects = suspenseNode.rects; + const nextRects = measureInstance(fiberInstance); + if (!areEqualRects(prevRects, nextRects)) { + suspenseNode.rects = nextRects; + recordSuspenseResize(suspenseNode); + } + } + const {key} = fiber; const displayName = getDisplayNameForFiber(fiber); const elementType = getElementTypeForFiber(fiber); diff --git a/packages/react-devtools-shared/src/devtools/store.js b/packages/react-devtools-shared/src/devtools/store.js index 420817198b73..2631893a543c 100644 --- a/packages/react-devtools-shared/src/devtools/store.js +++ b/packages/react-devtools-shared/src/devtools/store.js @@ -669,6 +669,10 @@ export default class Store extends EventEmitter<{ return element; } + containsSuspense(id: SuspenseNode['id']): boolean { + return this._idToSuspense.has(id); + } + getSuspenseByID(id: SuspenseNode['id']): SuspenseNode | null { const suspense = this._idToSuspense.get(id); if (suspense === undefined) { diff --git a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseRects.js b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseRects.js index 221a6c90f823..f9ea6fd1f327 100644 --- a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseRects.js +++ b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseRects.js @@ -511,7 +511,11 @@ function SuspenseRectsContainer({ let selectedEnvironment = null; if (isRootSelected) { selectedEnvironment = rootEnvironment; - } else if (inspectedElementID !== null) { + } else if ( + inspectedElementID !== null && + // TODO: Separate inspected element and inspected Suspense and use the inspected Suspense ID here. + store.containsSuspense(inspectedElementID) + ) { const selectedSuspenseNode = store.getSuspenseByID(inspectedElementID); if ( selectedSuspenseNode !== null &&