@samitouri / QOS-React / commits / 88bf4e197a

[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

Lauren Tan committed Jul 25, 2024 at 15:18 UTC 88bf4e197a326c4490c328ae111d7366fd4b0f34
2 files changed +26 -40
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts
+24 -38
@@ -195,7 +195,7 @@ export function createNewFunctionNode(
195 return transformedFn;
196 }
197
198 -function insertNewFunctionNode(
198 +function insertNewOutlinedFunctionNode(
199 originalFn: BabelFn,
200 compiledFn: CodegenFunction,
201 ): NodePath<t.Function> {
@@ -206,30 +206,16 @@ function insertNewFunctionNode(
206 )[0]!;
207 }
208 /**
209 - * This is pretty gross but we can't just append an (Arrow)FunctionExpression as a sibling of
210 - * the original function. If the original function was itself an (Arrow)FunctionExpression,
211 - * this would cause its parent to become a SequenceExpression instead which breaks a bunch of
212 - * assumptions elsewhere in the plugin.
209 + * We can't just append the outlined function as a sibling of the original function if it is an
210 + * (Arrow)FunctionExpression parented by a VariableDeclaration, as this would cause its parent
211 + * to become a SequenceExpression instead which breaks a bunch of assumptions elsewhere in the
212 + * plugin.
213 *
214 - * To get around this, we synthesize a new VariableDeclaration holding the compiled function
215 - * expression and insert it as a true sibling (ie within the Program's block statements).
214 + * To get around this, we always synthesize a new FunctionDeclaration for the outlined function
215 + * and insert it as a true sibling to the original function.
216 */
217 case 'ArrowFunctionExpression':
218 case 'FunctionExpression': {
219 - const funcExpr = createNewFunctionNode(originalFn, compiledFn);
220 - CompilerError.invariant(
221 - t.isArrowFunctionExpression(funcExpr) ||
222 - t.isFunctionExpression(funcExpr),
223 - {
224 - reason: 'Expected an (arrow) function expression to be created',
225 - description: `Got: ${funcExpr.type}`,
226 - loc: funcExpr.loc ?? null,
227 - },
228 - );
229 - CompilerError.invariant(compiledFn.id != null, {
230 - reason: 'Expected compiled function to have an identifier',
231 - loc: compiledFn.loc,
232 - });
219 CompilerError.invariant(
220 originalFn.parentPath.isVariableDeclarator() &&
221 originalFn.parentPath.parentPath.isVariableDeclaration(),
@@ -238,23 +224,23 @@ function insertNewFunctionNode(
224 loc: originalFn.node.loc ?? null,
225 },
226 );
227 + const fn: t.FunctionDeclaration = {
228 + type: 'FunctionDeclaration',
229 + id: compiledFn.id,
230 + loc: originalFn.node.loc ?? null,
231 + async: compiledFn.async,
232 + generator: compiledFn.generator,
233 + params: compiledFn.params,
234 + body: compiledFn.body,
235 + };
236 const varDecl = originalFn.parentPath.parentPath;
242 - varDecl.insertAfter(
243 - t.variableDeclaration('const', [
244 - t.variableDeclarator(compiledFn.id, funcExpr),
245 - ]),
246 - );
247 - const insertedFuncExpr = varDecl.get('declarations')[0]!.get('init')!;
248 - CompilerError.invariant(
249 - insertedFuncExpr.isArrowFunctionExpression() ||
250 - insertedFuncExpr.isFunctionExpression(),
251 - {
252 - reason: 'Expected inserted (arrow) function expression',
253 - description: `Got: ${insertedFuncExpr}`,
254 - loc: insertedFuncExpr.node?.loc ?? null,
255 - },
256 - );
257 - return insertedFuncExpr;
237 + const insertedFuncDecl = varDecl.insertAfter(fn)[0]!;
238 + CompilerError.invariant(insertedFuncDecl.isFunctionDeclaration(), {
239 + reason: 'Expected inserted function declaration',
240 + description: `Got: ${insertedFuncDecl}`,
241 + loc: insertedFuncDecl.node?.loc ?? null,
242 + });
243 + return insertedFuncDecl;
244 }
245 default: {
246 assertExhaustive(
@@ -482,7 +468,7 @@ export function compileProgram(
468 reason: 'Unexpected nested outlined functions',
469 loc: outlined.fn.loc,
470 });
485 - const fn = insertNewFunctionNode(current.fn, outlined.fn);
471 + const fn = insertNewOutlinedFunctionNode(current.fn, outlined.fn);
472 fn.skip();
473 ALREADY_COMPILED.add(fn.node);
474 if (outlined.type !== null) {
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlining-in-func-expr.expect.md
+2 -2
@@ -50,9 +50,9 @@ const Component2 = (props) => {
50 }
51 return t1;
52 };
53 -const _temp = (item) => {
53 +function _temp(item) {
54 return <li key={item.id}>{item.name}</li>;
55 -};
55 +}
56
57 export const FIXTURE_ENTRYPOINT = {
58 fn: Component2,