Repository: facebook/react
Pull request: [autodeps] Support namespaces


Summary: Summary: Correctly supports React.useEffect when React is imported as `import * as React from 'react'`
(as well as other namespaces as specified in the config).

Test Plan:
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32162).
* #32236
* __->__ #32162

Changed files:
filename: compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts
patch: @@ -49,6 +49,7 @@ export function inferEffectDependencies(fn: HIRFunction): void {
     );
   }
   const autodepFnLoads = new Map<IdentifierId, number>();
+  const autodepModuleLoads = new Map<IdentifierId, Map<string, number>>();
 
   const scopeInfos = new Map<
     ScopeId,
@@ -89,9 +90,34 @@ export function inferEffectDependencies(fn: HIRFunction): void {
           lvalue.identifier.id,
           instr as TInstruction<FunctionExpression>,
         );
+      } else if (value.kind === 'PropertyLoad') {
+        if (
+          typeof value.property === 'string' &&
+          autodepModuleLoads.has(value.object.identifier.id)
+        ) {
+          const moduleTargets = autodepModuleLoads.get(
+            value.object.identifier.id,
+          )!;
+          const propertyName = value.property;
+          const numRequiredArgs = moduleTargets.get(propertyName);
+          if (numRequiredArgs != null) {
+            autodepFnLoads.set(lvalue.identifier.id, numRequiredArgs);
+          }
+        }
       } else if (value.kind === 'LoadGlobal') {
         loadGlobals.add(lvalue.identifier.id);
 
+        /*
+         * TODO: Handle properties on default exports, like
+         * import React from 'react';
+         * React.useEffect(...);
+         */
+        if (value.binding.kind === 'ImportNamespace') {
+          const moduleTargets = autodepFnConfigs.get(value.binding.module);
+          if (moduleTargets != null) {
+            autodepModuleLoads.set(lvalue.identifier.id, moduleTargets);
+          }
+        }
         if (
           value.binding.kind === 'ImportSpecifier' ||
           value.binding.kind === 'ImportDefault'
@@ -109,84 +135,88 @@ export function inferEffectDependencies(fn: HIRFunction): void {
           }
         }
       } else if (
-        /*
-         * TODO: Handle method calls
-         */
-        value.kind === 'CallExpression' &&
-        autodepFnLoads.get(value.callee.identifier.id) === value.args.length &&
-        value.args[0].kind === 'Identifier'
+        value.kind === 'CallExpression' ||
+        value.kind === 'MethodCall'
       ) {
-        const effectDeps: Array<Place> = [];
-        const newInstructions: Array<Instruction> = [];
-        const deps: ArrayExpression = {
-          kind: 'ArrayExpression',
-          elements: effectDeps,
-          loc: GeneratedSource,
-        };
-        const depsPlace = createTemporaryPlace(fn.env, GeneratedSource);
-        depsPlace.effect = Effect.Read;
+        const callee =
+          value.kind === 'CallExpression' ? value.callee : value.property;
+        if (
+          value.args.length === autodepFnLoads.get(callee.identifier.id) &&
+          value.args[0].kind === 'Identifier'
+        ) {
+          // We have a useEffect call with no deps array, so we need to infer the deps
+          const effectDeps: Array<Place> = [];
+          const newInstructions: Array<Instruction> = [];
+          const deps: ArrayExpression = {
+            kind: 'ArrayExpression',
+            elements: effectDeps,
+            loc: GeneratedSource,
+          };
+          const depsPlace = createTemporaryPlace(fn.env, GeneratedSource);
+          depsPlace.effect = Effect.Read;
+
+          const fnExpr = fnExpressions.get(value.args[0].identifier.id);
+          if (fnExpr != null) {
+            // We have a function expression, so we can infer its dependencies
+            const scopeInfo =
+              fnExpr.lvalue.identifier.scope != null
+                ? scopeInfos.get(fnExpr.lvalue.identifier.scope.id)
+                : null;
+            CompilerError.invariant(scopeInfo != null, {
+              reason: 'Expected function expression scope to exist',
+              loc: value.loc,
+            });
+            if (scopeInfo.pruned || !scopeInfo.hasSingleInstr) {
+              /**
+               * TODO: retry pipeline that ensures effect function expressions
+               * are placed into their own scope
+               */
+              CompilerError.throwTodo({
+                reason:
+                  '[InferEffectDependencies] Expected effect function to have non-pruned scope and its scope to have exactly one instruction',
+                loc: fnExpr.loc,
+              });
+            }
 
-        const fnExpr = fnExpressions.get(value.args[0].identifier.id);
-        if (fnExpr != null) {
-          // We have a function expression, so we can infer its dependencies
-          const scopeInfo =
-            fnExpr.lvalue.identifier.scope != null
-              ? scopeInfos.get(fnExpr.lvalue.identifier.scope.id)
-              : null;
-          CompilerError.invariant(scopeInfo != null, {
-            reason: 'Expected function expression scope to exist',
-            loc: value.loc,
-          });
-          if (scopeInfo.pruned || !scopeInfo.hasSingleInstr) {
             /**
-             * TODO: retry pipeline that ensures effect function expressions
-             * are placed into their own scope
+             * Step 1: push dependencies to the effect deps array
+             *
+             * Note that it's invalid to prune non-reactive deps in this pass, see
+             * the `infer-effect-deps/pruned-nonreactive-obj` fixture for an
+             * explanation.
              */
-            CompilerError.throwTodo({
-              reason:
-                '[InferEffectDependencies] Expected effect function to have non-pruned scope and its scope to have exactly one instruction',
-              loc: fnExpr.loc,
+            for (const dep of scopeInfo.deps) {
+              const {place, instructions} = writeDependencyToInstructions(
+                dep,
+                reactiveIds.has(dep.identifier.id),
+                fn.env,
+                fnExpr.loc,
+              );
+              newInstructions.push(...instructions);
+              effectDeps.push(place);
+            }
+
+            newInstructions.push({
+              id: makeInstructionId(0),
+              loc: GeneratedSource,
+              lvalue: {...depsPlace, effect: Effect.Mutate},
+              value: deps,
             });
-          }
 
-          /**
-           * Step 1: push dependencies to the effect deps array
-           *
-           * Note that it's invalid to prune non-reactive deps in this pass, see
-           * the `infer-effect-deps/pruned-nonreactive-obj` fixture for an
-           * explanation.
-           */
-          for (const dep of scopeInfo.deps) {
-            const {place, instructions} = writeDependencyToInstructions(
-              dep,
-              reactiveIds.has(dep.identifier.id),
-              fn.env,
-              fnExpr.loc,
-            );
-            newInstructions.push(...instructions);
-            effectDeps.push(place);
+            // Step 2: push the inferred deps array as an argument of the useEffect
+            value.args.push({...depsPlace, effect: Effect.Freeze});
+            rewriteInstrs.set(instr.id, newInstructions);
+          } else if (loadGlobals.has(value.args[0].identifier.id)) {
+            // Global functions have no reactive dependencies, so we can insert an empty array
+            newInstructions.push({
+              id: makeInstructionId(0),
+              loc: GeneratedSource,
+              lvalue: {...depsPlace, effect: Effect.Mutate},
+              value: deps,
+            });
+            value.args.push({...depsPlace, effect: Effect.Freeze});
+            rewriteInstrs.set(instr.id, newInstructions);
           }
-
-          newInstructions.push({
-            id: makeInstructionId(0),
-            loc: GeneratedSource,
-            lvalue: {...depsPlace, effect: Effect.Mutate},
-            value: deps,
-          });
-
-          // Step 2: push the inferred deps array as an argument of the useEffect
-          value.args.push({...depsPlace, effect: Effect.Freeze});
-          rewriteInstrs.set(instr.id, newInstructions);
-        } else if (loadGlobals.has(value.args[0].identifier.id)) {
-          // Global functions have no reactive dependencies, so we can insert an empty array
-          newInstructions.push({
-            id: makeInstructionId(0),
-            loc: GeneratedSource,
-            lvalue: {...depsPlace, effect: Effect.Mutate},
-            value: deps,
-          });
-          value.args.push({...depsPlace, effect: Effect.Freeze});
-          rewriteInstrs.set(instr.id, newInstructions);
         }
       }
     }

filename: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/import-namespace-useEffect.expect.md
patch: @@ -0,0 +1,59 @@
+
+## Input
+
+```javascript
+// @inferEffectDependencies
+import * as React from 'react';
+import * as SharedRuntime from 'shared-runtime';
+
+function NonReactiveDepInEffect() {
+  const obj = makeObject_Primitives();
+  React.useEffect(() => print(obj));
+  SharedRuntime.useSpecialEffect(() => print(obj), [obj]);
+}
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies
+import * as React from "react";
+import * as SharedRuntime from "shared-runtime";
+
+function NonReactiveDepInEffect() {
+  const $ = _c(4);
+  let t0;
+  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
+    t0 = makeObject_Primitives();
+    $[0] = t0;
+  } else {
+    t0 = $[0];
+  }
+  const obj = t0;
+  let t1;
+  if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
+    t1 = () => print(obj);
+    $[1] = t1;
+  } else {
+    t1 = $[1];
+  }
+  React.useEffect(t1, [obj]);
+  let t2;
+  let t3;
+  if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
+    t2 = () => print(obj);
+    t3 = [obj];
+    $[2] = t2;
+    $[3] = t3;
+  } else {
+    t2 = $[2];
+    t3 = $[3];
+  }
+  SharedRuntime.useSpecialEffect(t2, t3, [obj]);
+}
+
+```
+      
+### Eval output
+(kind: exception) Fixture not implemented
\ No newline at end of file

filename: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/import-namespace-useEffect.js
patch: @@ -0,0 +1,9 @@
+// @inferEffectDependencies
+import * as React from 'react';
+import * as SharedRuntime from 'shared-runtime';
+
+function NonReactiveDepInEffect() {
+  const obj = makeObject_Primitives();
+  React.useEffect(() => print(obj));
+  SharedRuntime.useSpecialEffect(() => print(obj), [obj]);
+}

filename: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo.import-default-property-useEffect.expect.md
patch: @@ -3,11 +3,8 @@
 
 ```javascript
 // @inferEffectDependencies
-import * as React from 'react';
+import React from 'react';
 
-/**
- * TODO: recognize import namespace
- */
 function NonReactiveDepInEffect() {
   const obj = makeObject_Primitives();
   React.useEffect(() => print(obj));
@@ -19,11 +16,8 @@ function NonReactiveDepInEffect() {
 
 ```javascript
 import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies
-import * as React from "react";
+import React from "react";
 
-/**
- * TODO: recognize import namespace
- */
 function NonReactiveDepInEffect() {
   const $ = _c(2);
   let t0;

filename: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo.import-default-property-useEffect.js
patch: @@ -1,9 +1,6 @@
 // @inferEffectDependencies
-import * as React from 'react';
+import React from 'react';
 
-/**
- * TODO: recognize import namespace
- */
 function NonReactiveDepInEffect() {
   const obj = makeObject_Primitives();
   React.useEffect(() => print(obj));