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 ; +}