@samitouri / QOS-React / commits / 93b61fc4ec

[compiler][ez] Stop bailing out early for hoisted gated functions (#32597)

Some code movement for the next PR --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32597). * #32598 * __->__ #32597

mofeiZ committed Mar 13, 2025 at 19:08 UTC 93b61fc4ecb34abec2b55c206f34ed22dd340b71
3 files changed +93 -77
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Gating.ts
+52 -39
@@ -8,6 +8,7 @@
8 import {NodePath} from '@babel/core';
9 import * as t from '@babel/types';
10 import {PluginOptions} from './Options';
11 +import {CompilerError} from '../CompilerError';
12
13 export function insertGatedFunctionDeclaration(
14 fnPath: NodePath<
@@ -18,47 +19,59 @@ export function insertGatedFunctionDeclaration(
19 | t.ArrowFunctionExpression
20 | t.FunctionExpression,
21 gating: NonNullable<PluginOptions['gating']>,
22 + referencedBeforeDeclaration: boolean,
23 ): void {
22 - const gatingExpression = t.conditionalExpression(
23 - t.callExpression(t.identifier(gating.importSpecifierName), []),
24 - buildFunctionExpression(compiled),
25 - buildFunctionExpression(fnPath.node),
26 - );
27 -
28 - /*
29 - * Convert function declarations to named variables *unless* this is an
30 - * `export default function ...` since `export default const ...` is
31 - * not supported. For that case we fall through to replacing w the raw
32 - * conditional expression
33 - */
34 - if (
35 - fnPath.parentPath.node.type !== 'ExportDefaultDeclaration' &&
36 - fnPath.node.type === 'FunctionDeclaration' &&
37 - fnPath.node.id != null
38 - ) {
39 - fnPath.replaceWith(
40 - t.variableDeclaration('const', [
41 - t.variableDeclarator(fnPath.node.id, gatingExpression),
42 - ]),
43 - );
44 - } else if (
45 - fnPath.parentPath.node.type === 'ExportDefaultDeclaration' &&
46 - fnPath.node.type !== 'ArrowFunctionExpression' &&
47 - fnPath.node.id != null
48 - ) {
49 - fnPath.insertAfter(
50 - t.exportDefaultDeclaration(t.identifier(fnPath.node.id.name)),
51 - );
52 - fnPath.parentPath.replaceWith(
53 - t.variableDeclaration('const', [
54 - t.variableDeclarator(
55 - t.identifier(fnPath.node.id.name),
56 - gatingExpression,
57 - ),
58 - ]),
59 - );
24 + if (referencedBeforeDeclaration) {
25 + const identifier =
26 + fnPath.node.type === 'FunctionDeclaration' ? fnPath.node.id : null;
27 + CompilerError.invariant(false, {
28 + reason: `Encountered a function used before its declaration, which breaks Forget's gating codegen due to hoisting`,
29 + description: `Rewrite the reference to ${identifier?.name ?? 'this function'} to not rely on hoisting to fix this issue`,
30 + loc: identifier?.loc ?? null,
31 + suggestions: null,
32 + });
33 } else {
61 - fnPath.replaceWith(gatingExpression);
34 + const gatingExpression = t.conditionalExpression(
35 + t.callExpression(t.identifier(gating.importSpecifierName), []),
36 + buildFunctionExpression(compiled),
37 + buildFunctionExpression(fnPath.node),
38 + );
39 +
40 + /*
41 + * Convert function declarations to named variables *unless* this is an
42 + * `export default function ...` since `export default const ...` is
43 + * not supported. For that case we fall through to replacing w the raw
44 + * conditional expression
45 + */
46 + if (
47 + fnPath.parentPath.node.type !== 'ExportDefaultDeclaration' &&
48 + fnPath.node.type === 'FunctionDeclaration' &&
49 + fnPath.node.id != null
50 + ) {
51 + fnPath.replaceWith(
52 + t.variableDeclaration('const', [
53 + t.variableDeclarator(fnPath.node.id, gatingExpression),
54 + ]),
55 + );
56 + } else if (
57 + fnPath.parentPath.node.type === 'ExportDefaultDeclaration' &&
58 + fnPath.node.type !== 'ArrowFunctionExpression' &&
59 + fnPath.node.id != null
60 + ) {
61 + fnPath.insertAfter(
62 + t.exportDefaultDeclaration(t.identifier(fnPath.node.id.name)),
63 + );
64 + fnPath.parentPath.replaceWith(
65 + t.variableDeclaration('const', [
66 + t.variableDeclarator(
67 + t.identifier(fnPath.node.id.name),
68 + gatingExpression,
69 + ),
70 + ]),
71 + );
72 + } else {
73 + fnPath.replaceWith(gatingExpression);
74 + }
75 }
76 }
77
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Options.ts
+9
@@ -12,6 +12,7 @@ import {
12 EnvironmentConfig,
13 ExternalFunction,
14 parseEnvironmentConfig,
15 + tryParseExternalFunction,
16 } from '../HIR/Environment';
17 import {hasOwnProperty} from '../Utils/utils';
18 import {fromZodError} from 'zod-validation-error';
@@ -271,6 +272,14 @@ export function parsePluginOptions(obj: unknown): PluginOptions {
272 parsedOptions[key] = parseTargetConfig(value);
273 break;
274 }
275 + case 'gating': {
276 + if (value == null) {
277 + parsedOptions[key] = null;
278 + } else {
279 + parsedOptions[key] = tryParseExternalFunction(value);
280 + }
281 + break;
282 + }
283 default: {
284 parsedOptions[key] = value;
285 }
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts
+32 -38
@@ -17,7 +17,6 @@ import {
17 ExternalFunction,
18 ReactFunctionType,
19 MINIMAL_RETRY_CONFIG,
20 - tryParseExternalFunction,
20 } from '../HIR/Environment';
21 import {CodegenFunction} from '../ReactiveScopes';
22 import {isComponentDeclaration} from '../Utils/ComponentDeclaration';
@@ -541,30 +540,26 @@ export function compileProgram(
540 if (moduleScopeOptOutDirectives.length > 0) {
541 return;
542 }
544 -
543 + let gating: null | {
544 + gatingFn: ExternalFunction;
545 + referencedBeforeDeclared: Set<CompileResult>;
546 + } = null;
547 if (pass.opts.gating != null) {
546 - const error = checkFunctionReferencedBeforeDeclarationAtTopLevel(
547 - program,
548 - compiledFns.map(result => {
549 - return result.originalFn;
550 - }),
551 - );
552 - if (error) {
553 - handleError(error, pass, null);
554 - return;
555 - }
548 + gating = {
549 + gatingFn: pass.opts.gating,
550 + referencedBeforeDeclared:
551 + getFunctionReferencedBeforeDeclarationAtTopLevel(program, compiledFns),
552 + };
553 }
554
555 const hasLoweredContextAccess = compiledFns.some(
556 c => c.compiledFn.hasLoweredContextAccess,
557 );
558 const externalFunctions: Array<ExternalFunction> = [];
562 - let gating: null | ExternalFunction = null;
559 try {
560 // TODO: check for duplicate import specifiers
565 - if (pass.opts.gating != null) {
566 - gating = tryParseExternalFunction(pass.opts.gating);
567 - externalFunctions.push(gating);
561 + if (gating != null) {
562 + externalFunctions.push(gating.gatingFn);
563 }
564
565 const lowerContextAccess = environment.lowerContextAccess;
@@ -613,7 +608,12 @@ export function compileProgram(
608 const transformedFn = createNewFunctionNode(originalFn, compiledFn);
609
610 if (gating != null && kind === 'original') {
616 - insertGatedFunctionDeclaration(originalFn, transformedFn, gating);
611 + insertGatedFunctionDeclaration(
612 + originalFn,
613 + transformedFn,
614 + gating.gatingFn,
615 + gating.referencedBeforeDeclared.has(result),
616 + );
617 } else {
618 originalFn.replaceWith(transformedFn);
619 }
@@ -1093,20 +1093,23 @@ function getFunctionName(
1093 }
1094 }
1095
1096 -function checkFunctionReferencedBeforeDeclarationAtTopLevel(
1096 +function getFunctionReferencedBeforeDeclarationAtTopLevel(
1097 program: NodePath<t.Program>,
1098 - fns: Array<BabelFn>,
1099 -): CompilerError | null {
1100 - const fnIds = new Set(
1098 + fns: Array<CompileResult>,
1099 +): Set<CompileResult> {
1100 + const fnNames = new Map<string, {id: t.Identifier; fn: CompileResult}>(
1101 fns
1102 - .map(fn => getFunctionName(fn))
1102 + .map<[NodePath<t.Expression> | null, CompileResult]>(fn => [
1103 + getFunctionName(fn.originalFn),
1104 + fn,
1105 + ])
1106 .filter(
1104 - (name): name is NodePath<t.Identifier> => !!name && name.isIdentifier(),
1107 + (entry): entry is [NodePath<t.Identifier>, CompileResult] =>
1108 + !!entry[0] && entry[0].isIdentifier(),
1109 )
1106 - .map(name => name.node),
1110 + .map(entry => [entry[0].node.name, {id: entry[0].node, fn: entry[1]}]),
1111 );
1108 - const fnNames = new Map([...fnIds].map(id => [id.name, id]));
1109 - const errors = new CompilerError();
1112 + const referencedBeforeDeclaration = new Set<CompileResult>();
1113
1114 program.traverse({
1115 TypeAnnotation(path) {
@@ -1132,8 +1135,7 @@ function checkFunctionReferencedBeforeDeclarationAtTopLevel(
1135 * We've reached the declaration, hoisting is no longer possible, stop
1136 * checking for this component name.
1137 */
1135 - if (fnIds.has(id.node)) {
1136 - fnIds.delete(id.node);
1138 + if (id.node === fn.id) {
1139 fnNames.delete(id.node.name);
1140 return;
1141 }
@@ -1144,20 +1146,12 @@ function checkFunctionReferencedBeforeDeclarationAtTopLevel(
1146 * top level scope.
1147 */
1148 if (scope === null && id.isReferencedIdentifier()) {
1147 - errors.pushErrorDetail(
1148 - new CompilerErrorDetail({
1149 - reason: `Encountered a function used before its declaration, which breaks Forget's gating codegen due to hoisting`,
1150 - description: `Rewrite the reference to ${fn.name} to not rely on hoisting to fix this issue`,
1151 - loc: fn.loc ?? null,
1152 - suggestions: null,
1153 - severity: ErrorSeverity.Invariant,
1154 - }),
1155 - );
1149 + referencedBeforeDeclaration.add(fn.fn);
1150 }
1151 },
1152 });
1153
1160 - return errors.details.length > 0 ? errors : null;
1154 + return referencedBeforeDeclaration;
1155 }
1156
1157 function getReactCompilerRuntimeModule(opts: PluginOptions): string {