[pull] main from facebook:main

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/ValidateNoUntransformedReferences.ts

@@ -35,8 +35,41 @@ function throwInvalidReact(
   });
   CompilerError.throw(detail);
 }
+
+function isAutodepsSigil(
+  arg: NodePath<t.ArgumentPlaceholder | t.SpreadElement | t.Expression>,
+): boolean {
+  // Check for AUTODEPS identifier imported from React
+  if (arg.isIdentifier() && arg.node.name === 'AUTODEPS') {
+    const binding = arg.scope.getBinding(arg.node.name);
+    if (binding && binding.path.isImportSpecifier()) {
+      const importSpecifier = binding.path.node as t.ImportSpecifier;
+      if (importSpecifier.imported.type === 'Identifier') {
+        return (importSpecifier.imported as t.Identifier).name === 'AUTODEPS';
+      }
+    }
+    return false;
+  }
+
+  // Check for React.AUTODEPS member expression
+  if (arg.isMemberExpression() && !arg.node.computed) {
+    const object = arg.get('object');
+    const property = arg.get('property');
+
+    if (
+      object.isIdentifier() &&
+      object.node.name === 'React' &&
+      property.isIdentifier() &&
+      property.node.name === 'AUTODEPS'
+    ) {
+      return true;
+    }
+  }
+
+  return false;
+}
 function assertValidEffectImportReference(
-  numArgs: number,
+  autodepsIndex: number,
   paths: Array<NodePath<t.Node>>,
   context: TraversalState,
 ): void {
@@ -49,11 +82,10 @@ function assertValidEffectImportReference(
         maybeCalleeLoc != null &&
         context.inferredEffectLocations.has(maybeCalleeLoc);
       /**
-       * Only error on untransformed references of the form `useMyEffect(...)`
-       * or `moduleNamespace.useMyEffect(...)`, with matching argument counts.
-       * TODO: do we also want a mode to also hard error on non-call references?
+       * Error on effect calls that still have AUTODEPS in their args
        */
-      if (args.length === numArgs && !hasInferredEffect) {
+      const hasAutodepsArg = args.some(isAutodepsSigil);
+      if (hasAutodepsArg && !hasInferredEffect) {
         const maybeErrorDiagnostic = matchCompilerDiagnostic(
           path,
           context.transformErrors,
@@ -128,12 +160,12 @@ export default function validateNoUntransformedReferences(
   if (env.inferEffectDependencies) {
     for (const {
       function: {source, importSpecifierName},
-      numRequiredArgs,
+      autodepsIndex,
     } of env.inferEffectDependencies) {
       const module = getOrInsertWith(moduleLoadChecks, source, () => new Map());
       module.set(
         importSpecifierName,
-        assertValidEffectImportReference.bind(null, numRequiredArgs),
+        assertValidEffectImportReference.bind(null, autodepsIndex),
       );
     }
   }

compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts

@@ -265,21 +265,19 @@ export const EnvironmentConfigSchema = z.object({
    *   {
    *     module: 'react',
    *     imported: 'useEffect',
-   *     numRequiredArgs: 1,
+   *     autodepsIndex: 1,
    *   },{
    *     module: 'MyExperimentalEffectHooks',
    *     imported: 'useExperimentalEffect',
-   *     numRequiredArgs: 2,
+   *     autodepsIndex: 2,
    *   },
    * ]
    * would insert dependencies for calls of `useEffect` imported from `react` and calls of
    * useExperimentalEffect` from `MyExperimentalEffectHooks`.
    *
-   * `numRequiredArgs` tells the compiler the amount of arguments required to append a dependency
-   *  array to the end of the call. With the configuration above, we'd insert dependencies for
-   *  `useEffect` if it is only given a single argument and it would be appended to the argument list.
-   *
-   * numRequiredArgs must always be greater than 0, otherwise there is no function to analyze for dependencies
+   * `autodepsIndex` tells the compiler which index we expect the AUTODEPS to appear in.
+   *  With the configuration above, we'd insert dependencies for `useEffect` if it has two
+   *  arguments, and the second is AUTODEPS.
    *
    * Still experimental.
    */
@@ -288,7 +286,7 @@ export const EnvironmentConfigSchema = z.object({
       z.array(
         z.object({
           function: ExternalFunctionSchema,
-          numRequiredArgs: z.number().min(1, 'numRequiredArgs must be > 0'),
+          autodepsIndex: z.number().min(1, 'autodepsIndex must be > 0'),
         }),
       ),
     )

compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts

@@ -79,7 +79,7 @@ export function inferEffectDependencies(fn: HIRFunction): void {
     );
     moduleTargets.set(
       effectTarget.function.importSpecifierName,
-      effectTarget.numRequiredArgs,
+      effectTarget.autodepsIndex,
     );
   }
   const autodepFnLoads = new Map<IdentifierId, number>();
@@ -177,9 +177,14 @@ export function inferEffectDependencies(fn: HIRFunction): void {
             arg.identifier.type.kind === 'Object' &&
             arg.identifier.type.shapeId === BuiltInAutodepsId,
         );
+        const autodepsArgExpectedIndex = autodepFnLoads.get(
+          callee.identifier.id,
+        );
+
         if (
-          value.args.length > 1 &&
-          autodepsArgIndex > 0 &&
+          value.args.length > 0 &&
+          autodepsArgExpectedIndex != null &&
+          autodepsArgIndex === autodepsArgExpectedIndex &&
           autodepFnLoads.has(callee.identifier.id) &&
           value.args[0].kind === 'Identifier'
         ) {

compiler/packages/babel-plugin-react-compiler/src/Utils/TestUtils.ts

@@ -75,21 +75,21 @@ const testComplexConfigDefaults: PartialEnvironmentConfig = {
         source: 'react',
         importSpecifierName: 'useEffect',
       },
-      numRequiredArgs: 1,
+      autodepsIndex: 1,
     },
     {
       function: {
         source: 'shared-runtime',
         importSpecifierName: 'useSpecialEffect',
       },
-      numRequiredArgs: 2,
+      autodepsIndex: 2,
     },
     {
       function: {
         source: 'useEffectWrapper',
         importSpecifierName: 'default',
       },
-      numRequiredArgs: 1,
+      autodepsIndex: 1,
     },
   ],
 };

compiler/packages/babel-plugin-react-compiler/src/__tests__/envConfig-test.ts

@@ -33,12 +33,12 @@ describe('parseConfigPragma()', () => {
               source: 'react',
               importSpecifierName: 'useEffect',
             },
-            numRequiredArgs: 0,
+            autodepsIndex: 0,
           },
         ],
       } as any);
     }).toThrowErrorMatchingInlineSnapshot(
-      `"InvalidConfig: Could not validate environment config. Update React Compiler config to fix the error. Validation error: numRequiredArgs must be > 0 at "inferEffectDependencies[0].numRequiredArgs""`,
+      `"InvalidConfig: Could not validate environment config. Update React Compiler config to fix the error. Validation error: autodepsIndex must be > 0 at "inferEffectDependencies[0].autodepsIndex""`,
     );
   });
 

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/gating/error.dynamic-gating-invalid-identifier-nopanic-required-feature.expect.md

@@ -3,13 +3,13 @@
 
 ```javascript
 // @dynamicGating:{"source":"shared-runtime"} @panicThreshold:"none" @inferEffectDependencies
-import {useEffect} from 'react';
+import {useEffect, AUTODEPS} from 'react';
 import {print} from 'shared-runtime';
 
 function ReactiveVariable({propVal}) {
   'use memo if(invalid identifier)';
   const arr = [propVal];
-  useEffect(() => print(arr));
+  useEffect(() => print(arr), AUTODEPS);
 }
 
 export const FIXTURE_ENTRYPOINT = {
@@ -25,8 +25,8 @@ export const FIXTURE_ENTRYPOINT = {
 ```
    6 |   'use memo if(invalid identifier)';
    7 |   const arr = [propVal];
->  8 |   useEffect(() => print(arr));
-     |   ^^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: [InferEffectDependencies] React Compiler is unable to infer dependencies of this effect. This will break your build! To resolve, either pass your own dependency array or fix reported compiler bailout diagnostics. (8:8)
+>  8 |   useEffect(() => print(arr), AUTODEPS);
+     |   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: [InferEffectDependencies] React Compiler is unable to infer dependencies of this effect. This will break your build! To resolve, either pass your own dependency array or fix reported compiler bailout diagnostics. (8:8)
    9 | }
   10 |
   11 | export const FIXTURE_ENTRYPOINT = {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/gating/error.dynamic-gating-invalid-identifier-nopanic-required-feature.js

@@ -1,11 +1,11 @@
 // @dynamicGating:{"source":"shared-runtime"} @panicThreshold:"none" @inferEffectDependencies
-import {useEffect} from 'react';
+import {useEffect, AUTODEPS} from 'react';
 import {print} from 'shared-runtime';
 
 function ReactiveVariable({propVal}) {
   'use memo if(invalid identifier)';
   const arr = [propVal];
-  useEffect(() => print(arr));
+  useEffect(() => print(arr), AUTODEPS);
 }
 
 export const FIXTURE_ENTRYPOINT = {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn-default-import.expect.md

@@ -4,9 +4,10 @@
 ```javascript
 // @inferEffectDependencies @compilationMode:"infer" @panicThreshold:"none"
 import useMyEffect from 'useEffectWrapper';
+import {AUTODEPS} from 'react';
 
 function nonReactFn(arg) {
-  useMyEffect(() => [1, 2, arg]);
+  useMyEffect(() => [1, 2, arg], AUTODEPS);
 }
 
 ```
@@ -15,12 +16,12 @@ function nonReactFn(arg) {
 ## Error
 
 ```
-  3 |
-  4 | function nonReactFn(arg) {
-> 5 |   useMyEffect(() => [1, 2, arg]);
-    |   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: [InferEffectDependencies] React Compiler is unable to infer dependencies of this effect. This will break your build! To resolve, either pass your own dependency array or fix reported compiler bailout diagnostics. (5:5)
-  6 | }
-  7 |
+  4 |
+  5 | function nonReactFn(arg) {
+> 6 |   useMyEffect(() => [1, 2, arg], AUTODEPS);
+    |   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: [InferEffectDependencies] React Compiler is unable to infer dependencies of this effect. This will break your build! To resolve, either pass your own dependency array or fix reported compiler bailout diagnostics. (6:6)
+  7 | }
+  8 |
 ```
 
 

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn-default-import.js

@@ -1,6 +1,7 @@
 // @inferEffectDependencies @compilationMode:"infer" @panicThreshold:"none"
 import useMyEffect from 'useEffectWrapper';
+import {AUTODEPS} from 'react';
 
 function nonReactFn(arg) {
-  useMyEffect(() => [1, 2, arg]);
+  useMyEffect(() => [1, 2, arg], AUTODEPS);
 }

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn.expect.md

@@ -3,10 +3,10 @@
 
 ```javascript
 // @inferEffectDependencies @compilationMode:"infer" @panicThreshold:"none"
-import {useEffect} from 'react';
+import {useEffect, AUTODEPS} from 'react';
 
 function nonReactFn(arg) {
-  useEffect(() => [1, 2, arg]);
+  useEffect(() => [1, 2, arg], AUTODEPS);
 }
 
 ```
@@ -17,8 +17,8 @@ function nonReactFn(arg) {
 ```
   3 |
   4 | function nonReactFn(arg) {
-> 5 |   useEffect(() => [1, 2, arg]);
-    |   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: [InferEffectDependencies] React Compiler is unable to infer dependencies of this effect. This will break your build! To resolve, either pass your own dependency array or fix reported compiler bailout diagnostics. (5:5)
+> 5 |   useEffect(() => [1, 2, arg], AUTODEPS);
+    |   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: [InferEffectDependencies] React Compiler is unable to infer dependencies of this effect. This will break your build! To resolve, either pass your own dependency array or fix reported compiler bailout diagnostics. (5:5)
   6 | }
   7 |
 ```

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn.js

@@ -1,6 +1,6 @@
 // @inferEffectDependencies @compilationMode:"infer" @panicThreshold:"none"
-import {useEffect} from 'react';
+import {useEffect, AUTODEPS} from 'react';
 
 function nonReactFn(arg) {
-  useEffect(() => [1, 2, arg]);
+  useEffect(() => [1, 2, arg], AUTODEPS);
 }

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.non-inlined-effect-fn.expect.md

@@ -3,7 +3,7 @@
 
 ```javascript
 // @inferEffectDependencies @panicThreshold:"none"
-import {useEffect} from 'react';
+import {useEffect, AUTODEPS} from 'react';
 
 /**
  * Error on non-inlined effect functions:
@@ -21,7 +21,7 @@ function Component({foo}) {
   }
 
   // No inferred dep array, the argument is not a lambda
-  useEffect(f);
+  useEffect(f, AUTODEPS);
 }
 
 ```
@@ -32,8 +32,8 @@ function Component({foo}) {
 ```
   18 |
   19 |   // No inferred dep array, the argument is not a lambda
-> 20 |   useEffect(f);
-     |   ^^^^^^^^^^^^ InvalidReact: [InferEffectDependencies] React Compiler is unable to infer dependencies of this effect. This will break your build! To resolve, either pass your own dependency array or fix reported compiler bailout diagnostics. (20:20)
+> 20 |   useEffect(f, AUTODEPS);
+     |   ^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: [InferEffectDependencies] React Compiler is unable to infer dependencies of this effect. This will break your build! To resolve, either pass your own dependency array or fix reported compiler bailout diagnostics. (20:20)
   21 | }
   22 |
 ```

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.non-inlined-effect-fn.js

@@ -1,5 +1,5 @@
 // @inferEffectDependencies @panicThreshold:"none"
-import {useEffect} from 'react';
+import {useEffect, AUTODEPS} from 'react';
 
 /**
  * Error on non-inlined effect functions:
@@ -17,5 +17,5 @@ function Component({foo}) {
   }
 
   // No inferred dep array, the argument is not a lambda
-  useEffect(f);
+  useEffect(f, AUTODEPS);
 }

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-dynamic-gating.expect.md

@@ -5,6 +5,7 @@
 // @dynamicGating:{"source":"shared-runtime"} @inferEffectDependencies @panicThreshold:"none"
 
 import useEffectWrapper from 'useEffectWrapper';
+import {AUTODEPS} from 'react';
 
 /**
  * TODO: run the non-forget enabled version through the effect inference
@@ -13,7 +14,7 @@ import useEffectWrapper from 'useEffectWrapper';
 function Component({foo}) {
   'use memo if(getTrue)';
   const arr = [];
-  useEffectWrapper(() => arr.push(foo));
+  useEffectWrapper(() => arr.push(foo), AUTODEPS);
   arr.push(2);
   return arr;
 }
@@ -30,13 +31,13 @@ export const FIXTURE_ENTRYPOINT = {
 ## Error
 
 ```
-  10 |   'use memo if(getTrue)';
-  11 |   const arr = [];
-> 12 |   useEffectWrapper(() => arr.push(foo));
-     |   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: [InferEffectDependencies] React Compiler is unable to infer dependencies of this effect. This will break your build! To resolve, either pass your own dependency array or fix reported compiler bailout diagnostics. (12:12)
-  13 |   arr.push(2);
-  14 |   return arr;
-  15 | }
+  11 |   'use memo if(getTrue)';
+  12 |   const arr = [];
+> 13 |   useEffectWrapper(() => arr.push(foo), AUTODEPS);
+     |   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: [InferEffectDependencies] React Compiler is unable to infer dependencies of this effect. This will break your build! To resolve, either pass your own dependency array or fix reported compiler bailout diagnostics. (13:13)
+  14 |   arr.push(2);
+  15 |   return arr;
+  16 | }
 ```
 
 

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-dynamic-gating.js

@@ -1,6 +1,7 @@
 // @dynamicGating:{"source":"shared-runtime"} @inferEffectDependencies @panicThreshold:"none"
 
 import useEffectWrapper from 'useEffectWrapper';
+import {AUTODEPS} from 'react';
 
 /**
  * TODO: run the non-forget enabled version through the effect inference
@@ -9,7 +10,7 @@ import useEffectWrapper from 'useEffectWrapper';
 function Component({foo}) {
   'use memo if(getTrue)';
   const arr = [];
-  useEffectWrapper(() => arr.push(foo));
+  useEffectWrapper(() => arr.push(foo), AUTODEPS);
   arr.push(2);
   return arr;
 }