mirror of
https://github.com/facebook/react.git
synced 2025-11-01 09:12:30 +00:00
[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.
This commit is contained in:
@@ -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), []),
|
||||
|
||||
-24
@@ -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 |
|
||||
```
|
||||
|
||||
|
||||
+29
@@ -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);
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
Reference in New Issue
Block a user