@samitouri / QOS-React / commits / 0962f684a0

[compiler][bugfix] Don't insert hook guards in retry pipeline (#32665)

Fixing bug from https://github.com/facebook/react/pull/32164 -- prior to this PR, we inserted hook guards even for functions that bailed out of compilation.

mofeiZ committed Mar 20, 2025 at 17:25 UTC 0962f684a066df4fd2a7db7489cb1984799ad674
6 files changed +116 -9
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts
+6
@@ -451,6 +451,12 @@ export function compileProgram(
451 pass.code,
452 ),
453 };
454 + if (
455 + !compileResult.compiledFn.hasFireRewrite &&
456 + !compileResult.compiledFn.hasLoweredContextAccess
457 + ) {
458 + return null;
459 + }
460 } catch (err) {
461 // TODO: we might want to log error here, but this will also result in duplicate logging
462 if (err instanceof CompilerError) {
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts
+13 -9
@@ -14,7 +14,7 @@ import {
14 renameVariables,
15 } from '.';
16 import {CompilerError, ErrorSeverity} from '../CompilerError';
17 -import {Environment, EnvironmentConfig, ExternalFunction} from '../HIR';
17 +import {Environment, ExternalFunction} from '../HIR';
18 import {
19 ArrayPattern,
20 BlockId,
@@ -156,7 +156,7 @@ export function codegenFunction(
156 const compiled = compileResult.unwrap();
157
158 const hookGuard = fn.env.config.enableEmitHookGuards;
159 - if (hookGuard != null) {
159 + if (hookGuard != null && fn.env.isInferredMemoEnabled) {
160 compiled.body = t.blockStatement([
161 createHookGuard(
162 hookGuard,
@@ -250,7 +250,11 @@ export function codegenFunction(
250 }
251
252 const emitInstrumentForget = fn.env.config.enableEmitInstrumentForget;
253 - if (emitInstrumentForget != null && fn.id != null) {
253 + if (
254 + emitInstrumentForget != null &&
255 + fn.id != null &&
256 + fn.env.isInferredMemoEnabled
257 + ) {
258 /*
259 * Technically, this is a conditional hook call. However, we expect
260 * __DEV__ and gating identifier to be runtime constants
@@ -548,7 +552,7 @@ function codegenBlockNoReset(
552 }
553
554 function wrapCacheDep(cx: Context, value: t.Expression): t.Expression {
551 - if (cx.env.config.enableEmitFreeze != null) {
555 + if (cx.env.config.enableEmitFreeze != null && cx.env.isInferredMemoEnabled) {
556 // The import declaration for emitFreeze is inserted in the Babel plugin
557 return t.conditionalExpression(
558 t.identifier('__DEV__'),
@@ -1553,7 +1557,7 @@ function createHookGuard(
1557 * ```
1558 */
1559 function createCallExpression(
1556 - config: EnvironmentConfig,
1560 + env: Environment,
1561 callee: t.Expression,
1562 args: Array<t.Expression | t.SpreadElement>,
1563 loc: SourceLocation | null,
@@ -1564,8 +1568,8 @@ function createCallExpression(
1568 callExpr.loc = loc;
1569 }
1570
1567 - const hookGuard = config.enableEmitHookGuards;
1568 - if (hookGuard != null && isHook) {
1571 + const hookGuard = env.config.enableEmitHookGuards;
1572 + if (hookGuard != null && isHook && env.isInferredMemoEnabled) {
1573 const iife = t.functionExpression(
1574 null,
1575 [],
@@ -1701,7 +1705,7 @@ function codegenInstructionValue(
1705 const callee = codegenPlaceToExpression(cx, instrValue.callee);
1706 const args = instrValue.args.map(arg => codegenArgument(cx, arg));
1707 value = createCallExpression(
1704 - cx.env.config,
1708 + cx.env,
1709 callee,
1710 args,
1711 instrValue.loc,
@@ -1791,7 +1795,7 @@ function codegenInstructionValue(
1795 );
1796 const args = instrValue.args.map(arg => codegenArgument(cx, arg));
1797 value = createCallExpression(
1794 - cx.env.config,
1798 + cx.env,
1799 memberExpr,
1800 args,
1801 instrValue.loc,
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-dont-add-hook-guards-on-retry.expect.md new
+27
@@ -0,0 +1,27 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @flow @enableEmitHookGuards @panicThreshold(none) @enableFire
6 +
7 +component Foo(useDynamicHook) {
8 + useDynamicHook();
9 + return <div>hello world</div>;
10 +}
11 +
12 +```
13 +
14 +## Code
15 +
16 +```javascript
17 +function Foo({
18 + useDynamicHook,
19 +}: $ReadOnly<{ useDynamicHook: any }>): React.Node {
20 + useDynamicHook();
21 + return <div>hello world</div>;
22 +}
23 +
24 +```
25 +
26 +### Eval output
27 +(kind: exception) Fixture not implemented
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-dont-add-hook-guards-on-retry.js new
+6
@@ -0,0 +1,6 @@
1 +// @flow @enableEmitHookGuards @panicThreshold(none) @enableFire
2 +
3 +component Foo(useDynamicHook) {
4 + useDynamicHook();
5 + return <div>hello world</div>;
6 +}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repro-dont-add-hook-guards-on-retry.expect.md new
+49
@@ -0,0 +1,49 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @flow @enableEmitHookGuards @panicThreshold(none) @enableFire
6 +import {useEffect, fire} from 'react';
7 +
8 +function Component(props, useDynamicHook) {
9 + 'use memo';
10 + useDynamicHook();
11 + const foo = props => {
12 + console.log(props);
13 + };
14 + useEffect(() => {
15 + fire(foo(props));
16 + });
17 +
18 + return <div>hello world</div>;
19 +}
20 +
21 +```
22 +
23 +## Code
24 +
25 +```javascript
26 +import { $dispatcherGuard } from "react-compiler-runtime";
27 +import { useFire } from "react/compiler-runtime";
28 +import { useEffect, fire } from "react";
29 +
30 +function Component(props, useDynamicHook) {
31 + "use memo";
32 +
33 + useDynamicHook();
34 + const foo = _temp;
35 + const t0 = useFire(foo);
36 +
37 + useEffect(() => {
38 + t0(props);
39 + });
40 + return <div>hello world</div>;
41 +}
42 +function _temp(props_0) {
43 + console.log(props_0);
44 +}
45 +
46 +```
47 +
48 +### Eval output
49 +(kind: exception) Fixture not implemented
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/repro-dont-add-hook-guards-on-retry.js new
+15
@@ -0,0 +1,15 @@
1 +// @flow @enableEmitHookGuards @panicThreshold(none) @enableFire
2 +import {useEffect, fire} from 'react';
3 +
4 +function Component(props, useDynamicHook) {
5 + 'use memo';
6 + useDynamicHook();
7 + const foo = props => {
8 + console.log(props);
9 + };
10 + useEffect(() => {
11 + fire(foo(props));
12 + });
13 +
14 + return <div>hello world</div>;
15 +}