From 88bf4e197a326c4490c328ae111d7366fd4b0f34 Mon Sep 17 00:00:00 2001
From: Lauren Tan <poteto@users.noreply.github.com>
Date: Thu, 25 Jul 2024 15:18:14 -0400
Subject: [PATCH] [compiler] Always emit FuncDecl for outlined functions

Addresses follow up feedback from #30446. Since the outlined function is
guaranteed to have a module-scoped unique identifier name, we can
simplify the insertion logic for the outlined function to always emit a
function declaration rather than switch based on the original function
type. This is fine because the outlined function is synthetic anyway.

ghstack-source-id: 0a4d1f7b0a5a34f8ed68c463b1a153c51c3ea75e
Pull Request resolved: https://github.com/facebook/react/pull/30464
---
 .../src/Entrypoint/Program.ts                 | 62 +++++++------------
 .../compiler/outlining-in-func-expr.expect.md |  4 +-
 2 files changed, 26 insertions(+), 40 deletions(-)

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 18a6c6806f..1aabb1acca 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts
+++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts
@@ -195,7 +195,7 @@ export function createNewFunctionNode(
   return transformedFn;
 }
 
-function insertNewFunctionNode(
+function insertNewOutlinedFunctionNode(
   originalFn: BabelFn,
   compiledFn: CodegenFunction,
 ): NodePath<t.Function> {
@@ -206,30 +206,16 @@ function insertNewFunctionNode(
       )[0]!;
     }
     /**
-     * This is pretty gross but we can't just append an (Arrow)FunctionExpression as a sibling of
-     * the original function. If the original function was itself an (Arrow)FunctionExpression,
-     * this would cause its parent to become a SequenceExpression instead which breaks a bunch of
-     * assumptions elsewhere in the plugin.
+     * We can't just append the outlined function as a sibling of the original function if it is an
+     * (Arrow)FunctionExpression parented by a VariableDeclaration, as this would cause its parent
+     * to become a SequenceExpression instead which breaks a bunch of assumptions elsewhere in the
+     * plugin.
      *
-     * To get around this, we synthesize a new VariableDeclaration holding the compiled function
-     * expression and insert it as a true sibling (ie within the Program's block statements).
+     * To get around this, we always synthesize a new FunctionDeclaration for the outlined function
+     * and insert it as a true sibling to the original function.
      */
     case 'ArrowFunctionExpression':
     case 'FunctionExpression': {
-      const funcExpr = createNewFunctionNode(originalFn, compiledFn);
-      CompilerError.invariant(
-        t.isArrowFunctionExpression(funcExpr) ||
-          t.isFunctionExpression(funcExpr),
-        {
-          reason: 'Expected an (arrow) function expression to be created',
-          description: `Got: ${funcExpr.type}`,
-          loc: funcExpr.loc ?? null,
-        },
-      );
-      CompilerError.invariant(compiledFn.id != null, {
-        reason: 'Expected compiled function to have an identifier',
-        loc: compiledFn.loc,
-      });
       CompilerError.invariant(
         originalFn.parentPath.isVariableDeclarator() &&
           originalFn.parentPath.parentPath.isVariableDeclaration(),
@@ -238,23 +224,23 @@ function insertNewFunctionNode(
           loc: originalFn.node.loc ?? null,
         },
       );
+      const fn: t.FunctionDeclaration = {
+        type: 'FunctionDeclaration',
+        id: compiledFn.id,
+        loc: originalFn.node.loc ?? null,
+        async: compiledFn.async,
+        generator: compiledFn.generator,
+        params: compiledFn.params,
+        body: compiledFn.body,
+      };
       const varDecl = originalFn.parentPath.parentPath;
-      varDecl.insertAfter(
-        t.variableDeclaration('const', [
-          t.variableDeclarator(compiledFn.id, funcExpr),
-        ]),
-      );
-      const insertedFuncExpr = varDecl.get('declarations')[0]!.get('init')!;
-      CompilerError.invariant(
-        insertedFuncExpr.isArrowFunctionExpression() ||
-          insertedFuncExpr.isFunctionExpression(),
-        {
-          reason: 'Expected inserted (arrow) function expression',
-          description: `Got: ${insertedFuncExpr}`,
-          loc: insertedFuncExpr.node?.loc ?? null,
-        },
-      );
-      return insertedFuncExpr;
+      const insertedFuncDecl = varDecl.insertAfter(fn)[0]!;
+      CompilerError.invariant(insertedFuncDecl.isFunctionDeclaration(), {
+        reason: 'Expected inserted function declaration',
+        description: `Got: ${insertedFuncDecl}`,
+        loc: insertedFuncDecl.node?.loc ?? null,
+      });
+      return insertedFuncDecl;
     }
     default: {
       assertExhaustive(
@@ -482,7 +468,7 @@ export function compileProgram(
         reason: 'Unexpected nested outlined functions',
         loc: outlined.fn.loc,
       });
-      const fn = insertNewFunctionNode(current.fn, outlined.fn);
+      const fn = insertNewOutlinedFunctionNode(current.fn, outlined.fn);
       fn.skip();
       ALREADY_COMPILED.add(fn.node);
       if (outlined.type !== null) {
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlining-in-func-expr.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlining-in-func-expr.expect.md
index 87c415002f..b403368e8a 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlining-in-func-expr.expect.md
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlining-in-func-expr.expect.md
@@ -50,9 +50,9 @@ const Component2 = (props) => {
   }
   return t1;
 };
-const _temp = (item) => {
+function _temp(item) {
   return <li key={item.id}>{item.name}</li>;
-};
+}
 
 export const FIXTURE_ENTRYPOINT = {
   fn: Component2,