From 21c1d51acb2c38b774e254e2a0022b044eacb548 Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Mon, 10 Nov 2025 20:01:59 +0100 Subject: [PATCH 1/8] [DevTools] Don't attempt to draw bounding box if inspected element is not a Suspense (#35097) --- packages/react-devtools-shared/src/devtools/store.js | 4 ++++ .../src/devtools/views/SuspenseTab/SuspenseRects.js | 6 +++++- 2 files changed, 9 insertions(+), 1 deletion(-) 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 && From ce4054ebdd550237cf20a509f8503d4624cbaffa Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Mon, 10 Nov 2025 20:55:31 +0100 Subject: [PATCH 2/8] [DevTools] Measure when reconnecting Suspense (#35098) --- .../src/__tests__/store-test.js | 96 +++++++++++++++++++ .../src/backend/fiber/renderer.js | 11 +++ 2 files changed, 107 insertions(+) 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); From 01fb3286321b6190b06b1cc86c7c1cd9e2d884d9 Mon Sep 17 00:00:00 2001 From: Jorge Cabiedes <57368278+jorge-cab@users.noreply.github.com> Date: Mon, 10 Nov 2025 12:08:05 -0800 Subject: [PATCH 3/8] [compiler] Prevent overriding a derivationEntry on effect mutation and instead update typeOfValue and fix infinite loops (#34967) Summary: With this we are now comparing a snapshot of the derivationCache with the new changes every time we are done recording the derivations happening in the HIR. We have to do this after recording everything since we still do some mutations on the cache when recording mutations. Test Plan: Test the following in playground: ``` // @validateNoDerivedComputationsInEffects_exp function Component({ value }) { const [checked, setChecked] = useState(''); useEffect(() => { setChecked(value === '' ? [] : value.split(',')); }, [value]); return (
{checked}
) } ``` This no longer causes an infinite loop. Added a test case in the next PR in the stack --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34967). * #35044 * #35020 * #34973 * #34972 * #34995 * __->__ #34967 --- ...idateNoDerivedComputationsInEffects_exp.ts | 74 +++++++++++++++---- 1 file changed, 59 insertions(+), 15 deletions(-) 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..e30c9e8581e 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 @@ -47,6 +47,43 @@ type ValidationContext = { class DerivationCache { hasChanges: boolean = false; cache: Map = new Map(); + private previousCache: Map | null = null; + + takeSnapshot(): void { + this.previousCache = new Map(); + for (const [key, value] of this.cache.entries()) { + this.previousCache.set(key, { + place: value.place, + sourcesIds: new Set(value.sourcesIds), + typeOfValue: value.typeOfValue, + }); + } + } + + checkForChanges(): void { + if (this.previousCache === null) { + this.hasChanges = true; + return; + } + + for (const [key, value] of this.cache.entries()) { + const previousValue = this.previousCache.get(key); + if ( + previousValue === undefined || + !this.isDerivationEqual(previousValue, value) + ) { + this.hasChanges = true; + return; + } + } + + if (this.cache.size !== this.previousCache.size) { + this.hasChanges = true; + return; + } + + this.hasChanges = false; + } snapshot(): boolean { const hasChanges = this.hasChanges; @@ -92,14 +129,7 @@ class DerivationCache { 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, newValue); } private isDerivationEqual( @@ -175,7 +205,6 @@ export function validateNoDerivedComputationsInEffects_exp( sourcesIds: new Set([param.identifier.id]), typeOfValue: 'fromProps', }); - context.derivationCache.hasChanges = true; } } } else if (fn.fnType === 'Component') { @@ -186,12 +215,13 @@ export function validateNoDerivedComputationsInEffects_exp( sourcesIds: new Set([props.identifier.id]), typeOfValue: 'fromProps', }); - context.derivationCache.hasChanges = true; } } let isFirstPass = true; do { + context.derivationCache.takeSnapshot(); + for (const block of fn.body.blocks.values()) { recordPhiDerivations(block, context); for (const instr of block.instructions) { @@ -199,6 +229,7 @@ export function validateNoDerivedComputationsInEffects_exp( } } + context.derivationCache.checkForChanges(); isFirstPass = false; } while (context.derivationCache.snapshot()); @@ -331,11 +362,24 @@ function recordInstructionDerivations( case Effect.ConditionallyMutateIterator: case Effect.Mutate: { if (isMutable(instr, operand)) { - context.derivationCache.addDerivationEntry( - operand, - sources, - typeOfValue, - ); + if (context.derivationCache.cache.has(operand.identifier.id)) { + const operandMetadata = context.derivationCache.cache.get( + operand.identifier.id, + ); + + if (operandMetadata !== undefined) { + operandMetadata.typeOfValue = joinValue( + typeOfValue, + operandMetadata.typeOfValue, + ); + } + } else { + context.derivationCache.addDerivationEntry( + operand, + sources, + typeOfValue, + ); + } } break; } From 6347c6d37336c7791098d2d817b22f02ea41a5d3 Mon Sep 17 00:00:00 2001 From: Jorge Cabiedes <57368278+jorge-cab@users.noreply.github.com> Date: Mon, 10 Nov 2025 12:09:13 -0800 Subject: [PATCH 4/8] [compiler] Fix false negatives and add data flow tree to compiler error for `no-deriving-state-in-effects` (#34995) Summary: Revamped the derivationCache graph. This fixes a bunch of bugs where sometimes we fail to track from which props/state we derived values from. Also, it is more intuitive and allows us to easily implement a Data Flow Tree. We can print this tree which gives insight on how the data is derived and should facilitate error resolution in complicated components Test Plan: Added a test case where we were failing to track derivations. Also updated the test cases with the new error containing the data flow tree --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34995). * #35044 * #35020 * #34973 * #34972 * __->__ #34995 * #34967 --- ...idateNoDerivedComputationsInEffects_exp.ts | 253 ++++++++++++++---- ...ed-state-conditionally-in-effect.expect.md | 11 +- ...derived-state-from-default-props.expect.md | 11 +- ...state-from-local-state-in-effect.expect.md | 11 +- ...-local-state-and-component-scope.expect.md | 13 +- ...d-state-from-prop-setter-ternary.expect.md | 48 ++++ ....derived-state-from-prop-setter-ternary.js | 11 + ...state-from-prop-with-side-effect.expect.md | 11 +- ...ect-contains-local-function-call.expect.md | 11 +- ...id-derived-computation-in-effect.expect.md | 11 +- ...erived-state-from-computed-props.expect.md | 12 +- ...ed-state-from-destructured-props.expect.md | 11 +- 12 files changed, 350 insertions(+), 64 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-setter-ternary.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-setter-ternary.js 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 e30c9e8581e..cdc884e945c 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 @@ -33,6 +33,7 @@ type DerivationMetadata = { typeOfValue: TypeOfValue; place: Place; sourcesIds: Set; + isStateSource: boolean; }; type ValidationContext = { @@ -56,6 +57,7 @@ class DerivationCache { place: value.place, sourcesIds: new Set(value.sourcesIds), typeOfValue: value.typeOfValue, + isStateSource: value.isStateSource, }); } } @@ -95,41 +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); - } - - this.cache.set(derivedVar.identifier.id, newValue); + this.cache.set(derivedVar.identifier.id, { + place: derivedVar, + sourcesIds: sourcesIds, + typeOfValue: typeOfValue ?? 'ignored', + isStateSource: finalIsSource, + }); } private isDerivationEqual( @@ -151,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. @@ -202,8 +199,9 @@ 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, }); } } @@ -212,8 +210,9 @@ 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, }); } } @@ -267,6 +266,7 @@ function recordPhiDerivations( phi.place, sourcesIds, typeOfValue, + false, ); } } @@ -288,11 +288,13 @@ function recordInstructionDerivations( isFirstPass: boolean, ): void { 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); } @@ -311,10 +313,7 @@ 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); - } + isSource = true; typeOfValue = joinValue(typeOfValue, 'fromState'); } } @@ -341,9 +340,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') { @@ -351,7 +348,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)) { @@ -378,6 +380,7 @@ function recordInstructionDerivations( operand, sources, typeOfValue, + false, ); } } @@ -411,6 +414,117 @@ 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 validateEffect( effectFunction: HIRFunction, context: ValidationContext, @@ -513,27 +627,56 @@ function validateEffect( .length - 1 ) { - const derivedDepsStr = Array.from(derivedSetStateCall.sourceIds) - .map(sourceId => { - const sourceMetadata = context.derivationCache.cache.get(sourceId); - return sourceMetadata?.place.identifier.name?.value; - }) - .filter(Boolean) - .join(', '); - - let description; - - if (derivedSetStateCall.typeOfValue === 'fromProps') { - description = `From props: [${derivedDepsStr}]`; - } else if (derivedSetStateCall.typeOfValue === 'fromState') { - description = `From local state: [${derivedDepsStr}]`; - } else { - description = `From props and local state: [${derivedDepsStr}]`; + const 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, + ), + ); + + 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/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 index 1fa7f7d7950..52074295ffa 100644 --- 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 @@ -34,7 +34,16 @@ 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. +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: + +Props: [value] + +Data Flow Tree: +└── value (Prop) + +See: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state. error.derived-state-conditionally-in-effect.ts:9:6 7 | useEffect(() => { 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 index f30235a064a..a8ec5240c8f 100644 --- 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 @@ -31,7 +31,16 @@ 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. +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: + +Props: [input] + +Data Flow Tree: +└── input (Prop) + +See: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state. error.derived-state-from-default-props.ts:9:4 7 | 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 index 779ddafc401..59a9e8845b0 100644 --- 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 @@ -28,7 +28,16 @@ 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. +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: + +State: [count] + +Data Flow Tree: +└── count (State) + +See: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state. error.derived-state-from-local-state-in-effect.ts:10:6 8 | useEffect(() => { 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 index 7b27b556b3e..919bf067ba5 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.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 @@ -38,7 +38,18 @@ 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. +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: + +Props: [firstName] +State: [lastName] + +Data Flow Tree: +├── firstName (Prop) +└── lastName (State) + +See: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state. error.derived-state-from-prop-local-state-and-component-scope.ts:11:4 9 | diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-setter-ternary.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-setter-ternary.expect.md new file mode 100644 index 00000000000..3d901ff48ff --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-setter-ternary.expect.md @@ -0,0 +1,48 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp + +function Component({value}) { + const [checked, setChecked] = useState(''); + + useEffect(() => { + setChecked(value === '' ? [] : value.split(',')); + }, [value]); + + return
{checked}
; +} + +``` + + +## Error + +``` +Found 1 error: + +Error: You might not need an effect. Derive values in render, not effects. + +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: + +Props: [value] + +Data Flow Tree: +└── value (Prop) + +See: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state. + +error.derived-state-from-prop-setter-ternary.ts:7:4 + 5 | + 6 | useEffect(() => { +> 7 | setChecked(value === '' ? [] : value.split(',')); + | ^^^^^^^^^^ This should be computed during render, not in an effect + 8 | }, [value]); + 9 | + 10 | return
{checked}
; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-setter-ternary.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.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/error.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/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 index 7fadae5667f..370d7f31307 100644 --- 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 @@ -31,7 +31,16 @@ 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. +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: + +Props: [value] + +Data Flow Tree: +└── value (Prop) + +See: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state. error.derived-state-from-prop-with-side-effect.ts:8:4 6 | 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 index aec543fcbf4..9fe34e01a50 100644 --- 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 @@ -35,7 +35,16 @@ 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. +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: + +Props: [propValue] + +Data Flow Tree: +└── propValue (Prop) + +See: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state. error.effect-contains-local-function-call.ts:12:4 10 | 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 index f1f755adfa8..5714131c0d7 100644 --- 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 @@ -33,7 +33,16 @@ 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. +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: + +State: [firstName] + +Data Flow Tree: +└── firstName (State) + +See: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state. error.invalid-derived-computation-in-effect.ts:11:4 9 | const [fullName, setFullName] = useState(''); 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 index 3a078896939..939de631f97 100644 --- 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 @@ -31,7 +31,17 @@ 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. +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: + +Props: [props] + +Data Flow Tree: +└── computed + └── props (Prop) + +See: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state. error.invalid-derived-state-from-computed-props.ts:9:4 7 | useEffect(() => { 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 index b28692c67b3..8abc7d6bbdf 100644 --- 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 @@ -32,7 +32,16 @@ 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. +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: + +Props: [props] + +Data Flow Tree: +└── props (Prop) + +See: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state. error.invalid-derived-state-from-destructured-props.ts:10:4 8 | From 72961203966a2f1d34dfca089e0a94a94ead7658 Mon Sep 17 00:00:00 2001 From: Jorge Cabiedes <57368278+jorge-cab@users.noreply.github.com> Date: Mon, 10 Nov 2025 12:16:13 -0800 Subject: [PATCH 5/8] [compiler] Update ValidateNoDerivedComputationsInEffects_exp to log the error instead of throwing (#34972) Summary: TSIA Simple change to log errors in Pipeline.ts instead of throwing in the validation Test Plan: updated snap tests --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34972). * #35044 * #35020 * #34973 * __->__ #34972 --- .../src/Entrypoint/Pipeline.ts | 2 +- ...idateNoDerivedComputationsInEffects_exp.ts | 7 +- ...ed-state-conditionally-in-effect.expect.md | 86 +++++++++++++ ... derived-state-conditionally-in-effect.js} | 2 +- ...derived-state-from-default-props.expect.md | 78 ++++++++++++ ...js => derived-state-from-default-props.js} | 2 +- ...state-from-local-state-in-effect.expect.md | 77 ++++++++++++ ...rived-state-from-local-state-in-effect.js} | 2 +- ...-local-state-and-component-scope.expect.md | 115 ++++++++++++++++++ ...m-prop-local-state-and-component-scope.js} | 2 +- ...ter-call-outside-effect-no-error.expect.md | 10 +- ...rop-setter-call-outside-effect-no-error.js | 2 +- ...d-state-from-prop-setter-ternary.expect.md | 57 +++++++++ ...derived-state-from-prop-setter-ternary.js} | 0 ...ter-used-outside-effect-no-error.expect.md | 11 +- ...rop-setter-used-outside-effect-no-error.js | 2 +- ...state-from-prop-with-side-effect.expect.md | 78 ++++++++++++ ...rived-state-from-prop-with-side-effect.js} | 2 +- ...tate-from-ref-and-state-no-error.expect.md | 10 +- ...rived-state-from-ref-and-state-no-error.js | 2 +- ...ect-contains-local-function-call.expect.md | 93 ++++++++++++++ ...=> effect-contains-local-function-call.js} | 2 +- ...ains-prop-function-call-no-error.expect.md | 11 +- ...ct-contains-prop-function-call-no-error.js | 2 +- ...th-global-function-call-no-error.expect.md | 10 +- ...fect-with-global-function-call-no-error.js | 2 +- ...ed-state-conditionally-in-effect.expect.md | 58 --------- ...derived-state-from-default-props.expect.md | 55 --------- ...state-from-local-state-in-effect.expect.md | 52 -------- ...-local-state-and-component-scope.expect.md | 64 ---------- ...d-state-from-prop-setter-ternary.expect.md | 48 -------- ...state-from-prop-with-side-effect.expect.md | 55 --------- ...ect-contains-local-function-call.expect.md | 59 --------- ...id-derived-computation-in-effect.expect.md | 57 --------- ...erived-state-from-computed-props.expect.md | 56 --------- ...ed-state-from-destructured-props.expect.md | 56 --------- ...id-derived-computation-in-effect.expect.md | 80 ++++++++++++ ... invalid-derived-computation-in-effect.js} | 2 +- ...erived-state-from-computed-props.expect.md | 79 ++++++++++++ ...alid-derived-state-from-computed-props.js} | 2 +- ...ed-state-from-destructured-props.expect.md | 81 ++++++++++++ ...-derived-state-from-destructured-props.js} | 2 +- ...f-conditional-in-effect-no-error.expect.md | 10 +- .../ref-conditional-in-effect-no-error.js | 2 +- 44 files changed, 893 insertions(+), 592 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-conditionally-in-effect.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/{error.derived-state-conditionally-in-effect.js => derived-state-conditionally-in-effect.js} (86%) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-default-props.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/{error.derived-state-from-default-props.js => derived-state-from-default-props.js} (86%) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-local-state-in-effect.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/{error.derived-state-from-local-state-in-effect.js => derived-state-from-local-state-in-effect.js} (79%) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-local-state-and-component-scope.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/{error.derived-state-from-prop-local-state-and-component-scope.js => derived-state-from-prop-local-state-and-component-scope.js} (90%) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-setter-ternary.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/{error.derived-state-from-prop-setter-ternary.js => derived-state-from-prop-setter-ternary.js} (100%) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-with-side-effect.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/{error.derived-state-from-prop-with-side-effect.js => derived-state-from-prop-with-side-effect.js} (84%) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-contains-local-function-call.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/{error.effect-contains-local-function-call.js => effect-contains-local-function-call.js} (86%) delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-conditionally-in-effect.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-default-props.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-local-state-in-effect.expect.md delete mode 100644 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 delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-setter-ternary.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-with-side-effect.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.effect-contains-local-function-call.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-computation-in-effect.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-state-from-computed-props.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.invalid-derived-state-from-destructured-props.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-computation-in-effect.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/{error.invalid-derived-computation-in-effect.js => invalid-derived-computation-in-effect.js} (87%) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-state-from-computed-props.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/{error.invalid-derived-state-from-computed-props.js => invalid-derived-state-from-computed-props.js} (87%) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/invalid-derived-state-from-destructured-props.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/{error.invalid-derived-state-from-destructured-props.js => invalid-derived-state-from-destructured-props.js} (87%) 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 cdc884e945c..9a92566c2c9 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 { @@ -173,7 +174,7 @@ function isNamedIdentifier(place: Place): place is Place & { */ export function validateNoDerivedComputationsInEffects_exp( fn: HIRFunction, -): void { +): Result { const functions: Map = new Map(); const derivationCache = new DerivationCache(); const errors = new CompilerError(); @@ -236,9 +237,7 @@ export function validateNoDerivedComputationsInEffects_exp( validateEffect(effect, context); } - if (errors.hasAnyErrors()) { - throw errors; - } + return errors.asResult(); } function recordPhiDerivations( 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/error.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 similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-setter-ternary.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-setter-ternary.js 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-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 52074295ffa..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,58 +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. - -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: - -Props: [value] - -Data Flow Tree: -└── value (Prop) - -See: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state. - -error.derived-state-conditionally-in-effect.ts:9:6 - 7 | useEffect(() => { - 8 | if (enabled) { -> 9 | setLocalValue(value); - | ^^^^^^^^^^^^^ 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 a8ec5240c8f..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,55 +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. - -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: - -Props: [input] - -Data Flow Tree: -└── input (Prop) - -See: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state. - -error.derived-state-from-default-props.ts:9:4 - 7 | - 8 | useEffect(() => { -> 9 | setCurrInput(input + localConst); - | ^^^^^^^^^^^^ 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 59a9e8845b0..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,52 +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. - -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: - -State: [count] - -Data Flow Tree: -└── count (State) - -See: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state. - -error.derived-state-from-local-state-in-effect.ts:10:6 - 8 | useEffect(() => { - 9 | if (shouldChange) { -> 10 | setCount(count + 1); - | ^^^^^^^^ 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 919bf067ba5..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,64 +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. - -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: - -Props: [firstName] -State: [lastName] - -Data Flow Tree: -├── firstName (Prop) -└── lastName (State) - -See: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state. - -error.derived-state-from-prop-local-state-and-component-scope.ts:11:4 - 9 | - 10 | useEffect(() => { -> 11 | setFullName(firstName + ' ' + middleName + ' ' + lastName); - | ^^^^^^^^^^^ 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-setter-ternary.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-setter-ternary.expect.md deleted file mode 100644 index 3d901ff48ff..00000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-setter-ternary.expect.md +++ /dev/null @@ -1,48 +0,0 @@ - -## Input - -```javascript -// @validateNoDerivedComputationsInEffects_exp - -function Component({value}) { - const [checked, setChecked] = useState(''); - - useEffect(() => { - setChecked(value === '' ? [] : value.split(',')); - }, [value]); - - return
{checked}
; -} - -``` - - -## Error - -``` -Found 1 error: - -Error: You might not need an effect. Derive values in render, not effects. - -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: - -Props: [value] - -Data Flow Tree: -└── value (Prop) - -See: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state. - -error.derived-state-from-prop-setter-ternary.ts:7:4 - 5 | - 6 | useEffect(() => { -> 7 | setChecked(value === '' ? [] : value.split(',')); - | ^^^^^^^^^^ This should be computed during render, not in an effect - 8 | }, [value]); - 9 | - 10 | return
{checked}
; -``` - - \ 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 370d7f31307..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,55 +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. - -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: - -Props: [value] - -Data Flow Tree: -└── value (Prop) - -See: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state. - -error.derived-state-from-prop-with-side-effect.ts:8:4 - 6 | - 7 | useEffect(() => { -> 8 | setLocalValue(value); - | ^^^^^^^^^^^^^ 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 9fe34e01a50..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,59 +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. - -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: - -Props: [propValue] - -Data Flow Tree: -└── propValue (Prop) - -See: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state. - -error.effect-contains-local-function-call.ts:12:4 - 10 | - 11 | useEffect(() => { -> 12 | setValue(propValue); - | ^^^^^^^^ 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 5714131c0d7..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,57 +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. - -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: - -State: [firstName] - -Data Flow Tree: -└── firstName (State) - -See: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state. - -error.invalid-derived-computation-in-effect.ts:11:4 - 9 | const [fullName, setFullName] = useState(''); - 10 | useEffect(() => { -> 11 | setFullName(firstName + ' ' + lastName); - | ^^^^^^^^^^^ 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 939de631f97..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,56 +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. - -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: - -Props: [props] - -Data Flow Tree: -└── computed - └── props (Prop) - -See: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state. - -error.invalid-derived-state-from-computed-props.ts:9:4 - 7 | useEffect(() => { - 8 | const computed = props.prefix + props.value + props.suffix; -> 9 | setDisplayValue(computed); - | ^^^^^^^^^^^^^^^ 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 8abc7d6bbdf..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,56 +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. - -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: - -Props: [props] - -Data Flow Tree: -└── props (Prop) - -See: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state. - -error.invalid-derived-state-from-destructured-props.ts:10:4 - 8 | - 9 | useEffect(() => { -> 10 | setFullName(props.firstName + ' ' + props.lastName); - | ^^^^^^^^^^^ 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}) { From f76c3617e0f00c98656a36d1b4083b397c7638f2 Mon Sep 17 00:00:00 2001 From: Jorge Cabiedes <57368278+jorge-cab@users.noreply.github.com> Date: Mon, 10 Nov 2025 12:16:27 -0800 Subject: [PATCH 6/8] [compiler] Switch to track setStates by aliasing and id instead of identifier names (#34973) Summary: This makes the setState usage logic much more robust. We no longer rely on identifierName. Now we track when a setState is loaded into a new promoted identifier variable and track this in a map `setStateLoaded` map. For other types of instructions we consider the setState to be being used. In this case we record its usage into the `setStateUsages` map. Test Plan: We expect no changes in behavior for the current tests --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34973). * #35044 * #35020 * __->__ #34973 * #34972 --- ...idateNoDerivedComputationsInEffects_exp.ts | 130 ++++++++++++------ 1 file changed, 89 insertions(+), 41 deletions(-) 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 9a92566c2c9..5cc9232ebfb 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 @@ -21,7 +21,6 @@ import { isUseStateType, BasicBlock, isUseRefType, - GeneratedSource, SourceLocation, } from '../HIR'; import {eachInstructionLValue, eachInstructionOperand} from '../HIR/visitors'; @@ -42,8 +41,8 @@ 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 { @@ -180,19 +179,16 @@ export function validateNoDerivedComputationsInEffects_exp( 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') { @@ -281,11 +277,61 @@ 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(); @@ -318,15 +364,13 @@ function recordInstructionDerivations( } 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); } } @@ -532,11 +576,16 @@ function validateEffect( const effectDerivedSetStateCalls: Array<{ value: CallExpression; - loc: SourceLocation; + id: IdentifierId; sourceIds: Set; typeOfValue: TypeOfValue; }> = []; + const effectSetStateUsages: Map< + IdentifierId, + Set + > = new Map(); + const globals: Set = new Set(); for (const block of effectFunction.body.blocks.values()) { for (const pred of block.preds) { @@ -552,19 +601,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); } } } @@ -582,7 +628,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, }); @@ -616,15 +662,17 @@ 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 propsSet = new Set(); const stateSet = new Set(); From 92ac4e8b80cb51a1be7071e8338176680ce8f619 Mon Sep 17 00:00:00 2001 From: Jorge Cabiedes <57368278+jorge-cab@users.noreply.github.com> Date: Mon, 10 Nov 2025 12:28:19 -0800 Subject: [PATCH 7/8] [compiler] Don't validate when effect cleanup function depends on effect localized setState state derived values (#35020) Summary: If we are using a clean up function in an effect and that clean up function depends on a value that is used to set the state we are validating for we shouldn't throw an error since it is a valid use case for an effect. Test Plan: added test --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35020). * #35044 * __->__ #35020 --- ...idateNoDerivedComputationsInEffects_exp.ts | 41 ++++++++++ ...ing-on-derived-computation-value.expect.md | 76 +++++++++++++++++++ ...-depending-on-derived-computation-value.js | 21 +++++ 3 files changed, 138 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-with-cleanup-function-depending-on-derived-computation-value.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-with-cleanup-function-depending-on-derived-computation-value.js 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 5cc9232ebfb..b0b680b0a9c 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 @@ -568,6 +568,26 @@ function renderTree( 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, @@ -586,8 +606,23 @@ function validateEffect( 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 @@ -698,6 +733,12 @@ function validateEffect( ), ); + for (const dep of derivedSetStateCall.sourceIds) { + if (cleanUpFunctionDeps !== undefined && cleanUpFunctionDeps.has(dep)) { + return; + } + } + const propsArr = Array.from(propsSet); const stateArr = Array.from(stateSet); 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 ; +} From 100fc4a8cf3b97de53b3566ed726b8ea7e6d7f81 Mon Sep 17 00:00:00 2001 From: Jorge Cabiedes <57368278+jorge-cab@users.noreply.github.com> Date: Mon, 10 Nov 2025 12:29:34 -0800 Subject: [PATCH 8/8] [compiler] Prevent local state source variables from depending on other state (#35044) Summary: When a local state is created sometimes it uses a `prop` or even other local state for its initial value. This value is only relevant on first render so we shouldn't consider it part of our data flow Test Plan: Added tests --- ...idateNoDerivedComputationsInEffects_exp.ts | 10 +++- ...m-prop-no-show-in-data-flow-tree.expect.md | 59 +++++++++++++++++++ ...ved-from-prop-no-show-in-data-flow-tree.js | 18 ++++++ 3 files changed, 85 insertions(+), 2 deletions(-) create mode 100644 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 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/usestate-derived-from-prop-no-show-in-data-flow-tree.js 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 b0b680b0a9c..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 @@ -358,8 +358,14 @@ function recordInstructionDerivations( context.effects.add(effectFunction.loweredFunc.func); } } else if (isUseStateType(lvalue.identifier) && value.args.length > 0) { - isSource = true; - typeOfValue = joinValue(typeOfValue, 'fromState'); + typeOfValue = 'fromState'; + context.derivationCache.addDerivationEntry( + lvalue, + new Set(), + typeOfValue, + true, + ); + return; } } 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}
; +}