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 90f53d495b5a3..fa581d8ed8d81 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -660,6 +660,13 @@ const testComplexConfigDefaults: PartialEnvironmentConfig = { }, numRequiredArgs: 2, }, + { + function: { + source: 'useEffectWrapper', + importSpecifierName: 'default', + }, + numRequiredArgs: 1, + }, ], }; @@ -1147,3 +1154,5 @@ export function tryParseExternalFunction( suggestions: null, }); } + +export const DEFAULT_EXPORT = 'default'; diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts index 9fb91a4ac829b..5936b1bc95bc4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts @@ -16,6 +16,7 @@ import { Place, ReactiveScopeDependencies, } from '../HIR'; +import {DEFAULT_EXPORT} from '../HIR/Environment'; import { createTemporaryPlace, fixScopeAndIdentifierRanges, @@ -97,6 +98,17 @@ export function inferEffectDependencies(fn: HIRFunction): void { autodepFnLoads.set(lvalue.identifier.id, numRequiredArgs); } } + } else if ( + value.kind === 'LoadGlobal' && + value.binding.kind === 'ImportDefault' + ) { + const moduleTargets = autodepFnConfigs.get(value.binding.module); + if (moduleTargets != null) { + const numRequiredArgs = moduleTargets.get(DEFAULT_EXPORT); + if (numRequiredArgs != null) { + autodepFnLoads.set(lvalue.identifier.id, numRequiredArgs); + } + } } else if ( /* * TODO: Handle method calls @@ -222,6 +234,13 @@ function writeDependencyToInstructions( */ break; } + if (path.property === 'current') { + /* + * Prune ref.current accesses. This may over-capture for non-ref values with + * a current property, but that's fine. + */ + break; + } const nextValue = createTemporaryPlace(env, GeneratedSource); nextValue.reactive = reactive; instructions.push({ diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md index 6e45941f24264..42dd999b3f6e8 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md @@ -4,6 +4,7 @@ ```javascript // @inferEffectDependencies import {useEffect, useRef} from 'react'; +import useEffectWrapper from 'useEffectWrapper'; const moduleNonReactive = 0; @@ -39,6 +40,10 @@ function Component({foo, bar}) { // No inferred dep array, the argument is not a lambda useEffect(f); + + useEffectWrapper(() => { + console.log(foo); + }); } ``` @@ -48,11 +53,12 @@ function Component({foo, bar}) { ```javascript import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies import { useEffect, useRef } from "react"; +import useEffectWrapper from "useEffectWrapper"; const moduleNonReactive = 0; function Component(t0) { - const $ = _c(12); + const $ = _c(14); const { foo, bar } = t0; const ref = useRef(0); @@ -125,6 +131,17 @@ function Component(t0) { const f = t5; useEffect(f); + let t6; + if ($[12] !== foo) { + t6 = () => { + console.log(foo); + }; + $[12] = foo; + $[13] = t6; + } else { + t6 = $[13]; + } + useEffectWrapper(t6, [foo]); } ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.js index 723ad6516565f..efdd4164789b4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.js @@ -1,5 +1,6 @@ // @inferEffectDependencies import {useEffect, useRef} from 'react'; +import useEffectWrapper from 'useEffectWrapper'; const moduleNonReactive = 0; @@ -35,4 +36,8 @@ function Component({foo, bar}) { // No inferred dep array, the argument is not a lambda useEffect(f); + + useEffectWrapper(() => { + console.log(foo); + }); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonreactive-ref-helper.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonreactive-ref-helper.expect.md new file mode 100644 index 0000000000000..bc5c3e4dd466f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonreactive-ref-helper.expect.md @@ -0,0 +1,89 @@ + +## Input + +```javascript +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; + +/** + * We never include a .current access in a dep array because it may be a ref access. + * This might over-capture objects that are not refs and happen to have fields named + * current, but that should be a rare case and the result would still be correct + * (assuming the effect is idempotent). In the worst case, you can always write a manual + * dep array. + */ +function RefsInEffects() { + const ref = useRefHelper(); + const wrapped = useDeeperRefHelper(); + useEffect(() => { + print(ref.current); + print(wrapped.foo.current); + }); +} + +function useRefHelper() { + return useRef(0); +} + +function useDeeperRefHelper() { + return {foo: useRefHelper()}; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies +import { useEffect } from "react"; +import { print } from "shared-runtime"; + +/** + * We never include a .current access in a dep array because it may be a ref access. + * This might over-capture objects that are not refs and happen to have fields named + * current, but that should be a rare case and the result would still be correct + * (assuming the effect is idempotent). In the worst case, you can always write a manual + * dep array. + */ +function RefsInEffects() { + const $ = _c(3); + const ref = useRefHelper(); + const wrapped = useDeeperRefHelper(); + let t0; + if ($[0] !== ref.current || $[1] !== wrapped.foo.current) { + t0 = () => { + print(ref.current); + print(wrapped.foo.current); + }; + $[0] = ref.current; + $[1] = wrapped.foo.current; + $[2] = t0; + } else { + t0 = $[2]; + } + useEffect(t0, [ref, wrapped.foo]); +} + +function useRefHelper() { + return useRef(0); +} + +function useDeeperRefHelper() { + const $ = _c(2); + const t0 = useRefHelper(); + let t1; + if ($[0] !== t0) { + t1 = { foo: t0 }; + $[0] = t0; + $[1] = t1; + } else { + t1 = $[1]; + } + return t1; +} + +``` + +### 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/nonreactive-ref-helper.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonreactive-ref-helper.js new file mode 100644 index 0000000000000..9a213fad5f447 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonreactive-ref-helper.js @@ -0,0 +1,27 @@ +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; + +/** + * We never include a .current access in a dep array because it may be a ref access. + * This might over-capture objects that are not refs and happen to have fields named + * current, but that should be a rare case and the result would still be correct + * (assuming the effect is idempotent). In the worst case, you can always write a manual + * dep array. + */ +function RefsInEffects() { + const ref = useRefHelper(); + const wrapped = useDeeperRefHelper(); + useEffect(() => { + print(ref.current); + print(wrapped.foo.current); + }); +} + +function useRefHelper() { + return useRef(0); +} + +function useDeeperRefHelper() { + return {foo: useRefHelper()}; +} diff --git a/compiler/packages/snap/src/compiler.ts b/compiler/packages/snap/src/compiler.ts index 95af40d62a880..d1212d21068c3 100644 --- a/compiler/packages/snap/src/compiler.ts +++ b/compiler/packages/snap/src/compiler.ts @@ -289,6 +289,8 @@ function getEvaluatorPresets( arg.value = './shared-runtime'; } else if (arg.value === 'ReactForgetFeatureFlag') { arg.value = './ReactForgetFeatureFlag'; + } else if (arg.value === 'useEffectWrapper') { + arg.value = './useEffectWrapper'; } } } diff --git a/compiler/packages/snap/src/sprout/useEffectWrapper.ts b/compiler/packages/snap/src/sprout/useEffectWrapper.ts new file mode 100644 index 0000000000000..b1589dac4072c --- /dev/null +++ b/compiler/packages/snap/src/sprout/useEffectWrapper.ts @@ -0,0 +1,18 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/* This file is used to test the effect auto-deps configuration, which + * allows you to specify functions that should have dependencies added to + * callsites. + */ +import {useEffect} from 'react'; + +export default function useEffectWrapper(f: () => void | (() => void)): void { + useEffect(() => { + f(); + }, [f]); +} diff --git a/compiler/packages/snap/src/types.d.ts b/compiler/packages/snap/src/types.d.ts index 40771d183c06b..b624caf19c548 100644 --- a/compiler/packages/snap/src/types.d.ts +++ b/compiler/packages/snap/src/types.d.ts @@ -6,14 +6,14 @@ */ // v0.17.1 -declare module "hermes-parser" { +declare module 'hermes-parser' { type HermesParserOptions = { allowReturnOutsideFunction?: boolean; babel?: boolean; - flow?: "all" | "detect"; + flow?: 'all' | 'detect'; enableExperimentalComponentSyntax?: boolean; sourceFilename?: string; - sourceType?: "module" | "script" | "unambiguous"; + sourceType?: 'module' | 'script' | 'unambiguous'; tokens?: boolean; }; export function parse(code: string, options: Partial);