From 1c657517a7a5b1c55e4900bc3a8f83060737ea79 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Thu, 17 Apr 2025 15:27:22 -0400 Subject: [PATCH 1/2] [compiler][fire] Fire syntax changes --- .../src/Transform/TransformFire.ts | 174 ++++++++++++------ .../bailout-capitalized-fn-call.expect.md | 6 +- .../bailout-capitalized-fn-call.js | 6 +- .../bailout-eslint-suppressions.expect.md | 6 +- .../bailout-eslint-suppressions.js | 6 +- .../bailout-validate-preserve-memo.expect.md | 6 +- .../bailout-validate-preserve-memo.js | 6 +- .../bailout-validate-prop-write.expect.md | 2 +- .../bailout-validate-prop-write.js | 2 +- ...lout-validate-ref-current-access.expect.md | 4 +- .../bailout-validate-ref-current-access.js | 4 +- .../bailout-retry/error.todo-syntax.expect.md | 4 +- .../bailout-retry/error.todo-syntax.js | 2 +- .../bailout-retry/error.use-no-memo.expect.md | 6 +- ...-fire-todo-syntax-shouldnt-throw.expect.md | 2 +- .../no-fire-todo-syntax-shouldnt-throw.js | 2 +- ...ailout-validate-conditional-hook.expect.md | 2 +- .../bailout-validate-conditional-hook.js | 2 +- .../compiler/transform-fire/basic.expect.md | 2 +- .../fixtures/compiler/transform-fire/basic.js | 2 +- .../transform-fire/deep-scope.expect.md | 2 +- .../compiler/transform-fire/deep-scope.js | 2 +- ...ror.invalid-mix-fire-and-no-fire.expect.md | 4 +- .../error.invalid-mix-fire-and-no-fire.js | 2 +- .../error.invalid-multiple-args.expect.md | 6 +- .../error.invalid-multiple-args.js | 2 +- .../error.invalid-not-call.expect.md | 18 +- .../transform-fire/error.invalid-not-call.js | 4 +- .../error.invalid-outside-effect.expect.md | 8 +- .../error.invalid-outside-effect.js | 4 +- ...id-rewrite-deps-no-array-literal.expect.md | 4 +- ...r.invalid-rewrite-deps-no-array-literal.js | 2 +- ...rror.invalid-rewrite-deps-spread.expect.md | 4 +- .../error.invalid-rewrite-deps-spread.js | 2 +- .../error.invalid-spread.expect.md | 6 +- .../transform-fire/error.invalid-spread.js | 2 +- .../error.todo-method.expect.md | 6 +- .../transform-fire/error.todo-method.js | 2 +- .../fire-and-autodeps.expect.md | 2 +- .../transform-fire/fire-and-autodeps.js | 2 +- .../transform-fire/hook-guard.expect.md | 2 +- .../compiler/transform-fire/hook-guard.js | 2 +- .../transform-fire/multiple-scope.expect.md | 6 +- .../compiler/transform-fire/multiple-scope.js | 6 +- .../transform-fire/repeated-calls.expect.md | 4 +- .../compiler/transform-fire/repeated-calls.js | 4 +- ...ro-dont-add-hook-guards-on-retry.expect.md | 2 +- .../repro-dont-add-hook-guards-on-retry.js | 2 +- .../transform-fire/rewrite-deps.expect.md | 2 +- .../compiler/transform-fire/rewrite-deps.js | 2 +- .../shared-hook-calls.expect.md | 6 +- .../transform-fire/shared-hook-calls.js | 6 +- 52 files changed, 226 insertions(+), 148 deletions(-) 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 943b6b8eca..981455fc21 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Transform/TransformFire.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Transform/TransformFire.ts @@ -35,7 +35,7 @@ import { import {createTemporaryPlace, markInstructionIds} from '../HIR/HIRBuilder'; import {getOrInsertWith} from '../Utils/utils'; import {BuiltInFireId, DefaultNonmutatingHook} from '../HIR/ObjectShape'; -import {eachInstructionOperand} from '../HIR/visitors'; +import {eachCallArgument, eachInstructionOperand} from '../HIR/visitors'; import {printSourceLocationLine} from '../HIR/PrintHIR'; import {USE_FIRE_FUNCTION_NAME} from '../HIR/Environment'; @@ -50,6 +50,8 @@ const CANNOT_COMPILE_FIRE = 'Cannot compile `fire`'; export function transformFire(fn: HIRFunction): void { const context = new Context(fn.env); + ensureLinearConsumption(fn, context); + context.throwIfErrorsFound(); replaceFireFunctions(fn, context); if (!context.hasErrors()) { ensureNoMoreFireUses(fn, context); @@ -197,58 +199,47 @@ function replaceFireFunctions(fn: HIRFunction, context: Context): void { 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 found a fire(identifier)() call. We remove the `fire()` call and replace the + * identifier with a freshly generated fire function binding, leaving fireIdentifier(). + * 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())` + * We only allow fire to be called with an identifier: `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, - value.loc, - ); - - loadLocal.place = {...fireFunctionBinding}; - - // Delete the fire call expression - deleteInstrs.add(instr.id); - } else { + const callee = value.args[0]; + // TODO: This value must be captured by the useEffect lambda + const loadLocal = context.getLoadLocalInstr(callee.identifier.id); + if (loadLocal == null) { 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', + description: `fire() can only be called with an identifier as an argument, like fire(myFunction)(myArgument)`, severity: ErrorSeverity.InvalidReact, reason: CANNOT_COMPILE_FIRE, suggestions: null, }); + continue; } + + const fireFunctionBinding = context.getOrGenerateFireFunctionBinding( + loadLocal.place, + value.loc, + ); + + context.addFireCallResultToReplacedBinding( + instr.lvalue.identifier.id, + fireFunctionBinding, + ); + + loadLocal.place = {...fireFunctionBinding}; + + deleteInstrs.add(instr.id); } else { let description: string = - 'fire() can only take in a single call expression as an argument'; + 'fire() can only take in a single identifier as an argument'; if (value.args.length === 0) { description += ' but received none'; } else if (value.args.length > 1) { @@ -265,7 +256,12 @@ function replaceFireFunctions(fn: HIRFunction, context: Context): void { }); } } else if (value.kind === 'CallExpression') { - context.addCallExpression(lvalue.identifier.id, value); + const callToFire = context.getFireCallResultToReplacedBinding( + value.callee.identifier.id, + ); + if (callToFire != null) { + value.callee = callToFire; + } } else if ( value.kind === 'FunctionExpression' && context.inUseEffectLambda() @@ -415,6 +411,80 @@ function ensureNoMoreFireUses(fn: HIRFunction, context: Context): void { } } +/* + * Fire calls must be linearly consumed as the callee of another call expression. All + * of these usages are syntax errors: + * 1. fire(props); + * 2. f(fire(props)); + * 3 const fireFunction = fire(props); + * 4. fire(fire(props)); + * + * We run this check *before* we do the transform because it is much simpler to implement + * by ensuring that a fire call lvalue is only ever used as a callee. + */ +function ensureLinearConsumption(fn: HIRFunction, context: Context): void { + const fireCallLvalues = new Map(); + const consumedFireCallLvalues = new Set(); + const addInvalidUseError = (id: IdentifierId, loc: SourceLocation): void => { + consumedFireCallLvalues.add(id); + context.pushError({ + loc, + description: + '`fire()` expressions can only be called, like fire(myFunction)(myArguments)', + severity: ErrorSeverity.InvalidReact, + reason: CANNOT_COMPILE_FIRE, + suggestions: null, + }); + }; + for (const [, block] of fn.body.blocks) { + for (const instr of block.instructions) { + const {value, lvalue} = instr; + if (value.kind === 'CallExpression') { + if ( + value.callee.identifier.type.kind === 'Function' && + value.callee.identifier.type.shapeId === BuiltInFireId + ) { + fireCallLvalues.set(lvalue.identifier.id, lvalue.loc); + } else { + if (fireCallLvalues.has(value.callee.identifier.id)) { + consumedFireCallLvalues.add(value.callee.identifier.id); + } + } + for (const argPlace of eachCallArgument(value.args)) { + if (fireCallLvalues.has(argPlace.identifier.id)) { + addInvalidUseError(argPlace.identifier.id, argPlace.loc); + } + } + } else if ( + value.kind === 'FunctionExpression' || + value.kind === 'ObjectMethod' + ) { + ensureLinearConsumption(value.loweredFunc.func, context); + } else { + for (const place of eachInstructionOperand(instr)) { + if (fireCallLvalues.has(place.identifier.id)) { + addInvalidUseError(place.identifier.id, place.loc); + } + } + } + } + } + + // Ensure every fire call was consumed + for (const [fireCallId, loc] of fireCallLvalues.entries()) { + if (!consumedFireCallLvalues.has(fireCallId)) { + context.pushError({ + loc: loc, + description: + '`fire(myFunction)` will not do anything on its own, you need to call the result like `fire(myFunction)(myArgument)`', + severity: ErrorSeverity.InvalidReact, + reason: CANNOT_COMPILE_FIRE, + suggestions: null, + }); + } + } +} + function makeLoadUseFireInstruction( env: Environment, importedLoadUseFire: NonLocalImportSpecifier, @@ -524,12 +594,6 @@ class Context { #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 @@ -574,10 +638,20 @@ class Context { */ #loadGlobalInstructionIds = new Map(); + #fireCallResultsToReplacedBindings = new Map(); + constructor(env: Environment) { this.#env = env; } + addFireCallResultToReplacedBinding(id: IdentifierId, binding: Place): void { + this.#fireCallResultsToReplacedBindings.set(id, binding); + } + + getFireCallResultToReplacedBinding(id: IdentifierId): Place | undefined { + return this.#fireCallResultsToReplacedBindings.get(id); + } + /* * We keep track of array expressions so we can rewrite dependency arrays passed to useEffect * to use the fire functions @@ -608,14 +682,6 @@ class Context { 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); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-capitalized-fn-call.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-capitalized-fn-call.expect.md index 77bdd31275..67ab3a63a6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-capitalized-fn-call.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-capitalized-fn-call.expect.md @@ -11,9 +11,9 @@ function Component({prop1, bar}) { console.log(prop1); }; useEffect(() => { - fire(foo(prop1)); - fire(foo()); - fire(bar()); + fire(foo)(prop1); + fire(foo)(); + fire(bar)(); }); return CapitalizedCall(); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-capitalized-fn-call.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-capitalized-fn-call.js index 679945eaab..7623983e42 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-capitalized-fn-call.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-capitalized-fn-call.js @@ -7,9 +7,9 @@ function Component({prop1, bar}) { console.log(prop1); }; useEffect(() => { - fire(foo(prop1)); - fire(foo()); - fire(bar()); + fire(foo)(prop1); + fire(foo)(); + fire(bar)(); }); return CapitalizedCall(); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-eslint-suppressions.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-eslint-suppressions.expect.md index 30bd6d42e5..8ca83c15b4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-eslint-suppressions.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-eslint-suppressions.expect.md @@ -10,9 +10,9 @@ function Component({props, bar}) { console.log(props); }; useEffect(() => { - fire(foo(props)); - fire(foo()); - fire(bar()); + fire(foo)(props); + fire(foo)(); + fire(bar)(); }); const ref = useRef(null); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-eslint-suppressions.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-eslint-suppressions.js index 5312e5707c..22e77653a9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-eslint-suppressions.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-eslint-suppressions.js @@ -6,9 +6,9 @@ function Component({props, bar}) { console.log(props); }; useEffect(() => { - fire(foo(props)); - fire(foo()); - fire(bar()); + fire(foo)(props); + fire(foo)(); + fire(bar)(); }); const ref = useRef(null); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-preserve-memo.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-preserve-memo.expect.md index 6477a01126..92595129fa 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-preserve-memo.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-preserve-memo.expect.md @@ -11,9 +11,9 @@ function Component({prop1, bar}) { console.log(prop1); }; useEffect(() => { - fire(foo(prop1)); - fire(foo()); - fire(bar()); + fire(foo)(prop1); + fire(foo)(); + fire(bar)(); }); return useMemo(() => sum(bar), []); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-preserve-memo.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-preserve-memo.js index c3bb8b4216..e355456499 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-preserve-memo.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-preserve-memo.js @@ -7,9 +7,9 @@ function Component({prop1, bar}) { console.log(prop1); }; useEffect(() => { - fire(foo(prop1)); - fire(foo()); - fire(bar()); + fire(foo)(prop1); + fire(foo)(); + fire(bar)(); }); return useMemo(() => sum(bar), []); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-prop-write.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-prop-write.expect.md index e6ce051f10..14be751d40 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-prop-write.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-prop-write.expect.md @@ -10,7 +10,7 @@ function Component({prop1}) { console.log(prop1); }; useEffect(() => { - fire(foo(prop1)); + fire(foo)(prop1); }); prop1.value += 1; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-prop-write.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-prop-write.js index b0cfd4fe39..4a2797d8aa 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-prop-write.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-prop-write.js @@ -6,7 +6,7 @@ function Component({prop1}) { console.log(prop1); }; useEffect(() => { - fire(foo(prop1)); + fire(foo)(prop1); }); prop1.value += 1; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-ref-current-access.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-ref-current-access.expect.md index 79f5a2986d..05061d544b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-ref-current-access.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-ref-current-access.expect.md @@ -11,9 +11,9 @@ component Component(prop1, ref) { console.log(prop1); }; useEffect(() => { - fire(foo(prop1)); + fire(foo)(prop1); bar(); - fire(foo()); + fire(foo)(); }); print(ref.current); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-ref-current-access.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-ref-current-access.js index f649fe5902..b431367997 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-ref-current-access.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-ref-current-access.js @@ -7,9 +7,9 @@ component Component(prop1, ref) { console.log(prop1); }; useEffect(() => { - fire(foo(prop1)); + fire(foo)(prop1); bar(); - fire(foo()); + fire(foo)(); }); print(ref.current); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.expect.md index c5d7456d65..de75f46fee 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.expect.md @@ -19,7 +19,7 @@ function Component({prop1}) { } }; useEffect(() => { - fire(foo()); + fire(foo)(); }); } @@ -31,7 +31,7 @@ function Component({prop1}) { ``` 16 | }; 17 | useEffect(() => { -> 18 | fire(foo()); +> 18 | fire(foo)(); | ^^^^ InvalidReact: [Fire] Untransformed reference to compiler-required feature. Either remove this `fire` call or ensure it is successfully transformed by the compiler. (Bailout reason: Todo: (BuildHIR::lowerStatement) Handle TryStatement without a catch clause (11:15)) (18:18) 19 | }); 20 | } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.js index b1ee459177..d94c21d18b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.js @@ -15,6 +15,6 @@ function Component({prop1}) { } }; useEffect(() => { - fire(foo()); + fire(foo)(); }); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.use-no-memo.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.use-no-memo.expect.md index 84a27b43b8..453db0bca9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.use-no-memo.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.use-no-memo.expect.md @@ -33,7 +33,11 @@ function Component({props, bar}) { 13 | }; 14 | useEffect(() => { > 15 | fire(foo(props)); - | ^^^^ InvalidReact: [Fire] Untransformed reference to compiler-required feature. Either remove this `fire` call or ensure it is successfully transformed by the compiler (15:15) + | ^^^^ InvalidReact: [Fire] Untransformed reference to compiler-required feature. Either remove this `fire` call or ensure it is successfully transformed by the compiler. (Bailout reason: InvalidReact: Cannot compile `fire`. `fire(myFunction)` will not do anything on its own, you need to call the result like `fire(myFunction)(myArgument)` (15:15) + +InvalidReact: Cannot compile `fire`. `fire(myFunction)` will not do anything on its own, you need to call the result like `fire(myFunction)(myArgument)` (16:16) + +InvalidReact: Cannot compile `fire`. `fire(myFunction)` will not do anything on its own, you need to call the result like `fire(myFunction)(myArgument)` (17:17)) (15:15) 16 | fire(foo()); 17 | fire(bar()); 18 | }); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/no-fire-todo-syntax-shouldnt-throw.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/no-fire-todo-syntax-shouldnt-throw.expect.md index 7ba4ee2811..c749c0c54b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/no-fire-todo-syntax-shouldnt-throw.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/no-fire-todo-syntax-shouldnt-throw.expect.md @@ -32,7 +32,7 @@ function FireComponent(props) { 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/bailout-retry/no-fire-todo-syntax-shouldnt-throw.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/no-fire-todo-syntax-shouldnt-throw.js index 899fa33376..b12706e277 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/no-fire-todo-syntax-shouldnt-throw.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/no-fire-todo-syntax-shouldnt-throw.js @@ -28,7 +28,7 @@ function FireComponent(props) { 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/bailout-validate-conditional-hook.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-validate-conditional-hook.expect.md index dde2b692f4..890d46e5bb 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-validate-conditional-hook.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-validate-conditional-hook.expect.md @@ -17,7 +17,7 @@ function Component(props) { if (props.cond) { useEffect(() => { - fire(foo(props)); + fire(foo)(props); }); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-validate-conditional-hook.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-validate-conditional-hook.js index fa8c034bfb..d1002e78a6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-validate-conditional-hook.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-validate-conditional-hook.js @@ -13,7 +13,7 @@ function Component(props) { if (props.cond) { useEffect(() => { - fire(foo(props)); + fire(foo)(props); }); } 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 36146cff00..bfca9f4e42 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 @@ -10,7 +10,7 @@ function Component(props) { 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/basic.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/basic.js index 2f7a72e4ee..a1d86b3da2 100644 --- 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 @@ -6,7 +6,7 @@ function Component(props) { 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/deep-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/deep-scope.expect.md index de003f7007..f3ea9d978f 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 @@ -13,7 +13,7 @@ function Component(props) { function nested() { function nestedAgain() { function nestedThrice() { - fire(foo(props)); + fire(foo)(props); } nestedThrice(); } 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 index b056c3f53a..f92250473f 100644 --- 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 @@ -9,7 +9,7 @@ function Component(props) { function nested() { function nestedAgain() { function nestedThrice() { - fire(foo(props)); + fire(foo)(props); } nestedThrice(); } 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 index e73451a896..54b12f9a0f 100644 --- 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 @@ -11,7 +11,7 @@ function Component(props) { }; useEffect(() => { function nested() { - fire(foo(props)); + fire(foo)(props); foo(props); } @@ -28,7 +28,7 @@ function Component(props) { ``` 9 | function nested() { - 10 | fire(foo(props)); + 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 | } 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 index ee2f915a34..d122d21195 100644 --- 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 @@ -7,7 +7,7 @@ function Component(props) { }; useEffect(() => { function nested() { - fire(foo(props)); + fire(foo)(props); foo(props); } 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 index 8329717cb3..d94ec58c3f 100644 --- 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 @@ -10,7 +10,7 @@ function Component({bar, baz}) { console.log(bar, baz); }; useEffect(() => { - fire(foo(bar), baz); + fire(foo(bar), baz)(); }); return null; @@ -24,8 +24,8 @@ function Component({bar, baz}) { ``` 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) +> 9 | fire(foo(bar), baz)(); + | ^^^^^^^^^^^^^^^^^^^ InvalidReact: Cannot compile `fire`. fire() can only take in a single identifier as an argument but received multiple arguments (9:9) 10 | }); 11 | 12 | return null; 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 index 980b0dfcb5..38bd773613 100644 --- 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 @@ -6,7 +6,7 @@ function Component({bar, baz}) { console.log(bar, baz); }; useEffect(() => { - fire(foo(bar), baz); + fire(foo(bar), baz)(); }); 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 index 855c7b7d70..e05a6a3bf3 100644 --- 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 @@ -10,7 +10,9 @@ function Component(props) { console.log(props); }; useEffect(() => { - fire(props); + foo(fire(props)); // Can't be used as a function argument + const stored = fire(foo); // Cannot be assigned + fire(props); // Invalid as an expression statement }); return null; @@ -24,11 +26,15 @@ function Component(props) { ``` 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; +> 9 | foo(fire(props)); // Can't be used as a function argument + | ^^^^^^^^^^^ InvalidReact: Cannot compile `fire`. `fire()` expressions can only be called, like fire(myFunction)(myArguments) (9:9) + +InvalidReact: Cannot compile `fire`. `fire()` expressions can only be called, like fire(myFunction)(myArguments) (10:10) + +InvalidReact: Cannot compile `fire`. `fire(myFunction)` will not do anything on its own, you need to call the result like `fire(myFunction)(myArgument)` (11:11) + 10 | const stored = fire(foo); // Cannot be assigned + 11 | fire(props); // Invalid as an expression statement + 12 | }); ``` \ 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 index 3d1ae3658f..6edf9842a2 100644 --- 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 @@ -6,7 +6,9 @@ function Component(props) { console.log(props); }; useEffect(() => { - fire(props); + foo(fire(props)); // Can't be used as a function argument + const stored = fire(foo); // Cannot be assigned + fire(props); // Invalid as an expression statement }); 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 index 687a21f98c..ddd76a4073 100644 --- 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 @@ -9,10 +9,10 @@ function Component({props, bar}) { const foo = () => { console.log(props); }; - fire(foo(props)); + fire(foo)(props); useCallback(() => { - fire(foo(props)); + fire(foo)(props); }, [foo, props]); return null; @@ -26,13 +26,13 @@ function Component({props, bar}) { ``` 6 | console.log(props); 7 | }; -> 8 | fire(foo(props)); +> 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)); + 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 index 8ac9be6d76..21b2c326a8 100644 --- 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 @@ -5,10 +5,10 @@ function Component({props, bar}) { const foo = () => { console.log(props); }; - fire(foo(props)); + fire(foo)(props); useCallback(() => { - fire(foo(props)); + fire(foo)(props); }, [foo, props]); return null; 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 index dcd9312bb2..3c81d3f0c0 100644 --- 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 @@ -13,7 +13,7 @@ function Component(props) { const deps = [foo, props]; useEffect(() => { - fire(foo(props)); + fire(foo)(props); }, deps); return null; @@ -26,7 +26,7 @@ function Component(props) { ``` 11 | useEffect(() => { - 12 | fire(foo(props)); + 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 | 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 index b82f735425..7a4823b865 100644 --- 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 @@ -9,7 +9,7 @@ function Component(props) { const deps = [foo, props]; useEffect(() => { - fire(foo(props)); + 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 index 91c5523564..2b5bd2232f 100644 --- 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 @@ -14,7 +14,7 @@ function Component(props) { useEffect( () => { - fire(foo(props)); + fire(foo)(props); }, ...deps ); @@ -28,7 +28,7 @@ function Component(props) { ## Error ``` - 13 | fire(foo(props)); + 13 | fire(foo)(props); 14 | }, > 15 | ...deps | ^^^^ Invariant: Cannot compile `fire`. You must use an array literal for an effect dependency array when that effect uses `fire()` (15:15) 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 index 27d1de4f46..66050fed3f 100644 --- 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 @@ -10,7 +10,7 @@ function Component(props) { useEffect( () => { - fire(foo(props)); + fire(foo)(props); }, ...deps ); 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 index c0b797fc14..a0d95347b1 100644 --- 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 @@ -10,7 +10,7 @@ function Component(props) { console.log(props); }; useEffect(() => { - fire(...foo); + fire(...foo)(); }); return null; @@ -24,8 +24,8 @@ function Component(props) { ``` 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) +> 9 | fire(...foo)(); + | ^^^^^^^^^^^^ InvalidReact: Cannot compile `fire`. fire() can only take in a single identifier as an argument but received a spread argument (9:9) 10 | }); 11 | 12 | return null; 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 index 68e317588b..c3f98888ba 100644 --- 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 @@ -6,7 +6,7 @@ function Component(props) { console.log(props); }; useEffect(() => { - fire(...foo); + 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 index 3f237cfc6f..9ee4c17a58 100644 --- 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 @@ -10,7 +10,7 @@ function Component(props) { console.log(props); }; useEffect(() => { - fire(props.foo()); + fire(props.foo)(); }); return null; @@ -24,8 +24,8 @@ function Component(props) { ``` 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) +> 9 | fire(props.foo)(); + | ^^^^^^^^^^^^^^^ InvalidReact: Cannot compile `fire`. fire() can only be called with an identifier as an argument, like fire(myFunction)(myArgument) (9:9) 10 | }); 11 | 12 | return null; 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 index c75622ca5e..7095d1ced4 100644 --- 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 @@ -6,7 +6,7 @@ function Component(props) { console.log(props); }; useEffect(() => { - fire(props.foo()); + fire(props.foo)(); }); 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 index 20260bd5e6..155b56b02d 100644 --- 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 @@ -10,7 +10,7 @@ function Component(props) { console.log(arg, props.bar); }; useEffect(() => { - fire(foo(props)); + fire(foo)(props); }); return null; 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 index e2a0068a19..e4ce5938f5 100644 --- 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 @@ -6,7 +6,7 @@ function Component(props) { console.log(arg, props.bar); }; useEffect(() => { - fire(foo(props)); + fire(foo)(props); }); return null; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/hook-guard.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/hook-guard.expect.md index d94cce5588..8ef80394cb 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/hook-guard.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/hook-guard.expect.md @@ -10,7 +10,7 @@ function Component(props) { 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/hook-guard.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/hook-guard.js index bc0b1a2d7c..02d174afa3 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/hook-guard.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/hook-guard.js @@ -6,7 +6,7 @@ function Component(props) { 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/multiple-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/multiple-scope.expect.md index 796c4397ee..cf952c3798 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 @@ -10,11 +10,11 @@ function Component(props) { console.log(props); }; useEffect(() => { - fire(foo(props)); + fire(foo)(props); function nested() { - fire(foo(props)); + fire(foo)(props); function innerNested() { - fire(foo(props)); + fire(foo)(props); } } 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 index 54410680e6..01febf07b3 100644 --- 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 @@ -6,11 +6,11 @@ function Component(props) { console.log(props); }; useEffect(() => { - fire(foo(props)); + fire(foo)(props); function nested() { - fire(foo(props)); + fire(foo)(props); function innerNested() { - fire(foo(props)); + fire(foo)(props); } } 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 e528823550..0090a47326 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 @@ -10,8 +10,8 @@ function Component(props) { console.log(props); }; useEffect(() => { - fire(foo(props)); - fire(foo(props)); + fire(foo)(props); + fire(foo)(props); }); return null; 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 index 14e1cb06b1..9f3a5f58d1 100644 --- 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 @@ -6,8 +6,8 @@ function Component(props) { console.log(props); }; useEffect(() => { - fire(foo(props)); - fire(foo(props)); + fire(foo)(props); + fire(foo)(props); }); return null; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repro-dont-add-hook-guards-on-retry.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repro-dont-add-hook-guards-on-retry.expect.md index c7ed50ceba..ddb562248f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repro-dont-add-hook-guards-on-retry.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repro-dont-add-hook-guards-on-retry.expect.md @@ -12,7 +12,7 @@ function Component(props, useDynamicHook) { console.log(props); }; useEffect(() => { - fire(foo(props)); + fire(foo)(props); }); return
hello world
; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repro-dont-add-hook-guards-on-retry.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repro-dont-add-hook-guards-on-retry.js index 077982e8d4..bc206ec630 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repro-dont-add-hook-guards-on-retry.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repro-dont-add-hook-guards-on-retry.js @@ -8,7 +8,7 @@ function Component(props, useDynamicHook) { console.log(props); }; useEffect(() => { - fire(foo(props)); + fire(foo)(props); }); return
hello world
; 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 index e569536ad3..63a414de04 100644 --- 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 @@ -10,7 +10,7 @@ function Component(props) { console.log(props); }; useEffect(() => { - fire(foo(props)); + fire(foo)(props); }, [foo, props]); return null; 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 index ad1af704c1..c2751c7a73 100644 --- 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 @@ -6,7 +6,7 @@ function Component(props) { console.log(props); }; useEffect(() => { - fire(foo(props)); + fire(foo)(props); }, [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 index 92dbf9843a..fda9498dfb 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 @@ -10,12 +10,12 @@ function Component({bar, baz}) { console.log(bar); }; useEffect(() => { - fire(foo(bar)); - fire(baz(bar)); + fire(foo)(bar); + fire(baz)(bar); }); useEffect(() => { - fire(foo(bar)); + fire(foo)(bar); }); return null; 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 index 5cb51e9bd3..de66da02ca 100644 --- 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 @@ -6,12 +6,12 @@ function Component({bar, baz}) { console.log(bar); }; useEffect(() => { - fire(foo(bar)); - fire(baz(bar)); + fire(foo)(bar); + fire(baz)(bar); }); useEffect(() => { - fire(foo(bar)); + fire(foo)(bar); }); return null; From 8c6158ad9931ea62510f0782085e9d1c5313521d Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Thu, 17 Apr 2025 15:27:27 -0400 Subject: [PATCH 2/2] [compiler][fire] Only allow values captured by the effect to be fired --- .../src/Transform/TransformFire.ts | 49 ++++++++++++++----- .../error.invalid-capture.expect.md | 34 +++++++++++++ .../transform-fire/error.invalid-capture.js | 13 +++++ 3 files changed, 85 insertions(+), 11 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-capture.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-capture.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 981455fc21..e7c4bcf050 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Transform/TransformFire.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Transform/TransformFire.ts @@ -211,8 +211,9 @@ function replaceFireFunctions(fn: HIRFunction, context: Context): void { */ if (value.args.length === 1 && value.args[0].kind === 'Identifier') { const callee = value.args[0]; - // TODO: This value must be captured by the useEffect lambda - const loadLocal = context.getLoadLocalInstr(callee.identifier.id); + const calleeId = callee.identifier.id; + + const loadLocal = context.getLoadLocalInstr(calleeId); if (loadLocal == null) { context.pushError({ loc: value.loc, @@ -224,6 +225,11 @@ function replaceFireFunctions(fn: HIRFunction, context: Context): void { continue; } + context.errorIfNotCapturedFromComponentScope( + loadLocal.place.identifier.id, + callee.loc, + ); + const fireFunctionBinding = context.getOrGenerateFireFunctionBinding( loadLocal.place, value.loc, @@ -318,13 +324,10 @@ function visitFunctionExpressionAndPropagateFireDependencies( context: Context, enteringUseEffect: boolean, ): FireCalleesToFireFunctionBinding { - let withScope = enteringUseEffect - ? context.withUseEffectLambdaScope.bind(context) - : context.withFunctionScope.bind(context); - - const calleesCapturedByFnExpression = withScope(() => - replaceFireFunctions(fnExpr.loweredFunc.func, context), - ); + const visitFn = (): void => replaceFireFunctions(fnExpr.loweredFunc.func, context); + const calleesCapturedByFnExpression = enteringUseEffect + ? context.withUseEffectLambdaScope(fnExpr, visitFn) + : context.withFunctionScope(visitFn); // For each replaced callee, update the context of the function expression to track it for ( @@ -631,6 +634,8 @@ class Context { */ #inUseEffectLambda = false; + #identifierIdsCapturedByEffectLambda = new Set(); + /* * 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 @@ -667,17 +672,23 @@ class Context { return this.#capturedCalleeIdentifierIds; } - withUseEffectLambdaScope(fn: () => void): FireCalleesToFireFunctionBinding { + withUseEffectLambdaScope( + lambda: FunctionExpression, + fn: () => void, + ): FireCalleesToFireFunctionBinding { const capturedCalleeIdentifierIds = this.#capturedCalleeIdentifierIds; const inUseEffectLambda = this.#inUseEffectLambda; this.#capturedCalleeIdentifierIds = new Map(); this.#inUseEffectLambda = true; - + this.#identifierIdsCapturedByEffectLambda = new Set( + lambda.loweredFunc.func.context.map(dep => dep.identifier.id), + ); const resultCapturedCalleeIdentifierIds = this.withFunctionScope(fn); this.#capturedCalleeIdentifierIds = capturedCalleeIdentifierIds; this.#inUseEffectLambda = inUseEffectLambda; + this.#identifierIdsCapturedByEffectLambda = new Set(); return resultCapturedCalleeIdentifierIds; } @@ -752,6 +763,22 @@ class Context { return this.#arrayExpressions.get(id); } + errorIfNotCapturedFromComponentScope( + id: IdentifierId, + loc: SourceLocation, + ): void { + if (!this.#identifierIdsCapturedByEffectLambda.has(id)) { + this.pushError({ + loc, + description: + '`fire()` only accepts identifiers defined in the component/hook scope. This value was defined in the useEffect callback.', + severity: ErrorSeverity.InvalidReact, + reason: CANNOT_COMPILE_FIRE, + suggestions: null, + }); + } + } + hasErrors(): boolean { return this.#errors.hasErrors(); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-capture.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-capture.expect.md new file mode 100644 index 0000000000..e843f79a01 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-capture.expect.md @@ -0,0 +1,34 @@ + +## Input + +```javascript +// @enableFire +import {fire} from 'react'; + +function Component(props) { + useEffect(() => { + const log = () => { + console.log(props); + }; + fire(log)(); + }); + + return null; +} + +``` + + +## Error + +``` + 7 | console.log(props); + 8 | }; +> 9 | fire(log)(); + | ^^^ InvalidReact: Cannot compile `fire`. `fire()` only accepts identifiers defined in the component/hook scope. This value was defined in the useEffect callback. (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-capture.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-capture.js new file mode 100644 index 0000000000..912e17bd3b --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/error.invalid-capture.js @@ -0,0 +1,13 @@ +// @enableFire +import {fire} from 'react'; + +function Component(props) { + useEffect(() => { + const log = () => { + console.log(props); + }; + fire(log)(); + }); + + return null; +}