[pull] main from facebook:main

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';

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({

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]);
 }
 
 ```

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

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

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

compiler/packages/eslint-plugin-react-compiler/package.json

@@ -13,7 +13,7 @@
   "dependencies": {
     "@babel/core": "^7.24.4",
     "@babel/parser": "^7.24.4",
-    "@babel/plugin-transform-private-methods": "^7.25.9",
+    "@babel/plugin-proposal-private-methods": "^7.18.6",
     "hermes-parser": "^0.25.1",
     "zod": "^3.22.4",
     "zod-validation-error": "^3.0.3"

compiler/packages/eslint-plugin-react-compiler/rollup.config.js

@@ -17,7 +17,7 @@ import banner2 from 'rollup-plugin-banner2';
 
 const NO_INLINE = new Set([
   '@babel/core',
-  '@babel/plugin-transform-private-methods',
+  '@babel/plugin-proposal-private-methods',
   'hermes-parser',
   'zod',
   'zod-validation-error',

compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts

@@ -7,7 +7,7 @@
 
 import {transformFromAstSync} from '@babel/core';
 // @ts-expect-error: no types available
-import PluginTransformPrivateMethods from '@babel/plugin-transform-private-methods';
+import PluginProposalPrivateMethods from '@babel/plugin-proposal-private-methods';
 import type {SourceLocation as BabelSourceLocation} from '@babel/types';
 import BabelPluginReactCompiler, {
   CompilerErrorDetailOptions,
@@ -282,7 +282,7 @@ const rule: Rule.RuleModule = {
           highlightCode: false,
           retainLines: true,
           plugins: [
-            [PluginTransformPrivateMethods, {loose: true}],
+            [PluginProposalPrivateMethods, {loose: true}],
             [BabelPluginReactCompiler, options],
           ],
           sourceType: 'module',

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

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

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<HermesParserOptions>);