@samitouri / QOS-React / commits / bdce84a539

[autodeps] Support namespaces (#32162)

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).

Jordan Brown committed Mar 3, 2025 at 15:20 UTC bdce84a53960c267f92a08241dc7b1924f30be55
5 files changed +172 -83
compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts
+101 -71
@@ -49,6 +49,7 @@ export function inferEffectDependencies(fn: HIRFunction): void {
49 );
50 }
51 const autodepFnLoads = new Map<IdentifierId, number>();
52 + const autodepModuleLoads = new Map<IdentifierId, Map<string, number>>();
53
54 const scopeInfos = new Map<
55 ScopeId,
@@ -89,9 +90,34 @@ export function inferEffectDependencies(fn: HIRFunction): void {
90 lvalue.identifier.id,
91 instr as TInstruction<FunctionExpression>,
92 );
93 + } else if (value.kind === 'PropertyLoad') {
94 + if (
95 + typeof value.property === 'string' &&
96 + autodepModuleLoads.has(value.object.identifier.id)
97 + ) {
98 + const moduleTargets = autodepModuleLoads.get(
99 + value.object.identifier.id,
100 + )!;
101 + const propertyName = value.property;
102 + const numRequiredArgs = moduleTargets.get(propertyName);
103 + if (numRequiredArgs != null) {
104 + autodepFnLoads.set(lvalue.identifier.id, numRequiredArgs);
105 + }
106 + }
107 } else if (value.kind === 'LoadGlobal') {
108 loadGlobals.add(lvalue.identifier.id);
109
110 + /*
111 + * TODO: Handle properties on default exports, like
112 + * import React from 'react';
113 + * React.useEffect(...);
114 + */
115 + if (value.binding.kind === 'ImportNamespace') {
116 + const moduleTargets = autodepFnConfigs.get(value.binding.module);
117 + if (moduleTargets != null) {
118 + autodepModuleLoads.set(lvalue.identifier.id, moduleTargets);
119 + }
120 + }
121 if (
122 value.binding.kind === 'ImportSpecifier' ||
123 value.binding.kind === 'ImportDefault'
@@ -109,84 +135,88 @@ export function inferEffectDependencies(fn: HIRFunction): void {
135 }
136 }
137 } else if (
112 - /*
113 - * TODO: Handle method calls
114 - */
115 - value.kind === 'CallExpression' &&
116 - autodepFnLoads.get(value.callee.identifier.id) === value.args.length &&
117 - value.args[0].kind === 'Identifier'
138 + value.kind === 'CallExpression' ||
139 + value.kind === 'MethodCall'
140 ) {
119 - const effectDeps: Array<Place> = [];
120 - const newInstructions: Array<Instruction> = [];
121 - const deps: ArrayExpression = {
122 - kind: 'ArrayExpression',
123 - elements: effectDeps,
124 - loc: GeneratedSource,
125 - };
126 - const depsPlace = createTemporaryPlace(fn.env, GeneratedSource);
127 - depsPlace.effect = Effect.Read;
141 + const callee =
142 + value.kind === 'CallExpression' ? value.callee : value.property;
143 + if (
144 + value.args.length === autodepFnLoads.get(callee.identifier.id) &&
145 + value.args[0].kind === 'Identifier'
146 + ) {
147 + // We have a useEffect call with no deps array, so we need to infer the deps
148 + const effectDeps: Array<Place> = [];
149 + const newInstructions: Array<Instruction> = [];
150 + const deps: ArrayExpression = {
151 + kind: 'ArrayExpression',
152 + elements: effectDeps,
153 + loc: GeneratedSource,
154 + };
155 + const depsPlace = createTemporaryPlace(fn.env, GeneratedSource);
156 + depsPlace.effect = Effect.Read;
157 +
158 + const fnExpr = fnExpressions.get(value.args[0].identifier.id);
159 + if (fnExpr != null) {
160 + // We have a function expression, so we can infer its dependencies
161 + const scopeInfo =
162 + fnExpr.lvalue.identifier.scope != null
163 + ? scopeInfos.get(fnExpr.lvalue.identifier.scope.id)
164 + : null;
165 + CompilerError.invariant(scopeInfo != null, {
166 + reason: 'Expected function expression scope to exist',
167 + loc: value.loc,
168 + });
169 + if (scopeInfo.pruned || !scopeInfo.hasSingleInstr) {
170 + /**
171 + * TODO: retry pipeline that ensures effect function expressions
172 + * are placed into their own scope
173 + */
174 + CompilerError.throwTodo({
175 + reason:
176 + '[InferEffectDependencies] Expected effect function to have non-pruned scope and its scope to have exactly one instruction',
177 + loc: fnExpr.loc,
178 + });
179 + }
180
129 - const fnExpr = fnExpressions.get(value.args[0].identifier.id);
130 - if (fnExpr != null) {
131 - // We have a function expression, so we can infer its dependencies
132 - const scopeInfo =
133 - fnExpr.lvalue.identifier.scope != null
134 - ? scopeInfos.get(fnExpr.lvalue.identifier.scope.id)
135 - : null;
136 - CompilerError.invariant(scopeInfo != null, {
137 - reason: 'Expected function expression scope to exist',
138 - loc: value.loc,
139 - });
140 - if (scopeInfo.pruned || !scopeInfo.hasSingleInstr) {
181 /**
142 - * TODO: retry pipeline that ensures effect function expressions
143 - * are placed into their own scope
182 + * Step 1: push dependencies to the effect deps array
183 + *
184 + * Note that it's invalid to prune non-reactive deps in this pass, see
185 + * the `infer-effect-deps/pruned-nonreactive-obj` fixture for an
186 + * explanation.
187 */
145 - CompilerError.throwTodo({
146 - reason:
147 - '[InferEffectDependencies] Expected effect function to have non-pruned scope and its scope to have exactly one instruction',
148 - loc: fnExpr.loc,
188 + for (const dep of scopeInfo.deps) {
189 + const {place, instructions} = writeDependencyToInstructions(
190 + dep,
191 + reactiveIds.has(dep.identifier.id),
192 + fn.env,
193 + fnExpr.loc,
194 + );
195 + newInstructions.push(...instructions);
196 + effectDeps.push(place);
197 + }
198 +
199 + newInstructions.push({
200 + id: makeInstructionId(0),
201 + loc: GeneratedSource,
202 + lvalue: {...depsPlace, effect: Effect.Mutate},
203 + value: deps,
204 });
150 - }
205
152 - /**
153 - * Step 1: push dependencies to the effect deps array
154 - *
155 - * Note that it's invalid to prune non-reactive deps in this pass, see
156 - * the `infer-effect-deps/pruned-nonreactive-obj` fixture for an
157 - * explanation.
158 - */
159 - for (const dep of scopeInfo.deps) {
160 - const {place, instructions} = writeDependencyToInstructions(
161 - dep,
162 - reactiveIds.has(dep.identifier.id),
163 - fn.env,
164 - fnExpr.loc,
165 - );
166 - newInstructions.push(...instructions);
167 - effectDeps.push(place);
206 + // Step 2: push the inferred deps array as an argument of the useEffect
207 + value.args.push({...depsPlace, effect: Effect.Freeze});
208 + rewriteInstrs.set(instr.id, newInstructions);
209 + } else if (loadGlobals.has(value.args[0].identifier.id)) {
210 + // Global functions have no reactive dependencies, so we can insert an empty array
211 + newInstructions.push({
212 + id: makeInstructionId(0),
213 + loc: GeneratedSource,
214 + lvalue: {...depsPlace, effect: Effect.Mutate},
215 + value: deps,
216 + });
217 + value.args.push({...depsPlace, effect: Effect.Freeze});
218 + rewriteInstrs.set(instr.id, newInstructions);
219 }
169 -
170 - newInstructions.push({
171 - id: makeInstructionId(0),
172 - loc: GeneratedSource,
173 - lvalue: {...depsPlace, effect: Effect.Mutate},
174 - value: deps,
175 - });
176 -
177 - // Step 2: push the inferred deps array as an argument of the useEffect
178 - value.args.push({...depsPlace, effect: Effect.Freeze});
179 - rewriteInstrs.set(instr.id, newInstructions);
180 - } else if (loadGlobals.has(value.args[0].identifier.id)) {
181 - // Global functions have no reactive dependencies, so we can insert an empty array
182 - newInstructions.push({
183 - id: makeInstructionId(0),
184 - loc: GeneratedSource,
185 - lvalue: {...depsPlace, effect: Effect.Mutate},
186 - value: deps,
187 - });
188 - value.args.push({...depsPlace, effect: Effect.Freeze});
189 - rewriteInstrs.set(instr.id, newInstructions);
220 }
221 }
222 }
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/import-namespace-useEffect.expect.md new
+59
@@ -0,0 +1,59 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @inferEffectDependencies
6 +import * as React from 'react';
7 +import * as SharedRuntime from 'shared-runtime';
8 +
9 +function NonReactiveDepInEffect() {
10 + const obj = makeObject_Primitives();
11 + React.useEffect(() => print(obj));
12 + SharedRuntime.useSpecialEffect(() => print(obj), [obj]);
13 +}
14 +
15 +```
16 +
17 +## Code
18 +
19 +```javascript
20 +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies
21 +import * as React from "react";
22 +import * as SharedRuntime from "shared-runtime";
23 +
24 +function NonReactiveDepInEffect() {
25 + const $ = _c(4);
26 + let t0;
27 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
28 + t0 = makeObject_Primitives();
29 + $[0] = t0;
30 + } else {
31 + t0 = $[0];
32 + }
33 + const obj = t0;
34 + let t1;
35 + if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
36 + t1 = () => print(obj);
37 + $[1] = t1;
38 + } else {
39 + t1 = $[1];
40 + }
41 + React.useEffect(t1, [obj]);
42 + let t2;
43 + let t3;
44 + if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
45 + t2 = () => print(obj);
46 + t3 = [obj];
47 + $[2] = t2;
48 + $[3] = t3;
49 + } else {
50 + t2 = $[2];
51 + t3 = $[3];
52 + }
53 + SharedRuntime.useSpecialEffect(t2, t3, [obj]);
54 +}
55 +
56 +```
57 +
58 +### Eval output
59 +(kind: exception) Fixture not implemented
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/import-namespace-useEffect.js new
+9
@@ -0,0 +1,9 @@
1 +// @inferEffectDependencies
2 +import * as React from 'react';
3 +import * as SharedRuntime from 'shared-runtime';
4 +
5 +function NonReactiveDepInEffect() {
6 + const obj = makeObject_Primitives();
7 + React.useEffect(() => print(obj));
8 + SharedRuntime.useSpecialEffect(() => print(obj), [obj]);
9 +}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo.import-default-property-useEffect.expect.md renamed
+2 -8
@@ -3,11 +3,8 @@
3
4 ```javascript
5 // @inferEffectDependencies
6 -import * as React from 'react';
6 +import React from 'react';
7
8 -/**
9 - * TODO: recognize import namespace
10 - */
8 function NonReactiveDepInEffect() {
9 const obj = makeObject_Primitives();
10 React.useEffect(() => print(obj));
@@ -19,11 +16,8 @@ function NonReactiveDepInEffect() {
16
17 ```javascript
18 import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies
22 -import * as React from "react";
19 +import React from "react";
20
24 -/**
25 - * TODO: recognize import namespace
26 - */
21 function NonReactiveDepInEffect() {
22 const $ = _c(2);
23 let t0;
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo.import-default-property-useEffect.js renamed
+1 -4
@@ -1,9 +1,6 @@
1 // @inferEffectDependencies
2 -import * as React from 'react';
2 +import React from 'react';
3
4 -/**
5 - * TODO: recognize import namespace
6 - */
4 function NonReactiveDepInEffect() {
5 const obj = makeObject_Primitives();
6 React.useEffect(() => print(obj));