From 1b1283ade72dfdfa8981b08cf4cc9669bbf905f0 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Tue, 3 Dec 2024 07:42:53 -0500 Subject: [PATCH] [compiler] Support default imports for autodep config (#31657) ## Summary Allows us to add deps for things like `import useWrapperEffect from 'useWrapperEffect'` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31657). * __->__ #31657 * #31652 --- .../src/HIR/Environment.ts | 9 +++++++++ .../src/Inference/InferEffectDependencies.ts | 12 ++++++++++++ .../infer-effect-dependencies.expect.md | 19 ++++++++++++++++++- .../compiler/infer-effect-dependencies.js | 5 +++++ compiler/packages/snap/src/compiler.ts | 2 ++ .../snap/src/sprout/useEffectWrapper.ts | 18 ++++++++++++++++++ compiler/packages/snap/src/types.d.ts | 6 +++--- 7 files changed, 67 insertions(+), 4 deletions(-) create mode 100644 compiler/packages/snap/src/sprout/useEffectWrapper.ts 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 c74ef64529a17..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 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/snap/src/compiler.ts b/compiler/packages/snap/src/compiler.ts index 68acdbc7900b7..1cb8fe48b9451 100644 --- a/compiler/packages/snap/src/compiler.ts +++ b/compiler/packages/snap/src/compiler.ts @@ -297,6 +297,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);