diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index 1d2e155848..855bc039ab 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -231,6 +231,8 @@ const EnvironmentConfigSchema = z.object({ */ enableUseTypeAnnotations: z.boolean().default(false), + enableFunctionDependencyRewrite: z.boolean().default(true), + /** * Enables inlining ReactElement object literals in place of JSX * An alternative to the standard JSX transform which replaces JSX with React's jsxProd() runtime diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts index 8f4abdf6da..bd938db03e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts @@ -669,35 +669,55 @@ function collectDependencies( ): boolean => { return processedInstrsInOptional.get(fn)?.has(id) ?? false; }; - for (const [blockId, block] of fn.body.blocks) { - scopeTraversal.recordScopes(block); - const scopeBlockInfo = scopeTraversal.blockInfos.get(blockId); - if (scopeBlockInfo?.kind === 'begin') { - context.enterScope(scopeBlockInfo.scope); - } else if (scopeBlockInfo?.kind === 'end') { - context.exitScope(scopeBlockInfo.scope, scopeBlockInfo?.pruned); - } - // Record referenced optional chains in phis - for (const phi of block.phis) { - for (const operand of phi.operands) { - const maybeOptionalChain = temporaries.get(operand[1].identifier.id); - if (maybeOptionalChain) { - context.visitDependency(maybeOptionalChain); + const handleFunction = (fn: HIRFunction): void => { + for (const [blockId, block] of fn.body.blocks) { + scopeTraversal.recordScopes(block); + const scopeBlockInfo = scopeTraversal.blockInfos.get(blockId); + if (scopeBlockInfo?.kind === 'begin') { + context.enterScope(scopeBlockInfo.scope); + } else if (scopeBlockInfo?.kind === 'end') { + context.exitScope(scopeBlockInfo.scope, scopeBlockInfo.pruned); + } + // Record referenced optional chains in phis + for (const phi of block.phis) { + for (const operand of phi.operands) { + const maybeOptionalChain = temporaries.get(operand[1].identifier.id); + if (maybeOptionalChain) { + context.visitDependency(maybeOptionalChain); + } + } + } + for (const instr of block.instructions) { + if ( + fn.env.config.enableFunctionDependencyRewrite && + (instr.value.kind === 'FunctionExpression' || + instr.value.kind === 'ObjectMethod') + ) { + context.declare(instr.lvalue.identifier, { + id: instr.id, + scope: context.currentScope, + }); + /** + * Recursively visit the inner function to extract dependencies there + */ + const wasInInnerFn = context.inInnerFn; + context.inInnerFn = true; + handleFunction(instr.value.loweredFunc.func); + context.inInnerFn = wasInInnerFn; + } else if (!shouldSkipInstructionDependencies(fn, instr.id)) { + handleInstruction(instr, context); + } + } + + if (!shouldSkipInstructionDependencies(fn, block.terminal.id)) { + for (const place of eachTerminalOperand(block.terminal)) { + context.visitOperand(place); } } } - for (const instr of block.instructions) { - if (!shouldSkipInstructionDependencies(fn, instr.id)) { - handleInstruction(instr, context); - } - } + }; - if (!shouldSkipInstructionDependencies(fn, block.terminal.id)) { - for (const place of eachTerminalOperand(block.terminal)) { - context.visitOperand(place); - } - } - } + handleFunction(fn); return context.deps; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-mutate-2.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-mutate-2.expect.md index b31a16da90..c071d5d20e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-mutate-2.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-mutate-2.expect.md @@ -26,29 +26,20 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; function component(a, b) { - const $ = _c(5); - let t0; - if ($[0] !== b) { - t0 = { b }; - $[0] = b; - $[1] = t0; - } else { - t0 = $[1]; - } - const y = t0; + const $ = _c(2); + const y = { b }; let z; - if ($[2] !== a || $[3] !== y) { + if ($[0] !== a) { z = { a }; const x = function () { z.a = 2; }; x(); - $[2] = a; - $[3] = y; - $[4] = z; + $[0] = a; + $[1] = z; } else { - z = $[4]; + z = $[1]; } return z; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.expect.md new file mode 100644 index 0000000000..ae44f27912 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.expect.md @@ -0,0 +1,53 @@ + +## Input + +```javascript +// @validatePreserveExistingMemoizationGuarantees +import {useCallback} from 'react'; +import {Stringify} from 'shared-runtime'; + +/** + * TODO: we're currently bailing out because `contextVar` is a context variable + * and not recorded into the PropagateScopeDeps LoadLocal / PropertyLoad + * sidemap. Previously, we were able to avoid this as `BuildHIR` hoisted + * `LoadContext` and `PropertyLoad` instructions into the outer function, which + * we took as eligible dependencies. + * + * One solution is to simply record `LoadContext` identifiers into the + * temporaries sidemap when the instruction occurs *after* the context + * variable's mutable range. + */ +function Foo(props) { + let contextVar; + if (props.cond) { + contextVar = {val: 2}; + } else { + contextVar = {}; + } + + const cb = useCallback(() => [contextVar.val], [contextVar.val]); + + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{cond: true}], +}; + +``` + + +## Error + +``` + 22 | } + 23 | +> 24 | const cb = useCallback(() => [contextVar.val], [contextVar.val]); + | ^^^^^^^^^^^^^^^^^^^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected (24:24) + 25 | + 26 | return ; + 27 | } +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.tsx new file mode 100644 index 0000000000..8447e3960d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.tsx @@ -0,0 +1,32 @@ +// @validatePreserveExistingMemoizationGuarantees +import {useCallback} from 'react'; +import {Stringify} from 'shared-runtime'; + +/** + * TODO: we're currently bailing out because `contextVar` is a context variable + * and not recorded into the PropagateScopeDeps LoadLocal / PropertyLoad + * sidemap. Previously, we were able to avoid this as `BuildHIR` hoisted + * `LoadContext` and `PropertyLoad` instructions into the outer function, which + * we took as eligible dependencies. + * + * One solution is to simply record `LoadContext` identifiers into the + * temporaries sidemap when the instruction occurs *after* the context + * variable's mutable range. + */ +function Foo(props) { + let contextVar; + if (props.cond) { + contextVar = {val: 2}; + } else { + contextVar = {}; + } + + const cb = useCallback(() => [contextVar.val], [contextVar.val]); + + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{cond: true}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useCallback-infer-less-specific-conditional-access.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useCallback-infer-less-specific-conditional-access.expect.md index 955d391f91..940b3975c1 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useCallback-infer-less-specific-conditional-access.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useCallback-infer-less-specific-conditional-access.expect.md @@ -44,8 +44,6 @@ function Component({propA, propB}) { | ^^^^^^^^^^^^^^^^^ > 14 | }, [propA?.a, propB.x.y]); | ^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected (6:14) - -CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected (6:14) 15 | } 16 | ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.expect.md deleted file mode 100644 index db69bc2821..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.expect.md +++ /dev/null @@ -1,81 +0,0 @@ - -## Input - -```javascript -// @validatePreserveExistingMemoizationGuarantees -import {useCallback} from 'react'; -import {Stringify} from 'shared-runtime'; - -function Foo(props) { - let contextVar; - if (props.cond) { - contextVar = {val: 2}; - } else { - contextVar = {}; - } - - const cb = useCallback(() => [contextVar.val], [contextVar.val]); - - return ; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Foo, - params: [{cond: true}], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees -import { useCallback } from "react"; -import { Stringify } from "shared-runtime"; - -function Foo(props) { - const $ = _c(6); - let contextVar; - if ($[0] !== props.cond) { - if (props.cond) { - contextVar = { val: 2 }; - } else { - contextVar = {}; - } - $[0] = props.cond; - $[1] = contextVar; - } else { - contextVar = $[1]; - } - - const t0 = contextVar; - let t1; - if ($[2] !== t0.val) { - t1 = () => [contextVar.val]; - $[2] = t0.val; - $[3] = t1; - } else { - t1 = $[3]; - } - contextVar; - const cb = t1; - let t2; - if ($[4] !== cb) { - t2 = ; - $[4] = cb; - $[5] = t2; - } else { - t2 = $[5]; - } - return t2; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Foo, - params: [{ cond: true }], -}; - -``` - -### Eval output -(kind: ok)
{"cb":{"kind":"Function","result":[2]},"shouldInvokeFns":true}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.tsx deleted file mode 100644 index cb6f65a9f4..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.tsx +++ /dev/null @@ -1,21 +0,0 @@ -// @validatePreserveExistingMemoizationGuarantees -import {useCallback} from 'react'; -import {Stringify} from 'shared-runtime'; - -function Foo(props) { - let contextVar; - if (props.cond) { - contextVar = {val: 2}; - } else { - contextVar = {}; - } - - const cb = useCallback(() => [contextVar.val], [contextVar.val]); - - return ; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Foo, - params: [{cond: true}], -}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context.expect.md index b66661fbca..41994e1e56 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context.expect.md @@ -45,18 +45,16 @@ function Foo(props) { } else { x = $[1]; } - - const t0 = x; - let t1; - if ($[2] !== t0) { - t1 = () => [x]; - $[2] = t0; - $[3] = t1; + let t0; + if ($[2] !== x) { + t0 = () => [x]; + $[2] = x; + $[3] = t0; } else { - t1 = $[3]; + t0 = $[3]; } x; - const cb = t1; + const cb = t0; return cb; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-extended-contextvar-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-extended-contextvar-scope.expect.md index b141c27614..9ce4a62e71 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-extended-contextvar-scope.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-extended-contextvar-scope.expect.md @@ -70,28 +70,26 @@ function useBar(t0, cond) { if (cond) { x = b; } - - const t2 = x; - let t3; - if ($[1] !== a || $[2] !== t2) { - t3 = () => [a, x]; + let t2; + if ($[1] !== a || $[2] !== x) { + t2 = () => [a, x]; $[1] = a; - $[2] = t2; - $[3] = t3; + $[2] = x; + $[3] = t2; } else { - t3 = $[3]; + t2 = $[3]; } x; - const cb = t3; - let t4; + const cb = t2; + let t3; if ($[4] !== cb) { - t4 = ; + t3 = ; $[4] = cb; - $[5] = t4; + $[5] = t3; } else { - t4 = $[5]; + t3 = $[5]; } - return t4; + return t3; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md index 02e60eff91..ed56ff0681 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md @@ -34,9 +34,9 @@ function useFoo(t0) { const $ = _c(2); const { a } = t0; let t1; - if ($[0] !== a.b) { + if ($[0] !== a.b?.c.d?.e) { t1 = a.b?.c.d?.e} shouldInvokeFns={true} />; - $[0] = a.b; + $[0] = a.b?.c.d?.e; $[1] = t1; } else { t1 = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/react-namespace.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/react-namespace.expect.md index 0afc5b651b..cab231da32 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/react-namespace.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/react-namespace.expect.md @@ -29,36 +29,38 @@ import { c as _c } from "react/compiler-runtime"; const FooContext = React.createContext({ current: null }); function Component(props) { - const $ = _c(5); + const $ = _c(7); React.useContext(FooContext); const ref = React.useRef(); const [x, setX] = React.useState(false); let t0; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + if ($[0] !== ref) { t0 = () => { setX(true); ref.current = true; }; - $[0] = t0; + $[0] = ref; + $[1] = t0; } else { - t0 = $[0]; + t0 = $[1]; } const onClick = t0; let t1; - if ($[1] !== props.children) { + if ($[2] !== props.children) { t1 = React.cloneElement(props.children); - $[1] = props.children; - $[2] = t1; + $[2] = props.children; + $[3] = t1; } else { - t1 = $[2]; + t1 = $[3]; } let t2; - if ($[3] !== t1) { + if ($[4] !== onClick || $[5] !== t1) { t2 =
{t1}
; - $[3] = t1; - $[4] = t2; + $[4] = onClick; + $[5] = t1; + $[6] = t2; } else { - t2 = $[4]; + t2 = $[6]; } return t2; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md index 157e2de81a..bb99a5d90f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md @@ -31,9 +31,9 @@ function useFoo(t0) { const $ = _c(2); const { a } = t0; let t1; - if ($[0] !== a.b) { + if ($[0] !== a.b?.c.d?.e) { t1 = a.b?.c.d?.e} shouldInvokeFns={true} />; - $[0] = a.b; + $[0] = a.b?.c.d?.e; $[1] = t1; } else { t1 = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-parameter-mutate-in-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-parameter-mutate-in-effect.expect.md index 8b5a2eb1a0..95c6a403de 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-parameter-mutate-in-effect.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-parameter-mutate-in-effect.expect.md @@ -26,28 +26,32 @@ import { c as _c } from "react/compiler-runtime"; import { useEffect } from "react"; function Foo(props, ref) { - const $ = _c(4); + const $ = _c(5); let t0; - let t1; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + if ($[0] !== ref) { t0 = () => { ref.current = 2; }; - t1 = []; - $[0] = t0; - $[1] = t1; + $[0] = ref; + $[1] = t0; } else { - t0 = $[0]; - t1 = $[1]; + t0 = $[1]; + } + let t1; + if ($[2] === Symbol.for("react.memo_cache_sentinel")) { + t1 = []; + $[2] = t1; + } else { + t1 = $[2]; } useEffect(t0, t1); let t2; - if ($[2] !== props.bar) { + if ($[3] !== props.bar) { t2 =
{props.bar}
; - $[2] = props.bar; - $[3] = t2; + $[3] = props.bar; + $[4] = t2; } else { - t2 = $[3]; + t2 = $[4]; } return t2; }