diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects.ts index 01f88d4b8a..024ecd0ef2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects.ts @@ -18,6 +18,7 @@ import { CallExpression, Instruction, isUseStateType, + isUseRefType, } from '../HIR'; import {eachInstructionLValue, eachInstructionOperand} from '../HIR/visitors'; import {isMutable} from '../ReactiveScopes/InferReactiveScopeVariables'; @@ -284,6 +285,11 @@ function validateEffect( } for (const instr of block.instructions) { + // Early return if any instruction is deriving a value from a ref + if (isUseRefType(instr.lvalue.identifier)) { + return; + } + if ( instr.value.kind === 'CallExpression' && isSetStateType(instr.value.callee.identifier) && 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 new file mode 100644 index 0000000000..1bb5e18626 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-ref-and-state-no-error.expect.md @@ -0,0 +1,73 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects +import {useEffect, useState, useRef} from 'react'; + +export default function Component({test}) { + const [local, setLocal] = useState(''); + + const myRef = useRef(null); + + useEffect(() => { + setLocal(myRef.current + test); + }, [test]); + + return <>{local}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{test: 'testString'}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects +import { useEffect, useState, useRef } from "react"; + +export default function Component(t0) { + const $ = _c(5); + const { test } = t0; + const [local, setLocal] = useState(""); + + const myRef = useRef(null); + let t1; + let t2; + if ($[0] !== test) { + t1 = () => { + setLocal(myRef.current + test); + }; + t2 = [test]; + $[0] = test; + $[1] = t1; + $[2] = t2; + } else { + t1 = $[1]; + t2 = $[2]; + } + useEffect(t1, t2); + let t3; + if ($[3] !== local) { + t3 = <>{local}; + $[3] = local; + $[4] = t3; + } else { + t3 = $[4]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ test: "testString" }], +}; + +``` + +### 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/error.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 similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-ref-and-state-no-error.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-ref-and-state-no-error.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-ref-and-state-no-error.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-ref-and-state-no-error.expect.md deleted file mode 100644 index 9f5783d41a..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-ref-and-state-no-error.expect.md +++ /dev/null @@ -1,45 +0,0 @@ - -## Input - -```javascript -// @validateNoDerivedComputationsInEffects -import {useEffect, useState, useRef} from 'react'; - -export default function Component({test}) { - const [local, setLocal] = useState(''); - - const myRef = useRef(null); - - useEffect(() => { - setLocal(myRef.current + test); - }, [test]); - - return <>{local}; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{test: 'testString'}], -}; - -``` - - -## Error - -``` -Found 1 error: - -Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) - -error.derived-state-from-ref-and-state-no-error.ts:10:4 - 8 | - 9 | useEffect(() => { -> 10 | setLocal(myRef.current + test); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) - 11 | }, [test]); - 12 | - 13 | return <>{local}; -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.ref-conditional-in-effect-no-error.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.ref-conditional-in-effect-no-error.expect.md deleted file mode 100644 index 5aad420e8e..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.ref-conditional-in-effect-no-error.expect.md +++ /dev/null @@ -1,60 +0,0 @@ - -## Input - -```javascript -// @validateNoDerivedComputationsInEffects -import {useEffect, useState, useRef} from 'react'; - -export default function Component({test}) { - const [local, setLocal] = useState(0); - - const myRef = useRef(null); - - useEffect(() => { - if (myRef.current) { - setLocal(test); - } else { - setLocal(test + test); - } - }, [test]); - - return <>{local}; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{test: 4}], -}; - -``` - - -## Error - -``` -Found 2 errors: - -Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) - -error.ref-conditional-in-effect-no-error.ts:11:6 - 9 | useEffect(() => { - 10 | if (myRef.current) { -> 11 | setLocal(test); - | ^^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) - 12 | } else { - 13 | setLocal(test + test); - 14 | } - -Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) - -error.ref-conditional-in-effect-no-error.ts:13:6 - 11 | setLocal(test); - 12 | } else { -> 13 | setLocal(test + test); - | ^^^^^^^^^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) - 14 | } - 15 | }, [test]); - 16 | -``` - - \ 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.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/ref-conditional-in-effect-no-error.expect.md new file mode 100644 index 0000000000..3cb010baaf --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/ref-conditional-in-effect-no-error.expect.md @@ -0,0 +1,82 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects +import {useEffect, useState, useRef} from 'react'; + +export default function Component({test}) { + const [local, setLocal] = useState(0); + + const myRef = useRef(null); + + useEffect(() => { + if (myRef.current) { + setLocal(test); + } else { + setLocal(test + test); + } + }, [test]); + + return <>{local}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{test: 4}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects +import { useEffect, useState, useRef } from "react"; + +export default function Component(t0) { + const $ = _c(5); + const { test } = t0; + const [local, setLocal] = useState(0); + + const myRef = useRef(null); + let t1; + let t2; + if ($[0] !== test) { + t1 = () => { + if (myRef.current) { + setLocal(test); + } else { + setLocal(test + test); + } + }; + + t2 = [test]; + $[0] = test; + $[1] = t1; + $[2] = t2; + } else { + t1 = $[1]; + t2 = $[2]; + } + useEffect(t1, t2); + let t3; + if ($[3] !== local) { + t3 = <>{local}; + $[3] = local; + $[4] = t3; + } else { + t3 = $[4]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ test: 4 }], +}; + +``` + +### 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/error.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 similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.ref-conditional-in-effect-no-error.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/ref-conditional-in-effect-no-error.js