From a6b7e438ca29aa65263d19284d73c1a2e6b11a3b Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Wed, 24 Jul 2024 15:57:04 -0400 Subject: [PATCH] [compiler] Correctly insert (Arrow)FunctionExpressions Previously we would insert new (Arrow)FunctionExpressions as a sibling of the original function. However this would break in the outlining case as it would cause the original function expression's parent to become a SequenceExpression, breaking a bunch of assumptions in the babel plugin. To get around this, we synthesize a new VariableDeclaration to contain the newly inserted function expression and therefore insert it as a true sibling to the original function. Yeah, it's kinda gross ghstack-source-id: df13e3b439962b95af4bbd82ef4302624668faf7 Pull Request resolved: https://github.com/facebook/react/pull/30446 --- .../src/Entrypoint/Program.ts | 81 ++++++++++++++++++- ...error.bug-outlining-in-func-expr.expect.md | 24 ------ .../error.bug-outlining-in-func-expr.js | 9 --- .../compiler/outlining-in-func-expr.expect.md | 72 +++++++++++++++++ .../compiler/outlining-in-func-expr.js | 21 +++++ 5 files changed, 170 insertions(+), 37 deletions(-) delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-outlining-in-func-expr.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-outlining-in-func-expr.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlining-in-func-expr.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlining-in-func-expr.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 d8b8a57fd42ec..18a6c6806fd36 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts @@ -183,13 +183,88 @@ export function createNewFunctionNode( transformedFn = fn; break; } + default: { + assertExhaustive( + originalFn.node, + `Creating unhandled function: ${originalFn.node}`, + ); + } } - // Avoid visiting the new transformed version ALREADY_COMPILED.add(transformedFn); return transformedFn; } +function insertNewFunctionNode( + originalFn: BabelFn, + compiledFn: CodegenFunction, +): NodePath { + switch (originalFn.type) { + case 'FunctionDeclaration': { + return originalFn.insertAfter( + createNewFunctionNode(originalFn, compiledFn), + )[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. + * + * 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). + */ + 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(), + { + reason: 'Expected a variable declarator parent', + loc: originalFn.node.loc ?? null, + }, + ); + 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; + } + default: { + assertExhaustive( + originalFn, + `Inserting unhandled function: ${originalFn}`, + ); + } + } +} + /* * This is a hack to work around what seems to be a Babel bug. Babel doesn't * consistently respect the `skip()` function to avoid revisiting a node within @@ -407,9 +482,7 @@ export function compileProgram( reason: 'Unexpected nested outlined functions', loc: outlined.fn.loc, }); - const fn = current.fn.insertAfter( - createNewFunctionNode(current.fn, outlined.fn), - )[0]!; + const fn = insertNewFunctionNode(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/error.bug-outlining-in-func-expr.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-outlining-in-func-expr.expect.md deleted file mode 100644 index 1a6995067dcb1..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-outlining-in-func-expr.expect.md +++ /dev/null @@ -1,24 +0,0 @@ - -## Input - -```javascript -const Component2 = props => { - return ( - - ); -}; - -``` - - -## Error - -``` -Invalid value used in weak set -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-outlining-in-func-expr.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-outlining-in-func-expr.js deleted file mode 100644 index f8bef4f1920bc..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-outlining-in-func-expr.js +++ /dev/null @@ -1,9 +0,0 @@ -const Component2 = props => { - return ( - - ); -}; 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 new file mode 100644 index 0000000000000..87c415002fed5 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlining-in-func-expr.expect.md @@ -0,0 +1,72 @@ + +## Input + +```javascript +const Component2 = props => { + return ( + + ); +}; + +export const FIXTURE_ENTRYPOINT = { + fn: Component2, + params: [ + { + items: [ + {id: 2, name: 'foo'}, + {id: 3, name: 'bar'}, + ], + }, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +const Component2 = (props) => { + const $ = _c(4); + let t0; + if ($[0] !== props.items) { + t0 = props.items.map(_temp); + $[0] = props.items; + $[1] = t0; + } else { + t0 = $[1]; + } + let t1; + if ($[2] !== t0) { + t1 = ; + $[2] = t0; + $[3] = t1; + } else { + t1 = $[3]; + } + return t1; +}; +const _temp = (item) => { + return
  • {item.name}
    • ; +}; + +export const FIXTURE_ENTRYPOINT = { + fn: Component2, + params: [ + { + items: [ + { id: 2, name: "foo" }, + { id: 3, name: "bar" }, + ], + }, + ], +}; + +``` + +### Eval output +(kind: ok) \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlining-in-func-expr.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlining-in-func-expr.js new file mode 100644 index 0000000000000..3a6ac59d40973 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlining-in-func-expr.js @@ -0,0 +1,21 @@ +const Component2 = props => { + return ( + + ); +}; + +export const FIXTURE_ENTRYPOINT = { + fn: Component2, + params: [ + { + items: [ + {id: 2, name: 'foo'}, + {id: 3, name: 'bar'}, + ], + }, + ], +};