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 8c9823765bbb6..7e317e6907961 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts @@ -49,6 +49,7 @@ export function inferEffectDependencies(fn: HIRFunction): void { ); } const autodepFnLoads = new Map(); + const autodepModuleLoads = new Map>(); const scopeInfos = new Map< ScopeId, @@ -89,9 +90,34 @@ export function inferEffectDependencies(fn: HIRFunction): void { lvalue.identifier.id, instr as TInstruction, ); + } 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 = []; - const newInstructions: Array = []; - 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 = []; + const newInstructions: Array = []; + 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); } } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/import-namespace-useEffect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/import-namespace-useEffect.expect.md new file mode 100644 index 0000000000000..3ae6766fc7e29 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/import-namespace-useEffect.expect.md @@ -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 diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/import-namespace-useEffect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/import-namespace-useEffect.js new file mode 100644 index 0000000000000..e440cfcc2302a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/import-namespace-useEffect.js @@ -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]); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo-import-namespace-useEffect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo.import-default-property-useEffect.expect.md similarity index 82% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo-import-namespace-useEffect.expect.md rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo.import-default-property-useEffect.expect.md index 6302067a5a5eb..a5a576c83067a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo-import-namespace-useEffect.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo.import-default-property-useEffect.expect.md @@ -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; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo-import-namespace-useEffect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo.import-default-property-useEffect.js similarity index 65% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo-import-namespace-useEffect.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo.import-default-property-useEffect.js index 4c9eec898614f..0dbae754ecf76 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo-import-namespace-useEffect.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo.import-default-property-useEffect.js @@ -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));