@samitouri / QOS-React / commits / a6b7e438ca

[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

Lauren Tan committed Jul 24, 2024 at 15:57 UTC a6b7e438ca29aa65263d19284d73c1a2e6b11a3b
5 files changed +170 -37
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts
+77 -4
@@ -183,13 +183,88 @@ export function createNewFunctionNode(
183 transformedFn = fn;
184 break;
185 }
186 + default: {
187 + assertExhaustive(
188 + originalFn.node,
189 + `Creating unhandled function: ${originalFn.node}`,
190 + );
191 + }
192 }
187 -
193 // Avoid visiting the new transformed version
194 ALREADY_COMPILED.add(transformedFn);
195 return transformedFn;
196 }
197
198 +function insertNewFunctionNode(
199 + originalFn: BabelFn,
200 + compiledFn: CodegenFunction,
201 +): NodePath<t.Function> {
202 + switch (originalFn.type) {
203 + case 'FunctionDeclaration': {
204 + return originalFn.insertAfter(
205 + createNewFunctionNode(originalFn, compiledFn),
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.
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).
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 + });
233 + CompilerError.invariant(
234 + originalFn.parentPath.isVariableDeclarator() &&
235 + originalFn.parentPath.parentPath.isVariableDeclaration(),
236 + {
237 + reason: 'Expected a variable declarator parent',
238 + loc: originalFn.node.loc ?? null,
239 + },
240 + );
241 + 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;
258 + }
259 + default: {
260 + assertExhaustive(
261 + originalFn,
262 + `Inserting unhandled function: ${originalFn}`,
263 + );
264 + }
265 + }
266 +}
267 +
268 /*
269 * This is a hack to work around what seems to be a Babel bug. Babel doesn't
270 * consistently respect the `skip()` function to avoid revisiting a node within
@@ -407,9 +482,7 @@ export function compileProgram(
482 reason: 'Unexpected nested outlined functions',
483 loc: outlined.fn.loc,
484 });
410 - const fn = current.fn.insertAfter(
411 - createNewFunctionNode(current.fn, outlined.fn),
412 - )[0]!;
485 + const fn = insertNewFunctionNode(current.fn, outlined.fn);
486 fn.skip();
487 ALREADY_COMPILED.add(fn.node);
488 if (outlined.type !== null) {
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-outlining-in-func-expr.expect.md deleted
-24
@@ -1,24 +0,0 @@
1 -
2 -## Input
3 -
4 -```javascript
5 -const Component2 = props => {
6 - return (
7 - <ul>
8 - {props.items.map(item => (
9 - <li key={item.id}>{item.name}</li>
10 - ))}
11 - </ul>
12 - );
13 -};
14 -
15 -```
16 -
17 -
18 -## Error
19 -
20 -```
21 -Invalid value used in weak set
22 -```
23 -
24 -
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-outlining-in-func-expr.js deleted
-9
@@ -1,9 +0,0 @@
1 -const Component2 = props => {
2 - return (
3 - <ul>
4 - {props.items.map(item => (
5 - <li key={item.id}>{item.name}</li>
6 - ))}
7 - </ul>
8 - );
9 -};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlining-in-func-expr.expect.md new
+72
@@ -0,0 +1,72 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +const Component2 = props => {
6 + return (
7 + <ul>
8 + {props.items.map(item => (
9 + <li key={item.id}>{item.name}</li>
10 + ))}
11 + </ul>
12 + );
13 +};
14 +
15 +export const FIXTURE_ENTRYPOINT = {
16 + fn: Component2,
17 + params: [
18 + {
19 + items: [
20 + {id: 2, name: 'foo'},
21 + {id: 3, name: 'bar'},
22 + ],
23 + },
24 + ],
25 +};
26 +
27 +```
28 +
29 +## Code
30 +
31 +```javascript
32 +import { c as _c } from "react/compiler-runtime";
33 +const Component2 = (props) => {
34 + const $ = _c(4);
35 + let t0;
36 + if ($[0] !== props.items) {
37 + t0 = props.items.map(_temp);
38 + $[0] = props.items;
39 + $[1] = t0;
40 + } else {
41 + t0 = $[1];
42 + }
43 + let t1;
44 + if ($[2] !== t0) {
45 + t1 = <ul>{t0}</ul>;
46 + $[2] = t0;
47 + $[3] = t1;
48 + } else {
49 + t1 = $[3];
50 + }
51 + return t1;
52 +};
53 +const _temp = (item) => {
54 + return <li key={item.id}>{item.name}</li>;
55 +};
56 +
57 +export const FIXTURE_ENTRYPOINT = {
58 + fn: Component2,
59 + params: [
60 + {
61 + items: [
62 + { id: 2, name: "foo" },
63 + { id: 3, name: "bar" },
64 + ],
65 + },
66 + ],
67 +};
68 +
69 +```
70 +
71 +### Eval output
72 +(kind: ok) <ul><li>foo</li><li>bar</li></ul>
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlining-in-func-expr.js new
+21
@@ -0,0 +1,21 @@
1 +const Component2 = props => {
2 + return (
3 + <ul>
4 + {props.items.map(item => (
5 + <li key={item.id}>{item.name}</li>
6 + ))}
7 + </ul>
8 + );
9 +};
10 +
11 +export const FIXTURE_ENTRYPOINT = {
12 + fn: Component2,
13 + params: [
14 + {
15 + items: [
16 + {id: 2, name: 'foo'},
17 + {id: 3, name: 'bar'},
18 + ],
19 + },
20 + ],
21 +};