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 a83b22651e7..0c777f87707 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 a755d0e2c65..5c9462fd551 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 00000000000..756a219e645 --- /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 2ccd52500c7..fb65cbfeb82 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 00000000000..2f3a3d0e616 --- /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 1a0f5126e7a..1de911c9b37 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 00000000000..37458dcea06 --- /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 9568e490029..79e65e4849a 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 00000000000..fdcbccd3de5 --- /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 3090ef00412..f25e20863d3 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 ef817a3ebf2..69814825450 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 502402be51a..6df5f2eed6f 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 00000000000..48811aa5a94 --- /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 00000000000..afd198caa26 --- /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 2924de0da61..b5100dc2a63 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 d33af16ec59..43b5a8c52ad 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 00000000000..0160fbbb4a7 --- /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 88c66ce1ef6..5bb963daac3 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 4d0b6663e32..e88d5833a9a 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 6b24f73ac74..18ad2bdca19 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 00000000000..fdc7081f375 --- /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 1efb3177e51..a6442b36477 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 c83ea552a6a..74391e86ad3 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 512df7cb36e..f19e9518d6f 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 00000000000..e84031591e0 --- /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 00000000000..e419583cc6d --- /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 e17f1e26f6c..e26643723d9 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 4cded6dcc8e..ae7622d4d06 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 1fa7f7d7950..00000000000 --- 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 f30235a064a..00000000000 --- 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 779ddafc401..00000000000 --- 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 7b27b556b3e..00000000000 --- 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 7fadae5667f..00000000000 --- 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 aec543fcbf4..00000000000 --- 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 f1f755adfa8..00000000000 --- 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 3a078896939..00000000000 --- 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 b28692c67b3..00000000000 --- 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 00000000000..29dea440b46 --- /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 17779a5b4c5..e29ece67bd5 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 00000000000..c7199d95480 --- /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 24afa944fc5..39648ef8d5e 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 00000000000..5a6af555408 --- /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 bdfb47a2c6a..3f662f13f71 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 365ee1fef45..9a843d1883c 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 ee59ccb78f0..3594deaa02e 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 00000000000..7ab14466b25 --- /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 00000000000..5c62fa2e8f1 --- /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 9b2b2ae54ff..37272ea0579 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 2cd037f78d4..8b7e20dcf09 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 420817198b7..2631893a543 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 221a6c90f82..f9ea6fd1f32 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 &&