From 2a9f4c04e54294b668e0a2ae11c1930c2e57b248 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 22 Nov 2024 17:19:20 -0500 Subject: [PATCH] [compiler] Infer deps configuration (#31616) Adds a way to configure how we insert deps for experimental purposes. ``` [ { module: 'react', imported: 'useEffect', numRequiredArgs: 1, }, { module: 'MyExperimentalEffectHooks', imported: 'useExperimentalEffect', numRequiredArgs: 2, }, ] ``` would insert dependencies for calls of `useEffect` imported from `react` if they have 1 argument and calls of useExperimentalEffect` from `MyExperimentalEffectHooks` if they have 2 arguments. The pushed dep array is appended to the arg list. --- .../src/Entrypoint/Pipeline.ts | 2 +- .../src/HIR/Environment.ts | 51 +++++++++++++++- .../src/Inference/InferEffectDependencies.ts | 46 ++++++++++---- .../infer-deps-custom-config.expect.md | 61 +++++++++++++++++++ .../compiler/infer-deps-custom-config.js | 9 +++ .../infer-effect-dependencies.expect.md | 4 ++ .../compiler/infer-effect-dependencies.js | 2 + compiler/packages/snap/src/compiler.ts | 6 -- .../snap/src/sprout/shared-runtime.ts | 9 +++ 9 files changed, 168 insertions(+), 22 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-deps-custom-config.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-deps-custom-config.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts index bbd076e46a914..90921454c864f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -356,7 +356,7 @@ function* runWithEnvironment( }); if (env.config.inferEffectDependencies) { - inferEffectDependencies(env, hir); + inferEffectDependencies(hir); } if (env.config.inlineJsxTransform) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index 432eaf96ff99b..90f53d495b5a3 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -242,9 +242,40 @@ const EnvironmentConfigSchema = z.object({ enableOptionalDependencies: z.boolean().default(true), /** - * Enables inference and auto-insertion of effect dependencies. Still experimental. + * Enables inference and auto-insertion of effect dependencies. Takes in an array of + * configurable module and import pairs to allow for user-land experimentation. For example, + * [ + * { + * module: 'react', + * imported: 'useEffect', + * numRequiredArgs: 1, + * },{ + * module: 'MyExperimentalEffectHooks', + * imported: 'useExperimentalEffect', + * numRequiredArgs: 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 + * + * Still experimental. */ - inferEffectDependencies: z.boolean().default(false), + inferEffectDependencies: z + .nullable( + z.array( + z.object({ + function: ExternalFunctionSchema, + numRequiredArgs: z.number(), + }), + ), + ) + .default(null), /** * Enables inlining ReactElement object literals in place of JSX @@ -614,6 +645,22 @@ const testComplexConfigDefaults: PartialEnvironmentConfig = { source: 'react-compiler-runtime', importSpecifierName: 'useContext_withSelector', }, + inferEffectDependencies: [ + { + function: { + source: 'react', + importSpecifierName: 'useEffect', + }, + numRequiredArgs: 1, + }, + { + function: { + source: 'shared-runtime', + importSpecifierName: 'useSpecialEffect', + }, + numRequiredArgs: 2, + }, + ], }; /** 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 9dc7dff78a6af..9fb91a4ac829b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts @@ -8,7 +8,6 @@ import { HIRFunction, IdentifierId, Instruction, - isUseEffectHookType, makeInstructionId, TInstruction, InstructionId, @@ -23,20 +22,33 @@ import { markInstructionIds, } from '../HIR/HIRBuilder'; import {eachInstructionOperand, eachTerminalOperand} from '../HIR/visitors'; +import {getOrInsertWith} from '../Utils/utils'; /** * Infers reactive dependencies captured by useEffect lambdas and adds them as * a second argument to the useEffect call if no dependency array is provided. */ -export function inferEffectDependencies( - env: Environment, - fn: HIRFunction, -): void { +export function inferEffectDependencies(fn: HIRFunction): void { let hasRewrite = false; const fnExpressions = new Map< IdentifierId, TInstruction >(); + + const autodepFnConfigs = new Map>(); + for (const effectTarget of fn.env.config.inferEffectDependencies!) { + const moduleTargets = getOrInsertWith( + autodepFnConfigs, + effectTarget.function.source, + () => new Map(), + ); + moduleTargets.set( + effectTarget.function.importSpecifierName, + effectTarget.numRequiredArgs, + ); + } + const autodepFnLoads = new Map(); + const scopeInfos = new Map< ScopeId, {pruned: boolean; deps: ReactiveScopeDependencies; hasSingleInstr: boolean} @@ -74,15 +86,23 @@ export function inferEffectDependencies( lvalue.identifier.id, instr as TInstruction, ); + } else if ( + value.kind === 'LoadGlobal' && + value.binding.kind === 'ImportSpecifier' + ) { + const moduleTargets = autodepFnConfigs.get(value.binding.module); + if (moduleTargets != null) { + const numRequiredArgs = moduleTargets.get(value.binding.imported); + if (numRequiredArgs != null) { + autodepFnLoads.set(lvalue.identifier.id, numRequiredArgs); + } + } } else if ( /* - * This check is not final. Right now we only look for useEffects without a dependency array. - * This is likely not how we will ship this feature, but it is good enough for us to make progress - * on the implementation and test it. + * TODO: Handle method calls */ value.kind === 'CallExpression' && - isUseEffectHookType(value.callee.identifier) && - value.args.length === 1 && + autodepFnLoads.get(value.callee.identifier.id) === value.args.length && value.args[0].kind === 'Identifier' ) { const fnExpr = fnExpressions.get(value.args[0].identifier.id); @@ -132,7 +152,7 @@ export function inferEffectDependencies( loc: GeneratedSource, }; - const depsPlace = createTemporaryPlace(env, GeneratedSource); + const depsPlace = createTemporaryPlace(fn.env, GeneratedSource); depsPlace.effect = Effect.Read; newInstructions.push({ @@ -142,8 +162,8 @@ export function inferEffectDependencies( value: deps, }); - // Step 2: insert the deps array as an argument of the useEffect - value.args[1] = {...depsPlace, effect: Effect.Freeze}; + // 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); } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-deps-custom-config.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-deps-custom-config.expect.md new file mode 100644 index 0000000000000..b439d5dee07e2 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-deps-custom-config.expect.md @@ -0,0 +1,61 @@ + +## Input + +```javascript +// @inferEffectDependencies +import {print, useSpecialEffect} from 'shared-runtime'; + +function CustomConfig({propVal}) { + // Insertion + useSpecialEffect(() => print(propVal), [propVal]); + // No insertion + useSpecialEffect(() => print(propVal), [propVal], [propVal]); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies +import { print, useSpecialEffect } from "shared-runtime"; + +function CustomConfig(t0) { + const $ = _c(7); + const { propVal } = t0; + let t1; + let t2; + if ($[0] !== propVal) { + t1 = () => print(propVal); + t2 = [propVal]; + $[0] = propVal; + $[1] = t1; + $[2] = t2; + } else { + t1 = $[1]; + t2 = $[2]; + } + useSpecialEffect(t1, t2, [propVal]); + let t3; + let t4; + let t5; + if ($[3] !== propVal) { + t3 = () => print(propVal); + t4 = [propVal]; + t5 = [propVal]; + $[3] = propVal; + $[4] = t3; + $[5] = t4; + $[6] = t5; + } else { + t3 = $[4]; + t4 = $[5]; + t5 = $[6]; + } + useSpecialEffect(t3, t4, t5); +} + +``` + +### 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-deps-custom-config.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-deps-custom-config.js new file mode 100644 index 0000000000000..124e607a8ff48 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-deps-custom-config.js @@ -0,0 +1,9 @@ +// @inferEffectDependencies +import {print, useSpecialEffect} from 'shared-runtime'; + +function CustomConfig({propVal}) { + // Insertion + useSpecialEffect(() => print(propVal), [propVal]); + // No insertion + useSpecialEffect(() => print(propVal), [propVal], [propVal]); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md index febdd005a3cb0..6e45941f24264 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md @@ -3,6 +3,8 @@ ```javascript // @inferEffectDependencies +import {useEffect, useRef} from 'react'; + const moduleNonReactive = 0; function Component({foo, bar}) { @@ -45,6 +47,8 @@ function Component({foo, bar}) { ```javascript import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies +import { useEffect, useRef } from "react"; + const moduleNonReactive = 0; function Component(t0) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.js index 6a70bc1298b0a..723ad6516565f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.js @@ -1,4 +1,6 @@ // @inferEffectDependencies +import {useEffect, useRef} from 'react'; + const moduleNonReactive = 0; function Component({foo, bar}) { diff --git a/compiler/packages/snap/src/compiler.ts b/compiler/packages/snap/src/compiler.ts index 6942ffb2adc7b..95af40d62a880 100644 --- a/compiler/packages/snap/src/compiler.ts +++ b/compiler/packages/snap/src/compiler.ts @@ -174,11 +174,6 @@ function makePluginOptions( .filter(s => s.length > 0); } - let inferEffectDependencies = false; - if (firstLine.includes('@inferEffectDependencies')) { - inferEffectDependencies = true; - } - let logs: Array<{filename: string | null; event: LoggerEvent}> = []; let logger: Logger | null = null; if (firstLine.includes('@logger')) { @@ -202,7 +197,6 @@ function makePluginOptions( hookPattern, validatePreserveExistingMemoizationGuarantees, validateBlocklistedImports, - inferEffectDependencies, }, compilationMode, logger, diff --git a/compiler/packages/snap/src/sprout/shared-runtime.ts b/compiler/packages/snap/src/sprout/shared-runtime.ts index bd069ae6d0299..58815842cb03c 100644 --- a/compiler/packages/snap/src/sprout/shared-runtime.ts +++ b/compiler/packages/snap/src/sprout/shared-runtime.ts @@ -363,6 +363,14 @@ export function useFragment(..._args: Array): object { }; } +export function useSpecialEffect( + fn: () => any, + _secondArg: any, + deps: Array, +) { + React.useEffect(fn, deps); +} + export function typedArrayPush(array: Array, item: T): void { array.push(item); } @@ -370,4 +378,5 @@ export function typedArrayPush(array: Array, item: T): void { export function typedLog(...values: Array): void { console.log(...values); } + export default typedLog;