From 03297e048d08de2f7c4c0d2950e2cb1c13875f66 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 20 Dec 2024 15:09:09 -0500 Subject: [PATCH 1/4] [compiler] transform fire calls (#31796) This is the diff with the meaningful changes. The approach is: 1. Collect fire callees and remove fire() calls, create a new binding for the useFire result 2. Update LoadLocals for captured callees to point to the useFire result 3. Update function context to reference useFire results 4. Insert useFire calls after getting to the component scope This approach aims to minimize the amount of new bindings we introduce for the function expressions to minimize bookkeeping for dependency arrays. We keep all of the LoadLocals leading up to function calls as they are and insert new instructions to load the originally captured function, call useFire, and store the result in a new promoted temporary. The lvalues that referenced the original callee are changed to point to the new useFire result. This is the minimal diff to implement the expected behavior (up to importing the useFire call, next diff) and further stacked diffs implement error handling. The rules for fire are: 1. If you use fire for a callee in the effect once you must use it for every time you call it in that effect 2. You can only use fire in a useEffect lambda/functions defined inside the useEffect lambda There is still more work to do here, like updating the effect dependency array and handling object methods -- --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31796). * #31811 * #31798 * #31797 * __->__ #31796 --- .../src/Entrypoint/Pipeline.ts | 6 + .../src/Transform/TransformFire.ts | 613 ++++++++++++++++++ .../src/Transform/index.ts | 7 + .../compiler/transform-fire/basic.expect.md | 52 ++ .../fixtures/compiler/transform-fire/basic.js | 13 + .../transform-fire/deep-scope.expect.md | 73 +++ .../compiler/transform-fire/deep-scope.js | 22 + ...r.invalid-conditional-use-effect.expect.md | 37 ++ .../error.invalid-conditional-use-effect.js | 16 + .../error.invalid-multiple-args.expect.md | 34 + .../error.invalid-multiple-args.js | 13 + .../error.invalid-nested-use-effect.expect.md | 40 ++ .../error.invalid-nested-use-effect.js | 19 + .../error.invalid-not-call.expect.md | 34 + .../transform-fire/error.invalid-not-call.js | 13 + .../error.invalid-spread.expect.md | 34 + .../transform-fire/error.invalid-spread.js | 13 + .../error.todo-method.expect.md | 34 + .../transform-fire/error.todo-method.js | 13 + .../transform-fire/multiple-scope.expect.md | 65 ++ .../compiler/transform-fire/multiple-scope.js | 21 + .../transform-fire/repeated-calls.expect.md | 61 ++ .../compiler/transform-fire/repeated-calls.js | 14 + .../shared-hook-calls.expect.md | 80 +++ .../transform-fire/shared-hook-calls.js | 18 + .../use-effect-no-args-no-op.expect.md | 30 + .../use-effect-no-args-no-op.js | 8 + 27 files changed, 1383 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/Transform/TransformFire.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/Transform/index.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/basic.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/basic.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/deep-scope.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/deep-scope.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-conditional-use-effect.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-conditional-use-effect.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-multiple-args.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-multiple-args.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-nested-use-effect.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-nested-use-effect.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-not-call.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-not-call.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-spread.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-spread.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.todo-method.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.todo-method.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/multiple-scope.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/multiple-scope.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repeated-calls.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repeated-calls.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/shared-hook-calls.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/shared-hook-calls.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/use-effect-no-args-no-op.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/use-effect-no-args-no-op.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 c48cba32b2642..ca6abc0748eb9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -98,6 +98,7 @@ import {validateNoJSXInTryStatement} from '../Validation/ValidateNoJSXInTryState import {propagateScopeDependenciesHIR} from '../HIR/PropagateScopeDependenciesHIR'; import {outlineJSX} from '../Optimization/OutlineJsx'; import {optimizePropsMethodCalls} from '../Optimization/OptimizePropsMethodCalls'; +import {transformFire} from '../Transform'; export type CompilerPipelineValue = | {kind: 'ast'; name: string; value: CodegenFunction} @@ -197,6 +198,11 @@ function runWithEnvironment( validateHooksUsage(hir); } + if (env.config.enableFire) { + transformFire(hir); + log({kind: 'hir', name: 'TransformFire', value: hir}); + } + if (env.config.validateNoCapitalizedCalls) { validateNoCapitalizedCalls(hir); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Transform/TransformFire.ts b/compiler/packages/babel-plugin-react-compiler/src/Transform/TransformFire.ts new file mode 100644 index 0000000000000..3fbd141212db2 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/Transform/TransformFire.ts @@ -0,0 +1,613 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import {CompilerError, CompilerErrorDetailOptions, ErrorSeverity} from '..'; +import { + CallExpression, + Effect, + Environment, + FunctionExpression, + GeneratedSource, + HIRFunction, + Identifier, + IdentifierId, + Instruction, + InstructionId, + InstructionKind, + InstructionValue, + isUseEffectHookType, + LoadLocal, + makeInstructionId, + Place, + promoteTemporary, +} from '../HIR'; +import {createTemporaryPlace, markInstructionIds} from '../HIR/HIRBuilder'; +import {getOrInsertWith} from '../Utils/utils'; +import {BuiltInFireId, DefaultNonmutatingHook} from '../HIR/ObjectShape'; + +/* + * TODO(jmbrown): + * In this stack: + * - Insert useFire import + * - Assert no lingering fire calls + * - Ensure a fired function is not called regularly elsewhere in the same effect + * + * Future: + * - rewrite dep arrays + * - traverse object methods + * - method calls + * - React.useEffect calls + */ + +const CANNOT_COMPILE_FIRE = 'Cannot compile `fire`'; + +export function transformFire(fn: HIRFunction): void { + const context = new Context(fn.env); + replaceFireFunctions(fn, context); + context.throwIfErrorsFound(); +} + +function replaceFireFunctions(fn: HIRFunction, context: Context): void { + let hasRewrite = false; + for (const [, block] of fn.body.blocks) { + const rewriteInstrs = new Map>(); + const deleteInstrs = new Set(); + for (const instr of block.instructions) { + const {value, lvalue} = instr; + if ( + value.kind === 'CallExpression' && + isUseEffectHookType(value.callee.identifier) && + value.args.length > 0 && + value.args[0].kind === 'Identifier' + ) { + const lambda = context.getFunctionExpression( + value.args[0].identifier.id, + ); + if (lambda != null) { + const capturedCallees = + visitFunctionExpressionAndPropagateFireDependencies( + lambda, + context, + true, + ); + + // Add useFire calls for all fire calls in found in the lambda + const newInstrs = []; + for (const [ + fireCalleePlace, + fireCalleeInfo, + ] of capturedCallees.entries()) { + if (!context.hasCalleeWithInsertedFire(fireCalleePlace)) { + context.addCalleeWithInsertedFire(fireCalleePlace); + const loadUseFireInstr = makeLoadUseFireInstruction(fn.env); + const loadFireCalleeInstr = makeLoadFireCalleeInstruction( + fn.env, + fireCalleeInfo.capturedCalleeIdentifier, + ); + const callUseFireInstr = makeCallUseFireInstruction( + fn.env, + loadUseFireInstr.lvalue, + loadFireCalleeInstr.lvalue, + ); + const storeUseFireInstr = makeStoreUseFireInstruction( + fn.env, + callUseFireInstr.lvalue, + fireCalleeInfo.fireFunctionBinding, + ); + newInstrs.push( + loadUseFireInstr, + loadFireCalleeInstr, + callUseFireInstr, + storeUseFireInstr, + ); + + // We insert all of these instructions before the useEffect is loaded + const loadUseEffectInstrId = context.getLoadGlobalInstrId( + value.callee.identifier.id, + ); + if (loadUseEffectInstrId == null) { + context.pushError({ + loc: value.loc, + description: null, + severity: ErrorSeverity.Invariant, + reason: '[InsertFire] No LoadGlobal found for useEffect call', + suggestions: null, + }); + continue; + } + rewriteInstrs.set(loadUseEffectInstrId, newInstrs); + } + } + } + } else if ( + value.kind === 'CallExpression' && + value.callee.identifier.type.kind === 'Function' && + value.callee.identifier.type.shapeId === BuiltInFireId && + context.inUseEffectLambda() + ) { + /* + * We found a fire(callExpr()) call. We remove the `fire()` call and replace the callExpr() + * with a freshly generated fire function binding. We'll insert the useFire call before the + * useEffect call, which happens in the CallExpression (useEffect) case above. + */ + + /* + * We only allow fire to be called with a CallExpression: `fire(f())` + * TODO: add support for method calls: `fire(this.method())` + */ + if (value.args.length === 1 && value.args[0].kind === 'Identifier') { + const callExpr = context.getCallExpression( + value.args[0].identifier.id, + ); + + if (callExpr != null) { + const calleeId = callExpr.callee.identifier.id; + const loadLocal = context.getLoadLocalInstr(calleeId); + if (loadLocal == null) { + context.pushError({ + loc: value.loc, + description: null, + severity: ErrorSeverity.Invariant, + reason: + '[InsertFire] No loadLocal found for fire call argument', + suggestions: null, + }); + continue; + } + + const fireFunctionBinding = + context.getOrGenerateFireFunctionBinding(loadLocal.place); + + loadLocal.place = {...fireFunctionBinding}; + + // Delete the fire call expression + deleteInstrs.add(instr.id); + } else { + context.pushError({ + loc: value.loc, + description: + '`fire()` can only receive a function call such as `fire(fn(a,b)). Method calls and other expressions are not allowed', + severity: ErrorSeverity.InvalidReact, + reason: CANNOT_COMPILE_FIRE, + suggestions: null, + }); + } + } else { + let description: string = + 'fire() can only take in a single call expression as an argument'; + if (value.args.length === 0) { + description += ' but received none'; + } else if (value.args.length > 1) { + description += ' but received multiple arguments'; + } else if (value.args[0].kind === 'Spread') { + description += ' but received a spread argument'; + } + context.pushError({ + loc: value.loc, + description, + severity: ErrorSeverity.InvalidReact, + reason: CANNOT_COMPILE_FIRE, + suggestions: null, + }); + } + } else if (value.kind === 'CallExpression') { + context.addCallExpression(lvalue.identifier.id, value); + } else if ( + value.kind === 'FunctionExpression' && + context.inUseEffectLambda() + ) { + visitFunctionExpressionAndPropagateFireDependencies( + value, + context, + false, + ); + } else if (value.kind === 'FunctionExpression') { + context.addFunctionExpression(lvalue.identifier.id, value); + } else if (value.kind === 'LoadLocal') { + context.addLoadLocalInstr(lvalue.identifier.id, value); + } else if ( + value.kind === 'LoadGlobal' && + value.binding.kind === 'ImportSpecifier' && + value.binding.module === 'react' && + value.binding.imported === 'fire' && + context.inUseEffectLambda() + ) { + deleteInstrs.add(instr.id); + } else if (value.kind === 'LoadGlobal') { + context.addLoadGlobalInstrId(lvalue.identifier.id, instr.id); + } + } + block.instructions = rewriteInstructions(rewriteInstrs, block.instructions); + block.instructions = deleteInstructions(deleteInstrs, block.instructions); + + if (rewriteInstrs.size > 0 || deleteInstrs.size > 0) { + hasRewrite = true; + } + } + + if (hasRewrite) { + markInstructionIds(fn.body); + } +} + +/** + * Traverses a function expression to find fire calls fire(foo()) and replaces them with + * fireFoo(). + * + * When a function captures a fire call we need to update its context to reflect the newly created + * fire function bindings and update the LoadLocals referenced by the function's dependencies. + * + * @param isUseEffect is necessary so we can keep track of when we should additionally insert + * useFire hooks calls. + */ +function visitFunctionExpressionAndPropagateFireDependencies( + fnExpr: FunctionExpression, + context: Context, + enteringUseEffect: boolean, +): FireCalleesToFireFunctionBinding { + let withScope = enteringUseEffect + ? context.withUseEffectLambdaScope.bind(context) + : context.withFunctionScope.bind(context); + + const calleesCapturedByFnExpression = withScope(() => + replaceFireFunctions(fnExpr.loweredFunc.func, context), + ); + + /* + * Make a mapping from each dependency to the corresponding LoadLocal for it so that + * we can replace the loaded place with the generated fire function binding + */ + const loadLocalsToDepLoads = new Map(); + for (const dep of fnExpr.loweredFunc.dependencies) { + const loadLocal = context.getLoadLocalInstr(dep.identifier.id); + if (loadLocal != null) { + loadLocalsToDepLoads.set(loadLocal.place.identifier.id, loadLocal); + } + } + + const replacedCallees = new Map(); + for (const [ + calleeIdentifierId, + loadedFireFunctionBindingPlace, + ] of calleesCapturedByFnExpression.entries()) { + /* + * Given the ids of captured fire callees, look at the deps for loads of those identifiers + * and replace them with the new fire function binding + */ + const loadLocal = loadLocalsToDepLoads.get(calleeIdentifierId); + if (loadLocal == null) { + context.pushError({ + loc: fnExpr.loc, + description: null, + severity: ErrorSeverity.Invariant, + reason: + '[InsertFire] No loadLocal found for fire call argument for lambda', + suggestions: null, + }); + continue; + } + + const oldPlaceId = loadLocal.place.identifier.id; + loadLocal.place = { + ...loadedFireFunctionBindingPlace.fireFunctionBinding, + }; + + replacedCallees.set( + oldPlaceId, + loadedFireFunctionBindingPlace.fireFunctionBinding, + ); + } + + // For each replaced callee, update the context of the function expression to track it + for ( + let contextIdx = 0; + contextIdx < fnExpr.loweredFunc.func.context.length; + contextIdx++ + ) { + const contextItem = fnExpr.loweredFunc.func.context[contextIdx]; + const replacedCallee = replacedCallees.get(contextItem.identifier.id); + if (replacedCallee != null) { + fnExpr.loweredFunc.func.context[contextIdx] = replacedCallee; + } + } + + context.mergeCalleesFromInnerScope(calleesCapturedByFnExpression); + + return calleesCapturedByFnExpression; +} + +function makeLoadUseFireInstruction(env: Environment): Instruction { + const useFirePlace = createTemporaryPlace(env, GeneratedSource); + useFirePlace.effect = Effect.Read; + useFirePlace.identifier.type = DefaultNonmutatingHook; + const instrValue: InstructionValue = { + kind: 'LoadGlobal', + binding: { + kind: 'ImportSpecifier', + name: 'useFire', + module: 'react', + imported: 'useFire', + }, + loc: GeneratedSource, + }; + return { + id: makeInstructionId(0), + value: instrValue, + lvalue: {...useFirePlace}, + loc: GeneratedSource, + }; +} + +function makeLoadFireCalleeInstruction( + env: Environment, + fireCalleeIdentifier: Identifier, +): Instruction { + const loadedFireCallee = createTemporaryPlace(env, GeneratedSource); + const fireCallee: Place = { + kind: 'Identifier', + identifier: fireCalleeIdentifier, + reactive: false, + effect: Effect.Unknown, + loc: fireCalleeIdentifier.loc, + }; + return { + id: makeInstructionId(0), + value: { + kind: 'LoadLocal', + loc: GeneratedSource, + place: {...fireCallee}, + }, + lvalue: {...loadedFireCallee}, + loc: GeneratedSource, + }; +} + +function makeCallUseFireInstruction( + env: Environment, + useFirePlace: Place, + argPlace: Place, +): Instruction { + const useFireCallResultPlace = createTemporaryPlace(env, GeneratedSource); + useFireCallResultPlace.effect = Effect.Read; + + const useFireCall: CallExpression = { + kind: 'CallExpression', + callee: {...useFirePlace}, + args: [argPlace], + loc: GeneratedSource, + }; + + return { + id: makeInstructionId(0), + value: useFireCall, + lvalue: {...useFireCallResultPlace}, + loc: GeneratedSource, + }; +} + +function makeStoreUseFireInstruction( + env: Environment, + useFireCallResultPlace: Place, + fireFunctionBindingPlace: Place, +): Instruction { + promoteTemporary(fireFunctionBindingPlace.identifier); + + const fireFunctionBindingLValuePlace = createTemporaryPlace( + env, + GeneratedSource, + ); + return { + id: makeInstructionId(0), + value: { + kind: 'StoreLocal', + lvalue: { + kind: InstructionKind.Const, + place: {...fireFunctionBindingPlace}, + }, + value: {...useFireCallResultPlace}, + type: null, + loc: GeneratedSource, + }, + lvalue: fireFunctionBindingLValuePlace, + loc: GeneratedSource, + }; +} + +type FireCalleesToFireFunctionBinding = Map< + IdentifierId, + { + fireFunctionBinding: Place; + capturedCalleeIdentifier: Identifier; + } +>; + +class Context { + #env: Environment; + + #errors: CompilerError = new CompilerError(); + + /* + * Used to look up the call expression passed to a `fire(callExpr())`. Gives back + * the `callExpr()`. + */ + #callExpressions = new Map(); + + /* + * We keep track of function expressions so that we can traverse them when + * we encounter a lambda passed to a useEffect call + */ + #functionExpressions = new Map(); + + /* + * Mapping from lvalue ids to the LoadLocal for it. Allows us to replace dependency LoadLocals. + */ + #loadLocals = new Map(); + + /* + * Maps all of the fire callees found in a component/hook to the generated fire function places + * we create for them. Allows us to reuse already-inserted useFire results + */ + #fireCalleesToFireFunctions: Map = new Map(); + + /* + * The callees for which we have already created fire bindings. Used to skip inserting a new + * useFire call for a fire callee if one has already been created. + */ + #calleesWithInsertedFire = new Set(); + + /* + * A mapping from fire callees to the created fire function bindings that are reachable from this + * scope. + * + * We additionally keep track of the captured callee identifier so that we can properly reference + * it in the place where we LoadLocal the callee as an argument to useFire. + */ + #capturedCalleeIdentifierIds: FireCalleesToFireFunctionBinding = new Map(); + + /* + * We only transform fire calls if we're syntactically within a useEffect lambda (for now) + */ + #inUseEffectLambda = false; + + /* + * Mapping from useEffect callee identifier ids to the instruction id of the + * load global instruction for the useEffect call. We use this to insert the + * useFire calls before the useEffect call + */ + #loadGlobalInstructionIds = new Map(); + + constructor(env: Environment) { + this.#env = env; + } + + pushError(error: CompilerErrorDetailOptions): void { + this.#errors.push(error); + } + + withFunctionScope(fn: () => void): FireCalleesToFireFunctionBinding { + fn(); + return this.#capturedCalleeIdentifierIds; + } + + withUseEffectLambdaScope(fn: () => void): FireCalleesToFireFunctionBinding { + const capturedCalleeIdentifierIds = this.#capturedCalleeIdentifierIds; + const inUseEffectLambda = this.#inUseEffectLambda; + + this.#capturedCalleeIdentifierIds = new Map(); + this.#inUseEffectLambda = true; + + const resultCapturedCalleeIdentifierIds = this.withFunctionScope(fn); + + this.#capturedCalleeIdentifierIds = capturedCalleeIdentifierIds; + this.#inUseEffectLambda = inUseEffectLambda; + + return resultCapturedCalleeIdentifierIds; + } + + addCallExpression(id: IdentifierId, callExpr: CallExpression): void { + this.#callExpressions.set(id, callExpr); + } + + getCallExpression(id: IdentifierId): CallExpression | undefined { + return this.#callExpressions.get(id); + } + + addLoadLocalInstr(id: IdentifierId, loadLocal: LoadLocal): void { + this.#loadLocals.set(id, loadLocal); + } + + getLoadLocalInstr(id: IdentifierId): LoadLocal | undefined { + return this.#loadLocals.get(id); + } + + getOrGenerateFireFunctionBinding(callee: Place): Place { + const fireFunctionBinding = getOrInsertWith( + this.#fireCalleesToFireFunctions, + callee.identifier.id, + () => createTemporaryPlace(this.#env, GeneratedSource), + ); + + this.#capturedCalleeIdentifierIds.set(callee.identifier.id, { + fireFunctionBinding, + capturedCalleeIdentifier: callee.identifier, + }); + + return fireFunctionBinding; + } + + mergeCalleesFromInnerScope( + innerCallees: FireCalleesToFireFunctionBinding, + ): void { + for (const [id, calleeInfo] of innerCallees.entries()) { + this.#capturedCalleeIdentifierIds.set(id, calleeInfo); + } + } + + addCalleeWithInsertedFire(id: IdentifierId): void { + this.#calleesWithInsertedFire.add(id); + } + + hasCalleeWithInsertedFire(id: IdentifierId): boolean { + return this.#calleesWithInsertedFire.has(id); + } + + inUseEffectLambda(): boolean { + return this.#inUseEffectLambda; + } + + addFunctionExpression(id: IdentifierId, fn: FunctionExpression): void { + this.#functionExpressions.set(id, fn); + } + + getFunctionExpression(id: IdentifierId): FunctionExpression | undefined { + return this.#functionExpressions.get(id); + } + + addLoadGlobalInstrId(id: IdentifierId, instrId: InstructionId): void { + this.#loadGlobalInstructionIds.set(id, instrId); + } + + getLoadGlobalInstrId(id: IdentifierId): InstructionId | undefined { + return this.#loadGlobalInstructionIds.get(id); + } + + throwIfErrorsFound(): void { + if (this.#errors.hasErrors()) throw this.#errors; + } +} + +function deleteInstructions( + deleteInstrs: Set, + instructions: Array, +): Array { + if (deleteInstrs.size > 0) { + const newInstrs = instructions.filter(instr => !deleteInstrs.has(instr.id)); + return newInstrs; + } + return instructions; +} + +function rewriteInstructions( + rewriteInstrs: Map>, + instructions: Array, +): Array { + if (rewriteInstrs.size > 0) { + const newInstrs = []; + for (const instr of instructions) { + const newInstrsAtId = rewriteInstrs.get(instr.id); + if (newInstrsAtId != null) { + newInstrs.push(...newInstrsAtId, instr); + } else { + newInstrs.push(instr); + } + } + + return newInstrs; + } + + return instructions; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Transform/index.ts b/compiler/packages/babel-plugin-react-compiler/src/Transform/index.ts new file mode 100644 index 0000000000000..8665ead0b1af0 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/Transform/index.ts @@ -0,0 +1,7 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ +export {transformFire} from './TransformFire'; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/basic.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/basic.expect.md new file mode 100644 index 0000000000000..f3b67da3ecfab --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/basic.expect.md @@ -0,0 +1,52 @@ + +## Input + +```javascript +// @enableFire +import {fire} from 'react'; + +function Component(props) { + const foo = props => { + console.log(props); + }; + useEffect(() => { + fire(foo(props)); + }); + + return null; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableFire +import { fire } from "react"; + +function Component(props) { + const $ = _c(3); + const foo = _temp; + const t0 = useFire(foo); + let t1; + if ($[0] !== props || $[1] !== t0) { + t1 = () => { + t0(props); + }; + $[0] = props; + $[1] = t0; + $[2] = t1; + } else { + t1 = $[2]; + } + useEffect(t1); + return null; +} +function _temp(props_0) { + console.log(props_0); +} + +``` + +### 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/transform-fire/basic.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/basic.js new file mode 100644 index 0000000000000..2f7a72e4eed51 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/basic.js @@ -0,0 +1,13 @@ +// @enableFire +import {fire} from 'react'; + +function Component(props) { + const foo = props => { + console.log(props); + }; + useEffect(() => { + fire(foo(props)); + }); + + return null; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/deep-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/deep-scope.expect.md new file mode 100644 index 0000000000000..ee9bf268d0e6a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/deep-scope.expect.md @@ -0,0 +1,73 @@ + +## Input + +```javascript +// @enableFire +import {fire} from 'react'; + +function Component(props) { + const foo = props => { + console.log(props); + }; + useEffect(() => { + function nested() { + function nestedAgain() { + function nestedThrice() { + fire(foo(props)); + } + nestedThrice(); + } + nestedAgain(); + } + nested(); + }); + + return null; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableFire +import { fire } from "react"; + +function Component(props) { + const $ = _c(3); + const foo = _temp; + const t0 = useFire(foo); + let t1; + if ($[0] !== props || $[1] !== t0) { + t1 = () => { + const nested = function nested() { + const nestedAgain = function nestedAgain() { + const nestedThrice = function nestedThrice() { + t0(props); + }; + + nestedThrice(); + }; + + nestedAgain(); + }; + + nested(); + }; + $[0] = props; + $[1] = t0; + $[2] = t1; + } else { + t1 = $[2]; + } + useEffect(t1); + return null; +} +function _temp(props_0) { + console.log(props_0); +} + +``` + +### 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/transform-fire/deep-scope.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/deep-scope.js new file mode 100644 index 0000000000000..b056c3f53a85a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/deep-scope.js @@ -0,0 +1,22 @@ +// @enableFire +import {fire} from 'react'; + +function Component(props) { + const foo = props => { + console.log(props); + }; + useEffect(() => { + function nested() { + function nestedAgain() { + function nestedThrice() { + fire(foo(props)); + } + nestedThrice(); + } + nestedAgain(); + } + nested(); + }); + + return null; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-conditional-use-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-conditional-use-effect.expect.md new file mode 100644 index 0000000000000..a24f27a695f54 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-conditional-use-effect.expect.md @@ -0,0 +1,37 @@ + +## Input + +```javascript +// @enableFire +import {fire, useEffect} from 'react'; + +function Component(props) { + const foo = props => { + console.log(props); + }; + + if (props.cond) { + useEffect(() => { + fire(foo(props)); + }); + } + + return null; +} + +``` + + +## Error + +``` + 8 | + 9 | if (props.cond) { +> 10 | useEffect(() => { + | ^^^^^^^^^ InvalidReact: Hooks must always be called in a consistent order, and may not be called conditionally. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (10:10) + 11 | fire(foo(props)); + 12 | }); + 13 | } +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-conditional-use-effect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-conditional-use-effect.js new file mode 100644 index 0000000000000..30ae8e59b986e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-conditional-use-effect.js @@ -0,0 +1,16 @@ +// @enableFire +import {fire, useEffect} from 'react'; + +function Component(props) { + const foo = props => { + console.log(props); + }; + + if (props.cond) { + useEffect(() => { + fire(foo(props)); + }); + } + + return null; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-multiple-args.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-multiple-args.expect.md new file mode 100644 index 0000000000000..8329717cb3939 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-multiple-args.expect.md @@ -0,0 +1,34 @@ + +## Input + +```javascript +// @enableFire +import {fire} from 'react'; + +function Component({bar, baz}) { + const foo = () => { + console.log(bar, baz); + }; + useEffect(() => { + fire(foo(bar), baz); + }); + + return null; +} + +``` + + +## Error + +``` + 7 | }; + 8 | useEffect(() => { +> 9 | fire(foo(bar), baz); + | ^^^^^^^^^^^^^^^^^^^ InvalidReact: Cannot compile `fire`. fire() can only take in a single call expression as an argument but received multiple arguments (9:9) + 10 | }); + 11 | + 12 | return null; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-multiple-args.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-multiple-args.js new file mode 100644 index 0000000000000..980b0dfcb5e78 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-multiple-args.js @@ -0,0 +1,13 @@ +// @enableFire +import {fire} from 'react'; + +function Component({bar, baz}) { + const foo = () => { + console.log(bar, baz); + }; + useEffect(() => { + fire(foo(bar), baz); + }); + + return null; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-nested-use-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-nested-use-effect.expect.md new file mode 100644 index 0000000000000..580fd6a2a68b8 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-nested-use-effect.expect.md @@ -0,0 +1,40 @@ + +## Input + +```javascript +// @enable +import {fire} from 'react'; + +function Component(props) { + const foo = props => { + console.log(props); + }; + useEffect(() => { + useEffect(() => { + function nested() { + fire(foo(props)); + } + + nested(); + }); + }); + + return null; +} + +``` + + +## Error + +``` + 7 | }; + 8 | useEffect(() => { +> 9 | useEffect(() => { + | ^^^^^^^^^ InvalidReact: Hooks must be called at the top level in the body of a function component or custom hook, and may not be called within function expressions. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning). Cannot call useEffect within a function component (9:9) + 10 | function nested() { + 11 | fire(foo(props)); + 12 | } +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-nested-use-effect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-nested-use-effect.js new file mode 100644 index 0000000000000..16f242572445c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-nested-use-effect.js @@ -0,0 +1,19 @@ +// @enable +import {fire} from 'react'; + +function Component(props) { + const foo = props => { + console.log(props); + }; + useEffect(() => { + useEffect(() => { + function nested() { + fire(foo(props)); + } + + nested(); + }); + }); + + return null; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-not-call.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-not-call.expect.md new file mode 100644 index 0000000000000..855c7b7d706cb --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-not-call.expect.md @@ -0,0 +1,34 @@ + +## Input + +```javascript +// @enableFire +import {fire} from 'react'; + +function Component(props) { + const foo = () => { + console.log(props); + }; + useEffect(() => { + fire(props); + }); + + return null; +} + +``` + + +## Error + +``` + 7 | }; + 8 | useEffect(() => { +> 9 | fire(props); + | ^^^^^^^^^^^ InvalidReact: Cannot compile `fire`. `fire()` can only receive a function call such as `fire(fn(a,b)). Method calls and other expressions are not allowed (9:9) + 10 | }); + 11 | + 12 | return null; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-not-call.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-not-call.js new file mode 100644 index 0000000000000..3d1ae3658fd20 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-not-call.js @@ -0,0 +1,13 @@ +// @enableFire +import {fire} from 'react'; + +function Component(props) { + const foo = () => { + console.log(props); + }; + useEffect(() => { + fire(props); + }); + + return null; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-spread.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-spread.expect.md new file mode 100644 index 0000000000000..c0b797fc14471 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-spread.expect.md @@ -0,0 +1,34 @@ + +## Input + +```javascript +// @enableFire +import {fire} from 'react'; + +function Component(props) { + const foo = () => { + console.log(props); + }; + useEffect(() => { + fire(...foo); + }); + + return null; +} + +``` + + +## Error + +``` + 7 | }; + 8 | useEffect(() => { +> 9 | fire(...foo); + | ^^^^^^^^^^^^ InvalidReact: Cannot compile `fire`. fire() can only take in a single call expression as an argument but received a spread argument (9:9) + 10 | }); + 11 | + 12 | return null; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-spread.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-spread.js new file mode 100644 index 0000000000000..68e317588bd51 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-spread.js @@ -0,0 +1,13 @@ +// @enableFire +import {fire} from 'react'; + +function Component(props) { + const foo = () => { + console.log(props); + }; + useEffect(() => { + fire(...foo); + }); + + return null; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.todo-method.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.todo-method.expect.md new file mode 100644 index 0000000000000..3f237cfc6f364 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.todo-method.expect.md @@ -0,0 +1,34 @@ + +## Input + +```javascript +// @enableFire +import {fire} from 'react'; + +function Component(props) { + const foo = () => { + console.log(props); + }; + useEffect(() => { + fire(props.foo()); + }); + + return null; +} + +``` + + +## Error + +``` + 7 | }; + 8 | useEffect(() => { +> 9 | fire(props.foo()); + | ^^^^^^^^^^^^^^^^^ InvalidReact: Cannot compile `fire`. `fire()` can only receive a function call such as `fire(fn(a,b)). Method calls and other expressions are not allowed (9:9) + 10 | }); + 11 | + 12 | return null; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.todo-method.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.todo-method.js new file mode 100644 index 0000000000000..c75622ca5e7a3 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.todo-method.js @@ -0,0 +1,13 @@ +// @enableFire +import {fire} from 'react'; + +function Component(props) { + const foo = () => { + console.log(props); + }; + useEffect(() => { + fire(props.foo()); + }); + + return null; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/multiple-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/multiple-scope.expect.md new file mode 100644 index 0000000000000..08c45ea279d8d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/multiple-scope.expect.md @@ -0,0 +1,65 @@ + +## Input + +```javascript +// @enableFire +import {fire} from 'react'; + +function Component(props) { + const foo = props => { + console.log(props); + }; + useEffect(() => { + fire(foo(props)); + function nested() { + fire(foo(props)); + function innerNested() { + fire(foo(props)); + } + } + + nested(); + }); + + return null; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableFire +import { fire } from "react"; + +function Component(props) { + const $ = _c(3); + const foo = _temp; + const t0 = useFire(foo); + let t1; + if ($[0] !== props || $[1] !== t0) { + t1 = () => { + t0(props); + const nested = function nested() { + t0(props); + }; + + nested(); + }; + $[0] = props; + $[1] = t0; + $[2] = t1; + } else { + t1 = $[2]; + } + useEffect(t1); + return null; +} +function _temp(props_0) { + console.log(props_0); +} + +``` + +### 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/transform-fire/multiple-scope.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/multiple-scope.js new file mode 100644 index 0000000000000..54410680e63c6 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/multiple-scope.js @@ -0,0 +1,21 @@ +// @enableFire +import {fire} from 'react'; + +function Component(props) { + const foo = props => { + console.log(props); + }; + useEffect(() => { + fire(foo(props)); + function nested() { + fire(foo(props)); + function innerNested() { + fire(foo(props)); + } + } + + nested(); + }); + + return null; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repeated-calls.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repeated-calls.expect.md new file mode 100644 index 0000000000000..693a8d380aa38 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repeated-calls.expect.md @@ -0,0 +1,61 @@ + +## Input + +```javascript +// @enableFire +import {fire} from 'react'; + +function Component(props) { + const foo = () => { + console.log(props); + }; + useEffect(() => { + fire(foo(props)); + fire(foo(props)); + }); + + return null; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableFire +import { fire } from "react"; + +function Component(props) { + const $ = _c(5); + let t0; + if ($[0] !== props) { + t0 = () => { + console.log(props); + }; + $[0] = props; + $[1] = t0; + } else { + t0 = $[1]; + } + const foo = t0; + const t1 = useFire(foo); + let t2; + if ($[2] !== props || $[3] !== t1) { + t2 = () => { + t1(props); + t1(props); + }; + $[2] = props; + $[3] = t1; + $[4] = t2; + } else { + t2 = $[4]; + } + useEffect(t2); + return null; +} + +``` + +### 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/transform-fire/repeated-calls.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repeated-calls.js new file mode 100644 index 0000000000000..14e1cb06b1bbe --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repeated-calls.js @@ -0,0 +1,14 @@ +// @enableFire +import {fire} from 'react'; + +function Component(props) { + const foo = () => { + console.log(props); + }; + useEffect(() => { + fire(foo(props)); + fire(foo(props)); + }); + + return null; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/shared-hook-calls.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/shared-hook-calls.expect.md new file mode 100644 index 0000000000000..959338b5d8d5d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/shared-hook-calls.expect.md @@ -0,0 +1,80 @@ + +## Input + +```javascript +// @enableFire +import {fire} from 'react'; + +function Component({bar, baz}) { + const foo = () => { + console.log(bar); + }; + useEffect(() => { + fire(foo(bar)); + fire(baz(bar)); + }); + + useEffect(() => { + fire(foo(bar)); + }); + + return null; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableFire +import { fire } from "react"; + +function Component(t0) { + const $ = _c(9); + const { bar, baz } = t0; + let t1; + if ($[0] !== bar) { + t1 = () => { + console.log(bar); + }; + $[0] = bar; + $[1] = t1; + } else { + t1 = $[1]; + } + const foo = t1; + const t2 = useFire(foo); + const t3 = useFire(baz); + let t4; + if ($[2] !== bar || $[3] !== t2 || $[4] !== t3) { + t4 = () => { + t2(bar); + t3(bar); + }; + $[2] = bar; + $[3] = t2; + $[4] = t3; + $[5] = t4; + } else { + t4 = $[5]; + } + useEffect(t4); + let t5; + if ($[6] !== bar || $[7] !== t2) { + t5 = () => { + t2(bar); + }; + $[6] = bar; + $[7] = t2; + $[8] = t5; + } else { + t5 = $[8]; + } + useEffect(t5); + return null; +} + +``` + +### 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/transform-fire/shared-hook-calls.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/shared-hook-calls.js new file mode 100644 index 0000000000000..5cb51e9bd3c78 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/shared-hook-calls.js @@ -0,0 +1,18 @@ +// @enableFire +import {fire} from 'react'; + +function Component({bar, baz}) { + const foo = () => { + console.log(bar); + }; + useEffect(() => { + fire(foo(bar)); + fire(baz(bar)); + }); + + useEffect(() => { + fire(foo(bar)); + }); + + return null; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/use-effect-no-args-no-op.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/use-effect-no-args-no-op.expect.md new file mode 100644 index 0000000000000..f482ac44ddda7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/use-effect-no-args-no-op.expect.md @@ -0,0 +1,30 @@ + +## Input + +```javascript +// @enableFire +import {fire} from 'react'; + +function Component(props) { + useEffect(); + + return null; +} + +``` + +## Code + +```javascript +// @enableFire +import { fire } from "react"; + +function Component(props) { + useEffect(); + return null; +} + +``` + +### 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/transform-fire/use-effect-no-args-no-op.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/use-effect-no-args-no-op.js new file mode 100644 index 0000000000000..731c45df677e9 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/use-effect-no-args-no-op.js @@ -0,0 +1,8 @@ +// @enableFire +import {fire} from 'react'; + +function Component(props) { + useEffect(); + + return null; +} From ab27231dc51aa2535df37555797e630d31047fa4 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 20 Dec 2024 15:25:30 -0500 Subject: [PATCH 2/4] [compiler] add fire imports (#31797) Summary: Adds import {useFire} from 'react' when fire syntax is used. This is experimentation and may not become a stable feature in the compiler. -- --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31797). * #31811 * #31798 * __->__ #31797 --- .../babel-plugin-react-compiler/src/Entrypoint/Program.ts | 5 +++++ .../babel-plugin-react-compiler/src/HIR/Environment.ts | 2 ++ .../src/ReactiveScopes/CodegenReactiveFunction.ts | 6 ++++++ .../src/Transform/TransformFire.ts | 2 +- .../fixtures/compiler/transform-fire/basic.expect.md | 1 + .../fixtures/compiler/transform-fire/deep-scope.expect.md | 1 + .../compiler/transform-fire/multiple-scope.expect.md | 1 + .../compiler/transform-fire/repeated-calls.expect.md | 1 + .../compiler/transform-fire/shared-hook-calls.expect.md | 1 + 9 files changed, 19 insertions(+), 1 deletion(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts index a6e09a1d061da..ca10e9c0d471f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts @@ -564,6 +564,11 @@ export function compileProgram( if (environment.enableChangeDetectionForDebugging != null) { externalFunctions.push(environment.enableChangeDetectionForDebugging); } + + const hasFireRewrite = compiledFns.some(c => c.compiledFn.hasFireRewrite); + if (environment.enableFire && hasFireRewrite) { + externalFunctions.push({source: 'react', importSpecifierName: 'useFire'}); + } } catch (err) { handleError(err, pass, null); return; 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 e2932296ca739..f3f426df56e44 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -787,6 +787,7 @@ export class Environment { fnType: ReactFunctionType; useMemoCacheIdentifier: string; hasLoweredContextAccess: boolean; + hasFireRewrite: boolean; #contextIdentifiers: Set; #hoistedIdentifiers: Set; @@ -811,6 +812,7 @@ export class Environment { this.#shapes = new Map(DEFAULT_SHAPES); this.#globals = new Map(DEFAULT_GLOBALS); this.hasLoweredContextAccess = false; + this.hasFireRewrite = false; if ( config.disableMemoizationForDebugging && diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts index b2f1b9e6d4edc..b9ec688d877ed 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -103,6 +103,11 @@ export type CodegenFunction = { * This is true if the compiler has the lowered useContext calls. */ hasLoweredContextAccess: boolean; + + /** + * This is true if the compiler has compiled a fire to a useFire call + */ + hasFireRewrite: boolean; }; export function codegenFunction( @@ -355,6 +360,7 @@ function codegenReactiveFunction( prunedMemoValues: countMemoBlockVisitor.prunedMemoValues, outlined: [], hasLoweredContextAccess: fn.env.hasLoweredContextAccess, + hasFireRewrite: fn.env.hasFireRewrite, }); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Transform/TransformFire.ts b/compiler/packages/babel-plugin-react-compiler/src/Transform/TransformFire.ts index 3fbd141212db2..5c7f906be881b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Transform/TransformFire.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Transform/TransformFire.ts @@ -32,7 +32,6 @@ import {BuiltInFireId, DefaultNonmutatingHook} from '../HIR/ObjectShape'; /* * TODO(jmbrown): * In this stack: - * - Insert useFire import * - Assert no lingering fire calls * - Ensure a fired function is not called regularly elsewhere in the same effect * @@ -226,6 +225,7 @@ function replaceFireFunctions(fn: HIRFunction, context: Context): void { if (rewriteInstrs.size > 0 || deleteInstrs.size > 0) { hasRewrite = true; + fn.env.hasFireRewrite = true; } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/basic.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/basic.expect.md index f3b67da3ecfab..a5bf42de7b008 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/basic.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/basic.expect.md @@ -21,6 +21,7 @@ function Component(props) { ## Code ```javascript +import { useFire } from "react"; import { c as _c } from "react/compiler-runtime"; // @enableFire import { fire } from "react"; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/deep-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/deep-scope.expect.md index ee9bf268d0e6a..585a820b13416 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/deep-scope.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/deep-scope.expect.md @@ -30,6 +30,7 @@ function Component(props) { ## Code ```javascript +import { useFire } from "react"; import { c as _c } from "react/compiler-runtime"; // @enableFire import { fire } from "react"; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/multiple-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/multiple-scope.expect.md index 08c45ea279d8d..0d531041355be 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/multiple-scope.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/multiple-scope.expect.md @@ -29,6 +29,7 @@ function Component(props) { ## Code ```javascript +import { useFire } from "react"; import { c as _c } from "react/compiler-runtime"; // @enableFire import { fire } from "react"; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repeated-calls.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repeated-calls.expect.md index 693a8d380aa38..3eb9f0e71c71a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repeated-calls.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repeated-calls.expect.md @@ -22,6 +22,7 @@ function Component(props) { ## Code ```javascript +import { useFire } from "react"; import { c as _c } from "react/compiler-runtime"; // @enableFire import { fire } from "react"; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/shared-hook-calls.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/shared-hook-calls.expect.md index 959338b5d8d5d..4362a3a8461e6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/shared-hook-calls.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/shared-hook-calls.expect.md @@ -26,6 +26,7 @@ function Component({bar, baz}) { ## Code ```javascript +import { useFire } from "react"; import { c as _c } from "react/compiler-runtime"; // @enableFire import { fire } from "react"; From 45a720f7c7ff98e22fb299b50fef90fe319081a7 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 20 Dec 2024 16:55:01 -0500 Subject: [PATCH 3/4] [compile] Error on fire outside of effects and ensure correct compilation, correct import (#31798) Traverse the compiled functions to ensure there are no lingering fires and that all fire calls are inside an effect lambda. Also corrects the import to import from the compiler runtime instead -- --- .../src/Entrypoint/Program.ts | 5 +- .../src/HIR/PrintHIR.ts | 8 ++ .../src/Transform/TransformFire.ts | 104 ++++++++++++++++-- .../compiler/transform-fire/basic.expect.md | 2 +- .../transform-fire/deep-scope.expect.md | 2 +- ...ror.invalid-mix-fire-and-no-fire.expect.md | 39 +++++++ .../error.invalid-mix-fire-and-no-fire.js | 18 +++ .../error.invalid-outside-effect.expect.md | 38 +++++++ .../error.invalid-outside-effect.js | 15 +++ .../transform-fire/multiple-scope.expect.md | 2 +- .../transform-fire/repeated-calls.expect.md | 2 +- .../shared-hook-calls.expect.md | 2 +- 12 files changed, 221 insertions(+), 16 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-mix-fire-and-no-fire.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-mix-fire-and-no-fire.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-outside-effect.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-outside-effect.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts index ca10e9c0d471f..bb0d662c4f67e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts @@ -567,7 +567,10 @@ export function compileProgram( const hasFireRewrite = compiledFns.some(c => c.compiledFn.hasFireRewrite); if (environment.enableFire && hasFireRewrite) { - externalFunctions.push({source: 'react', importSpecifierName: 'useFire'}); + externalFunctions.push({ + source: getReactCompilerRuntimeModule(pass.opts), + importSpecifierName: 'useFire', + }); } } catch (err) { handleError(err, pass, null); diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts index 526ab7c7e52bb..a6f6c606e118d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts @@ -897,6 +897,14 @@ export function printSourceLocation(loc: SourceLocation): string { } } +export function printSourceLocationLine(loc: SourceLocation): string { + if (typeof loc === 'symbol') { + return 'generated'; + } else { + return `${loc.start.line}:${loc.end.line}`; + } +} + export function printAliases(aliases: DisjointSet): string { const aliasSets = aliases.buildSets(); diff --git a/compiler/packages/babel-plugin-react-compiler/src/Transform/TransformFire.ts b/compiler/packages/babel-plugin-react-compiler/src/Transform/TransformFire.ts index 5c7f906be881b..a0256fd80cd66 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Transform/TransformFire.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Transform/TransformFire.ts @@ -5,7 +5,12 @@ * LICENSE file in the root directory of this source tree. */ -import {CompilerError, CompilerErrorDetailOptions, ErrorSeverity} from '..'; +import { + CompilerError, + CompilerErrorDetailOptions, + ErrorSeverity, + SourceLocation, +} from '..'; import { CallExpression, Effect, @@ -28,14 +33,11 @@ import { import {createTemporaryPlace, markInstructionIds} from '../HIR/HIRBuilder'; import {getOrInsertWith} from '../Utils/utils'; import {BuiltInFireId, DefaultNonmutatingHook} from '../HIR/ObjectShape'; +import {eachInstructionOperand} from '../HIR/visitors'; +import {printSourceLocationLine} from '../HIR/PrintHIR'; /* * TODO(jmbrown): - * In this stack: - * - Assert no lingering fire calls - * - Ensure a fired function is not called regularly elsewhere in the same effect - * - * Future: * - rewrite dep arrays * - traverse object methods * - method calls @@ -47,6 +49,9 @@ const CANNOT_COMPILE_FIRE = 'Cannot compile `fire`'; export function transformFire(fn: HIRFunction): void { const context = new Context(fn.env); replaceFireFunctions(fn, context); + if (!context.hasErrors()) { + ensureNoMoreFireUses(fn, context); + } context.throwIfErrorsFound(); } @@ -120,6 +125,11 @@ function replaceFireFunctions(fn: HIRFunction, context: Context): void { } rewriteInstrs.set(loadUseEffectInstrId, newInstrs); } + ensureNoRemainingCalleeCaptures( + lambda.loweredFunc.func, + context, + capturedCallees, + ); } } } else if ( @@ -159,7 +169,10 @@ function replaceFireFunctions(fn: HIRFunction, context: Context): void { } const fireFunctionBinding = - context.getOrGenerateFireFunctionBinding(loadLocal.place); + context.getOrGenerateFireFunctionBinding( + loadLocal.place, + value.loc, + ); loadLocal.place = {...fireFunctionBinding}; @@ -320,6 +333,69 @@ function visitFunctionExpressionAndPropagateFireDependencies( return calleesCapturedByFnExpression; } +/* + * eachInstructionOperand is not sufficient for our cases because: + * 1. fire is a global, which will not appear + * 2. The HIR may be malformed, so can't rely on function deps and must + * traverse the whole function. + */ +function* eachReachablePlace(fn: HIRFunction): Iterable { + for (const [, block] of fn.body.blocks) { + for (const instr of block.instructions) { + if ( + instr.value.kind === 'FunctionExpression' || + instr.value.kind === 'ObjectMethod' + ) { + yield* eachReachablePlace(instr.value.loweredFunc.func); + } else { + yield* eachInstructionOperand(instr); + } + } + } +} + +function ensureNoRemainingCalleeCaptures( + fn: HIRFunction, + context: Context, + capturedCallees: FireCalleesToFireFunctionBinding, +): void { + for (const place of eachReachablePlace(fn)) { + const calleeInfo = capturedCallees.get(place.identifier.id); + if (calleeInfo != null) { + const calleeName = + calleeInfo.capturedCalleeIdentifier.name?.kind === 'named' + ? calleeInfo.capturedCalleeIdentifier.name.value + : ''; + context.pushError({ + loc: place.loc, + description: `All uses of ${calleeName} must be either used with a fire() call in \ +this effect or not used with a fire() call at all. ${calleeName} was used with fire() on line \ +${printSourceLocationLine(calleeInfo.fireLoc)} in this effect`, + severity: ErrorSeverity.InvalidReact, + reason: CANNOT_COMPILE_FIRE, + suggestions: null, + }); + } + } +} + +function ensureNoMoreFireUses(fn: HIRFunction, context: Context): void { + for (const place of eachReachablePlace(fn)) { + if ( + place.identifier.type.kind === 'Function' && + place.identifier.type.shapeId === BuiltInFireId + ) { + context.pushError({ + loc: place.identifier.loc, + description: 'Cannot use `fire` outside of a useEffect function', + severity: ErrorSeverity.Invariant, + reason: CANNOT_COMPILE_FIRE, + suggestions: null, + }); + } + } +} + function makeLoadUseFireInstruction(env: Environment): Instruction { const useFirePlace = createTemporaryPlace(env, GeneratedSource); useFirePlace.effect = Effect.Read; @@ -422,6 +498,7 @@ type FireCalleesToFireFunctionBinding = Map< { fireFunctionBinding: Place; capturedCalleeIdentifier: Identifier; + fireLoc: SourceLocation; } >; @@ -523,8 +600,10 @@ class Context { getLoadLocalInstr(id: IdentifierId): LoadLocal | undefined { return this.#loadLocals.get(id); } - - getOrGenerateFireFunctionBinding(callee: Place): Place { + getOrGenerateFireFunctionBinding( + callee: Place, + fireLoc: SourceLocation, + ): Place { const fireFunctionBinding = getOrInsertWith( this.#fireCalleesToFireFunctions, callee.identifier.id, @@ -534,6 +613,7 @@ class Context { this.#capturedCalleeIdentifierIds.set(callee.identifier.id, { fireFunctionBinding, capturedCalleeIdentifier: callee.identifier, + fireLoc, }); return fireFunctionBinding; @@ -575,8 +655,12 @@ class Context { return this.#loadGlobalInstructionIds.get(id); } + hasErrors(): boolean { + return this.#errors.hasErrors(); + } + throwIfErrorsFound(): void { - if (this.#errors.hasErrors()) throw this.#errors; + if (this.hasErrors()) throw this.#errors; } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/basic.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/basic.expect.md index a5bf42de7b008..8d8bc179a245a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/basic.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/basic.expect.md @@ -21,7 +21,7 @@ function Component(props) { ## Code ```javascript -import { useFire } from "react"; +import { useFire } from "react/compiler-runtime"; import { c as _c } from "react/compiler-runtime"; // @enableFire import { fire } from "react"; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/deep-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/deep-scope.expect.md index 585a820b13416..a335fea8867b9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/deep-scope.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/deep-scope.expect.md @@ -30,7 +30,7 @@ function Component(props) { ## Code ```javascript -import { useFire } from "react"; +import { useFire } from "react/compiler-runtime"; import { c as _c } from "react/compiler-runtime"; // @enableFire import { fire } from "react"; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-mix-fire-and-no-fire.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-mix-fire-and-no-fire.expect.md new file mode 100644 index 0000000000000..e73451a896ee4 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-mix-fire-and-no-fire.expect.md @@ -0,0 +1,39 @@ + +## Input + +```javascript +// @enableFire +import {fire} from 'react'; + +function Component(props) { + const foo = props => { + console.log(props); + }; + useEffect(() => { + function nested() { + fire(foo(props)); + foo(props); + } + + nested(); + }); + + return null; +} + +``` + + +## Error + +``` + 9 | function nested() { + 10 | fire(foo(props)); +> 11 | foo(props); + | ^^^ InvalidReact: Cannot compile `fire`. All uses of foo must be either used with a fire() call in this effect or not used with a fire() call at all. foo was used with fire() on line 10:10 in this effect (11:11) + 12 | } + 13 | + 14 | nested(); +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-mix-fire-and-no-fire.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-mix-fire-and-no-fire.js new file mode 100644 index 0000000000000..ee2f915a34ed5 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-mix-fire-and-no-fire.js @@ -0,0 +1,18 @@ +// @enableFire +import {fire} from 'react'; + +function Component(props) { + const foo = props => { + console.log(props); + }; + useEffect(() => { + function nested() { + fire(foo(props)); + foo(props); + } + + nested(); + }); + + return null; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-outside-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-outside-effect.expect.md new file mode 100644 index 0000000000000..687a21f98cdb4 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-outside-effect.expect.md @@ -0,0 +1,38 @@ + +## Input + +```javascript +// @enableFire +import {fire, useCallback} from 'react'; + +function Component({props, bar}) { + const foo = () => { + console.log(props); + }; + fire(foo(props)); + + useCallback(() => { + fire(foo(props)); + }, [foo, props]); + + return null; +} + +``` + + +## Error + +``` + 6 | console.log(props); + 7 | }; +> 8 | fire(foo(props)); + | ^^^^ Invariant: Cannot compile `fire`. Cannot use `fire` outside of a useEffect function (8:8) + +Invariant: Cannot compile `fire`. Cannot use `fire` outside of a useEffect function (11:11) + 9 | + 10 | useCallback(() => { + 11 | fire(foo(props)); +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-outside-effect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-outside-effect.js new file mode 100644 index 0000000000000..8ac9be6d7648f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-outside-effect.js @@ -0,0 +1,15 @@ +// @enableFire +import {fire, useCallback} from 'react'; + +function Component({props, bar}) { + const foo = () => { + console.log(props); + }; + fire(foo(props)); + + useCallback(() => { + fire(foo(props)); + }, [foo, props]); + + return null; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/multiple-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/multiple-scope.expect.md index 0d531041355be..02f3935171253 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/multiple-scope.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/multiple-scope.expect.md @@ -29,7 +29,7 @@ function Component(props) { ## Code ```javascript -import { useFire } from "react"; +import { useFire } from "react/compiler-runtime"; import { c as _c } from "react/compiler-runtime"; // @enableFire import { fire } from "react"; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repeated-calls.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repeated-calls.expect.md index 3eb9f0e71c71a..1734ca3ab4584 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repeated-calls.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repeated-calls.expect.md @@ -22,7 +22,7 @@ function Component(props) { ## Code ```javascript -import { useFire } from "react"; +import { useFire } from "react/compiler-runtime"; import { c as _c } from "react/compiler-runtime"; // @enableFire import { fire } from "react"; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/shared-hook-calls.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/shared-hook-calls.expect.md index 4362a3a8461e6..9b689b31c7ba0 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/shared-hook-calls.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/shared-hook-calls.expect.md @@ -26,7 +26,7 @@ function Component({bar, baz}) { ## Code ```javascript -import { useFire } from "react"; +import { useFire } from "react/compiler-runtime"; import { c as _c } from "react/compiler-runtime"; // @enableFire import { fire } from "react"; From 6907aa2a309bdc47dc3504683159cb50b590eed8 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 20 Dec 2024 17:16:59 -0500 Subject: [PATCH 4/4] [compiler] Rewrite effect dep arrays that use fire (#31811) If an effect uses a dep array, also rewrite the dep array to use the fire binding -- --- .../src/Transform/TransformFire.ts | 73 +++++++++++++++++-- ...id-rewrite-deps-no-array-literal.expect.md | 37 ++++++++++ ...r.invalid-rewrite-deps-no-array-literal.js | 16 ++++ ...rror.invalid-rewrite-deps-spread.expect.md | 37 ++++++++++ .../error.invalid-rewrite-deps-spread.js | 19 +++++ .../fire-and-autodeps.expect.md | 60 +++++++++++++++ .../transform-fire/fire-and-autodeps.js | 13 ++++ .../transform-fire/rewrite-deps.expect.md | 57 +++++++++++++++ .../compiler/transform-fire/rewrite-deps.js | 13 ++++ 9 files changed, 320 insertions(+), 5 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-rewrite-deps-no-array-literal.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-rewrite-deps-no-array-literal.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-rewrite-deps-spread.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-rewrite-deps-spread.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/fire-and-autodeps.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/fire-and-autodeps.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/rewrite-deps.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/rewrite-deps.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Transform/TransformFire.ts b/compiler/packages/babel-plugin-react-compiler/src/Transform/TransformFire.ts index a0256fd80cd66..a35c4ddb0182c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Transform/TransformFire.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Transform/TransformFire.ts @@ -12,6 +12,7 @@ import { SourceLocation, } from '..'; import { + ArrayExpression, CallExpression, Effect, Environment, @@ -38,7 +39,6 @@ import {printSourceLocationLine} from '../HIR/PrintHIR'; /* * TODO(jmbrown): - * - rewrite dep arrays * - traverse object methods * - method calls * - React.useEffect calls @@ -125,11 +125,58 @@ function replaceFireFunctions(fn: HIRFunction, context: Context): void { } rewriteInstrs.set(loadUseEffectInstrId, newInstrs); } - ensureNoRemainingCalleeCaptures( - lambda.loweredFunc.func, - context, - capturedCallees, + } + ensureNoRemainingCalleeCaptures( + lambda.loweredFunc.func, + context, + capturedCallees, + ); + + if ( + value.args.length > 1 && + value.args[1] != null && + value.args[1].kind === 'Identifier' + ) { + const depArray = value.args[1]; + const depArrayExpression = context.getArrayExpression( + depArray.identifier.id, ); + if (depArrayExpression != null) { + for (const dependency of depArrayExpression.elements) { + if (dependency.kind === 'Identifier') { + const loadOfDependency = context.getLoadLocalInstr( + dependency.identifier.id, + ); + if (loadOfDependency != null) { + const replacedDepArrayItem = capturedCallees.get( + loadOfDependency.place.identifier.id, + ); + if (replacedDepArrayItem != null) { + loadOfDependency.place = + replacedDepArrayItem.fireFunctionBinding; + } + } + } + } + } else { + context.pushError({ + loc: value.args[1].loc, + description: + 'You must use an array literal for an effect dependency array when that effect uses `fire()`', + severity: ErrorSeverity.Invariant, + reason: CANNOT_COMPILE_FIRE, + suggestions: null, + }); + } + } else if (value.args.length > 1 && value.args[1].kind === 'Spread') { + context.pushError({ + loc: value.args[1].place.loc, + description: + 'You must use an array literal for an effect dependency array when that effect uses `fire()`', + severity: ErrorSeverity.Invariant, + reason: CANNOT_COMPILE_FIRE, + suggestions: null, + }); } } } else if ( @@ -231,6 +278,8 @@ function replaceFireFunctions(fn: HIRFunction, context: Context): void { deleteInstrs.add(instr.id); } else if (value.kind === 'LoadGlobal') { context.addLoadGlobalInstrId(lvalue.identifier.id, instr.id); + } else if (value.kind === 'ArrayExpression') { + context.addArrayExpression(lvalue.identifier.id, value); } } block.instructions = rewriteInstructions(rewriteInstrs, block.instructions); @@ -561,6 +610,12 @@ class Context { this.#env = env; } + /* + * We keep track of array expressions so we can rewrite dependency arrays passed to useEffect + * to use the fire functions + */ + #arrayExpressions = new Map(); + pushError(error: CompilerErrorDetailOptions): void { this.#errors.push(error); } @@ -655,6 +710,14 @@ class Context { return this.#loadGlobalInstructionIds.get(id); } + addArrayExpression(id: IdentifierId, array: ArrayExpression): void { + this.#arrayExpressions.set(id, array); + } + + getArrayExpression(id: IdentifierId): ArrayExpression | undefined { + return this.#arrayExpressions.get(id); + } + hasErrors(): boolean { return this.#errors.hasErrors(); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-rewrite-deps-no-array-literal.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-rewrite-deps-no-array-literal.expect.md new file mode 100644 index 0000000000000..dcd9312bb2e53 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-rewrite-deps-no-array-literal.expect.md @@ -0,0 +1,37 @@ + +## Input + +```javascript +// @enableFire +import {fire} from 'react'; + +function Component(props) { + const foo = props => { + console.log(props); + }; + + const deps = [foo, props]; + + useEffect(() => { + fire(foo(props)); + }, deps); + + return null; +} + +``` + + +## Error + +``` + 11 | useEffect(() => { + 12 | fire(foo(props)); +> 13 | }, deps); + | ^^^^ Invariant: Cannot compile `fire`. You must use an array literal for an effect dependency array when that effect uses `fire()` (13:13) + 14 | + 15 | return null; + 16 | } +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-rewrite-deps-no-array-literal.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-rewrite-deps-no-array-literal.js new file mode 100644 index 0000000000000..b82f735425efa --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-rewrite-deps-no-array-literal.js @@ -0,0 +1,16 @@ +// @enableFire +import {fire} from 'react'; + +function Component(props) { + const foo = props => { + console.log(props); + }; + + const deps = [foo, props]; + + useEffect(() => { + fire(foo(props)); + }, deps); + + return null; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-rewrite-deps-spread.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-rewrite-deps-spread.expect.md new file mode 100644 index 0000000000000..7c1b55f61d909 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-rewrite-deps-spread.expect.md @@ -0,0 +1,37 @@ + +## Input + +```javascript +// @enableFire +import {fire} from 'react'; + +function Component(props) { + const foo = props => { + console.log(props); + }; + + const deps = [foo, props]; + + useEffect(() => { + fire(foo(props)); + }, ...deps); + + return null; +} + +``` + + +## Error + +``` + 11 | useEffect(() => { + 12 | fire(foo(props)); +> 13 | }, ...deps); + | ^^^^ Invariant: Cannot compile `fire`. You must use an array literal for an effect dependency array when that effect uses `fire()` (13:13) + 14 | + 15 | return null; + 16 | } +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-rewrite-deps-spread.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-rewrite-deps-spread.js new file mode 100644 index 0000000000000..27d1de4f463d6 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-rewrite-deps-spread.js @@ -0,0 +1,19 @@ +// @enableFire +import {fire} from 'react'; + +function Component(props) { + const foo = props => { + console.log(props); + }; + + const deps = [foo, props]; + + useEffect( + () => { + fire(foo(props)); + }, + ...deps + ); + + return null; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/fire-and-autodeps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/fire-and-autodeps.expect.md new file mode 100644 index 0000000000000..5767ff0746c1b --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/fire-and-autodeps.expect.md @@ -0,0 +1,60 @@ + +## Input + +```javascript +// @enableFire @inferEffectDependencies +import {fire, useEffect} from 'react'; + +function Component(props) { + const foo = arg => { + console.log(arg, props.bar); + }; + useEffect(() => { + fire(foo(props)); + }); + + return null; +} + +``` + +## Code + +```javascript +import { useFire } from "react/compiler-runtime"; +import { c as _c } from "react/compiler-runtime"; // @enableFire @inferEffectDependencies +import { fire, useEffect } from "react"; + +function Component(props) { + const $ = _c(5); + let t0; + if ($[0] !== props.bar) { + t0 = (arg) => { + console.log(arg, props.bar); + }; + $[0] = props.bar; + $[1] = t0; + } else { + t0 = $[1]; + } + const foo = t0; + const t1 = useFire(foo); + let t2; + if ($[2] !== props || $[3] !== t1) { + t2 = () => { + t1(props); + }; + $[2] = props; + $[3] = t1; + $[4] = t2; + } else { + t2 = $[4]; + } + useEffect(t2, [t1, props]); + return null; +} + +``` + +### 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/transform-fire/fire-and-autodeps.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/fire-and-autodeps.js new file mode 100644 index 0000000000000..e2a0068a19067 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/fire-and-autodeps.js @@ -0,0 +1,13 @@ +// @enableFire @inferEffectDependencies +import {fire, useEffect} from 'react'; + +function Component(props) { + const foo = arg => { + console.log(arg, props.bar); + }; + useEffect(() => { + fire(foo(props)); + }); + + return null; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/rewrite-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/rewrite-deps.expect.md new file mode 100644 index 0000000000000..ae71f60393281 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/rewrite-deps.expect.md @@ -0,0 +1,57 @@ + +## Input + +```javascript +// @enableFire +import {fire} from 'react'; + +function Component(props) { + const foo = props => { + console.log(props); + }; + useEffect(() => { + fire(foo(props)); + }, [foo, props]); + + return null; +} + +``` + +## Code + +```javascript +import { useFire } from "react/compiler-runtime"; +import { c as _c } from "react/compiler-runtime"; // @enableFire +import { fire } from "react"; + +function Component(props) { + const $ = _c(4); + const foo = _temp; + const t0 = useFire(foo); + let t1; + let t2; + if ($[0] !== props || $[1] !== t0) { + t1 = () => { + t0(props); + }; + t2 = [t0, props]; + $[0] = props; + $[1] = t0; + $[2] = t1; + $[3] = t2; + } else { + t1 = $[2]; + t2 = $[3]; + } + useEffect(t1, t2); + return null; +} +function _temp(props_0) { + console.log(props_0); +} + +``` + +### 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/transform-fire/rewrite-deps.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/rewrite-deps.js new file mode 100644 index 0000000000000..ad1af704c1bcc --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/rewrite-deps.js @@ -0,0 +1,13 @@ +// @enableFire +import {fire} from 'react'; + +function Component(props) { + const foo = props => { + console.log(props); + }; + useEffect(() => { + fire(foo(props)); + }, [foo, props]); + + return null; +}