From 97fbebbbae42f31d7fb28edee87f75d15ab9c22d Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Wed, 12 Mar 2025 20:15:23 -0400 Subject: [PATCH] [compiler] Avoid bailouts when inserting gating This change fixes a coverage hole in rolling out with `gating`. Prior to this PR, configuring `gating` causes React Compiler to bail out of optimizing some functions. This means that it's not entirely safe to cutover from `gating` enabled for all users (i.e. rolled out 100%) to removing the `gating` config altogether, as new functions may be opted into compilation (as they stop bailing out due to gating-specific logic). This is technically slightly slower due to the additional function indirection + argument spreads / parameter rest elements. An alternative approach is to recommend running a codemod before removing the`gating` config. --- .../src/Entrypoint/Gating.ts | 55 +++++++++++++++++-- .../error.gating-use-before-decl.expect.md | 24 -------- .../gating/gating-use-before-decl.expect.md | 29 ++++++++++ ...fore-decl.js => gating-use-before-decl.js} | 0 4 files changed, 80 insertions(+), 28 deletions(-) delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/gating/error.gating-use-before-decl.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/gating/gating-use-before-decl.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/gating/{error.gating-use-before-decl.js => gating-use-before-decl.js} (100%) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Gating.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Gating.ts index 74a200feb0..ab4a179b59 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Gating.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Gating.ts @@ -22,12 +22,59 @@ export function insertGatedFunctionDeclaration( referencedBeforeDeclaration: boolean, ): void { if (referencedBeforeDeclaration && fnPath.isFunctionDeclaration()) { - CompilerError.throwTodo({ - reason: `Encountered a function used before its declaration, which breaks Forget's gating codegen due to hoisting`, - description: `Rewrite the reference to ${fnPath.node.id?.name ?? 'this function'} to not rely on hoisting to fix this issue`, + CompilerError.invariant(compiled.type === 'FunctionDeclaration', { + reason: 'Expected compiled node type to match input type', + description: `Got ${compiled.type} but expected FunctionDeclaration`, loc: fnPath.node.loc ?? null, - suggestions: null, }); + CompilerError.invariant(fnPath.node.id != null && compiled.id != null, { + reason: + 'Function declarations that are referenced elsewhere must have an id', + loc: fnPath.node.loc ?? null, + }); + + const gatingCondition = fnPath.scope.generateUidIdentifier( + `_${gating.importSpecifierName}_result`, + ); + const originalFnName = fnPath.node.id; + const unoptimizedFnName = fnPath.scope.generateUidIdentifier( + `${fnPath.node.id.name}_unoptimized`, + ); + const optimizedFnName = fnPath.scope.generateUidIdentifier( + `${fnPath.node.id.name}_optimized`, + ); + compiled.id.name = optimizedFnName.name; + fnPath.get('id').replaceInline(unoptimizedFnName); + fnPath.insertAfter( + t.functionDeclaration( + originalFnName, + [t.restElement(t.identifier('args'))], + t.blockStatement([ + t.ifStatement( + gatingCondition, + t.returnStatement( + t.callExpression(compiled.id, [ + t.spreadElement(t.identifier('args')), + ]), + ), + t.returnStatement( + t.callExpression(unoptimizedFnName, [ + t.spreadElement(t.identifier('args')), + ]), + ), + ), + ]), + ), + ); + fnPath.insertBefore( + t.variableDeclaration('const', [ + t.variableDeclarator( + gatingCondition, + t.callExpression(t.identifier(gating.importSpecifierName), []), + ), + ]), + ); + fnPath.insertBefore(compiled); } else { const gatingExpression = t.conditionalExpression( t.callExpression(t.identifier(gating.importSpecifierName), []), diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/gating/error.gating-use-before-decl.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/gating/error.gating-use-before-decl.expect.md deleted file mode 100644 index a1b5a61c21..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/gating/error.gating-use-before-decl.expect.md +++ /dev/null @@ -1,24 +0,0 @@ - -## Input - -```javascript -// @gating -import {memo} from 'react'; - -export default memo(Foo); -function Foo() {} - -``` - - -## Error - -``` - 3 | - 4 | export default memo(Foo); -> 5 | function Foo() {} - | ^^^ Invariant: Encountered a function used before its declaration, which breaks Forget's gating codegen due to hoisting. Rewrite the reference to Foo to not rely on hoisting to fix this issue (5:5) - 6 | -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/gating/gating-use-before-decl.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/gating/gating-use-before-decl.expect.md new file mode 100644 index 0000000000..e25184358f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/gating/gating-use-before-decl.expect.md @@ -0,0 +1,29 @@ + +## Input + +```javascript +// @gating +import {memo} from 'react'; + +export default memo(Foo); +function Foo() {} + +``` + +## Code + +```javascript +import { isForgetEnabled_Fixtures } from "ReactForgetFeatureFlag"; // @gating +import { memo } from "react"; + +export default memo(Foo); +const _isForgetEnabled_Fixtures_result = isForgetEnabled_Fixtures(); +function _Foo_optimized() {} +function _Foo_unoptimized() {} +function Foo(...args) { + if (_isForgetEnabled_Fixtures_result) return _Foo_optimized(...args); + else return _Foo_unoptimized(...args); +} + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/gating/error.gating-use-before-decl.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/gating/gating-use-before-decl.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/gating/error.gating-use-before-decl.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/gating/gating-use-before-decl.js