@samitouri / QOS-React / commits / 7939d92fcc

[compiler] clean up retry pipeline: `fireRetry` flag -> compileMode (#32511)

Removes `EnvironmentConfig.enableMinimalTransformsForRetry` in favor of `run` parameters. This is a minimal difference but lets us explicitly opt out certain compiler passes based on mode parameters, instead of environment configurations Retry flags don't really make sense to have in `EnvironmentConfig` anyways as the config is user-facing API, while retrying is a compiler implementation detail. (per @josephsavona's feedback https://github.com/facebook/react/pull/32164#issuecomment-2608616479) > Re the "hacky" framing of this in the PR title: I think this is fine. I can see having something like a compilation or output mode that we use when running the pipeline. Rather than changing environment settings when we re-run, various passes could take effect based on the combination of the mode + env flags. The modes might be: > > * Full: transform, validate, memoize. This is the default today. > * Transform: Along the lines of the backup mode in this PR. Only applies transforms that do not require following the rules of React, like `fire()`. > * Validate: This could be used for ESLint. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32511). * #32512 * __->__ #32511

mofeiZ committed Mar 13, 2025 at 19:54 UTC 7939d92fcc95ad5ee719c38272eaef14a3750fc0
18 files changed +111 -89
compiler/packages/babel-plugin-react-compiler/scripts/jest/makeTransform.ts
+1
@@ -181,6 +181,7 @@ function ReactForgetFunctionTransform() {
181 fn,
182 forgetOptions,
183 'Other',
184 + 'all_features',
185 '_c',
186 null,
187 null,
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts
+46 -27
@@ -24,6 +24,7 @@ import {
24 pruneUnusedLabelsHIR,
25 } from '../HIR';
26 import {
27 + CompilerMode,
28 Environment,
29 EnvironmentConfig,
30 ReactFunctionType,
@@ -100,6 +101,7 @@ import {outlineJSX} from '../Optimization/OutlineJsx';
101 import {optimizePropsMethodCalls} from '../Optimization/OptimizePropsMethodCalls';
102 import {transformFire} from '../Transform';
103 import {validateNoImpureFunctionsInRender} from '../Validation/ValiateNoImpureFunctionsInRender';
104 +import {CompilerError} from '..';
105
106 export type CompilerPipelineValue =
107 | {kind: 'ast'; name: string; value: CodegenFunction}
@@ -113,6 +115,7 @@ function run(
115 >,
116 config: EnvironmentConfig,
117 fnType: ReactFunctionType,
118 + mode: CompilerMode,
119 useMemoCacheIdentifier: string,
120 logger: Logger | null,
121 filename: string | null,
@@ -122,6 +125,7 @@ function run(
125 const env = new Environment(
126 func.scope,
127 fnType,
128 + mode,
129 config,
130 contextIdentifiers,
131 logger,
@@ -160,10 +164,10 @@ function runWithEnvironment(
164 validateUseMemo(hir);
165
166 if (
167 + env.isInferredMemoEnabled &&
168 !env.config.enablePreserveExistingManualUseMemo &&
169 !env.config.disableMemoizationForDebugging &&
165 - !env.config.enableChangeDetectionForDebugging &&
166 - !env.config.enableMinimalTransformsForRetry
170 + !env.config.enableChangeDetectionForDebugging
171 ) {
172 dropManualMemoization(hir);
173 log({kind: 'hir', name: 'DropManualMemoization', value: hir});
@@ -196,8 +200,13 @@ function runWithEnvironment(
200 inferTypes(hir);
201 log({kind: 'hir', name: 'InferTypes', value: hir});
202
199 - if (env.config.validateHooksUsage) {
200 - validateHooksUsage(hir);
203 + if (env.isInferredMemoEnabled) {
204 + if (env.config.validateHooksUsage) {
205 + validateHooksUsage(hir);
206 + }
207 + if (env.config.validateNoCapitalizedCalls) {
208 + validateNoCapitalizedCalls(hir);
209 + }
210 }
211
212 if (env.config.enableFire) {
@@ -205,10 +214,6 @@ function runWithEnvironment(
214 log({kind: 'hir', name: 'TransformFire', value: hir});
215 }
216
208 - if (env.config.validateNoCapitalizedCalls) {
209 - validateNoCapitalizedCalls(hir);
210 - }
211 -
217 if (env.config.lowerContextAccess) {
218 lowerContextAccess(hir, env.config.lowerContextAccess);
219 }
@@ -219,7 +224,12 @@ function runWithEnvironment(
224 analyseFunctions(hir);
225 log({kind: 'hir', name: 'AnalyseFunctions', value: hir});
226
222 - inferReferenceEffects(hir);
227 + const fnEffectErrors = inferReferenceEffects(hir);
228 + if (env.isInferredMemoEnabled) {
229 + if (fnEffectErrors.length > 0) {
230 + CompilerError.throw(fnEffectErrors[0]);
231 + }
232 + }
233 log({kind: 'hir', name: 'InferReferenceEffects', value: hir});
234
235 validateLocalsNotReassignedAfterRender(hir);
@@ -239,28 +249,30 @@ function runWithEnvironment(
249 inferMutableRanges(hir);
250 log({kind: 'hir', name: 'InferMutableRanges', value: hir});
251
242 - if (env.config.assertValidMutableRanges) {
243 - assertValidMutableRanges(hir);
244 - }
252 + if (env.isInferredMemoEnabled) {
253 + if (env.config.assertValidMutableRanges) {
254 + assertValidMutableRanges(hir);
255 + }
256
246 - if (env.config.validateRefAccessDuringRender) {
247 - validateNoRefAccessInRender(hir);
248 - }
257 + if (env.config.validateRefAccessDuringRender) {
258 + validateNoRefAccessInRender(hir);
259 + }
260
250 - if (env.config.validateNoSetStateInRender) {
251 - validateNoSetStateInRender(hir);
252 - }
261 + if (env.config.validateNoSetStateInRender) {
262 + validateNoSetStateInRender(hir);
263 + }
264
254 - if (env.config.validateNoSetStateInPassiveEffects) {
255 - validateNoSetStateInPassiveEffects(hir);
256 - }
265 + if (env.config.validateNoSetStateInPassiveEffects) {
266 + validateNoSetStateInPassiveEffects(hir);
267 + }
268
258 - if (env.config.validateNoJSXInTryStatements) {
259 - validateNoJSXInTryStatement(hir);
260 - }
269 + if (env.config.validateNoJSXInTryStatements) {
270 + validateNoJSXInTryStatement(hir);
271 + }
272
262 - if (env.config.validateNoImpureFunctionsInRender) {
263 - validateNoImpureFunctionsInRender(hir);
273 + if (env.config.validateNoImpureFunctionsInRender) {
274 + validateNoImpureFunctionsInRender(hir);
275 + }
276 }
277
278 inferReactivePlaces(hir);
@@ -280,7 +292,12 @@ function runWithEnvironment(
292 value: hir,
293 });
294
283 - if (!env.config.enableMinimalTransformsForRetry) {
295 + if (env.isInferredMemoEnabled) {
296 + /**
297 + * Only create reactive scopes (which directly map to generated memo blocks)
298 + * if inferred memoization is enabled. This makes all later passes which
299 + * transform reactive-scope labeled instructions no-ops.
300 + */
301 inferReactiveScopeVariables(hir);
302 log({kind: 'hir', name: 'InferReactiveScopeVariables', value: hir});
303 }
@@ -529,6 +546,7 @@ export function compileFn(
546 >,
547 config: EnvironmentConfig,
548 fnType: ReactFunctionType,
549 + mode: CompilerMode,
550 useMemoCacheIdentifier: string,
551 logger: Logger | null,
552 filename: string | null,
@@ -538,6 +556,7 @@ export function compileFn(
556 func,
557 config,
558 fnType,
559 + mode,
560 useMemoCacheIdentifier,
561 logger,
562 filename,
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts
+20 -20
@@ -16,7 +16,6 @@ import {
16 EnvironmentConfig,
17 ExternalFunction,
18 ReactFunctionType,
19 - MINIMAL_RETRY_CONFIG,
19 } from '../HIR/Environment';
20 import {CodegenFunction} from '../ReactiveScopes';
21 import {isComponentDeclaration} from '../Utils/ComponentDeclaration';
@@ -407,6 +406,7 @@ export function compileProgram(
406 fn,
407 environment,
408 fnType,
409 + 'all_features',
410 useMemoCacheIdentifier.name,
411 pass.opts.logger,
412 pass.filename,
@@ -417,18 +417,29 @@ export function compileProgram(
417 compileResult = {kind: 'error', error: err};
418 }
419 }
420 - // If non-memoization features are enabled, retry regardless of error kind
421 - if (compileResult.kind === 'error' && environment.enableFire) {
420 +
421 + if (compileResult.kind === 'error') {
422 + /**
423 + * If an opt out directive is present, log only instead of throwing and don't mark as
424 + * containing a critical error.
425 + */
426 + if (optOutDirectives.length > 0) {
427 + logError(compileResult.error, pass, fn.node.loc ?? null);
428 + } else {
429 + handleError(compileResult.error, pass, fn.node.loc ?? null);
430 + }
431 + // If non-memoization features are enabled, retry regardless of error kind
432 + if (!environment.enableFire) {
433 + return null;
434 + }
435 try {
436 compileResult = {
437 kind: 'compile',
438 compiledFn: compileFn(
439 fn,
427 - {
428 - ...environment,
429 - ...MINIMAL_RETRY_CONFIG,
430 - },
440 + environment,
441 fnType,
442 + 'no_inferred_memo',
443 useMemoCacheIdentifier.name,
444 pass.opts.logger,
445 pass.filename,
@@ -436,20 +447,9 @@ export function compileProgram(
447 ),
448 };
449 } catch (err) {
439 - compileResult = {kind: 'error', error: err};
440 - }
441 - }
442 - if (compileResult.kind === 'error') {
443 - /**
444 - * If an opt out directive is present, log only instead of throwing and don't mark as
445 - * containing a critical error.
446 - */
447 - if (optOutDirectives.length > 0) {
448 - logError(compileResult.error, pass, fn.node.loc ?? null);
449 - } else {
450 - handleError(compileResult.error, pass, fn.node.loc ?? null);
450 + // TODO: we might want to log error here, but this will also result in duplicate logging
451 + return null;
452 }
452 - return null;
453 }
454
455 pass.opts.logger?.logEvent(pass.filename, {
compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts
+9 -13
@@ -96,6 +96,8 @@ export const MacroSchema = z.union([
96 z.tuple([z.string(), z.array(MacroMethodSchema)]),
97 ]);
98
99 +export type CompilerMode = 'all_features' | 'no_inferred_memo';
100 +
101 export type Macro = z.infer<typeof MacroSchema>;
102 export type MacroMethod = z.infer<typeof MacroMethodSchema>;
103
@@ -550,8 +552,6 @@ const EnvironmentConfigSchema = z.object({
552 */
553 disableMemoizationForDebugging: z.boolean().default(false),
554
553 - enableMinimalTransformsForRetry: z.boolean().default(false),
554 -
555 /**
556 * When true, rather using memoized values, the compiler will always re-compute
557 * values, and then use a heuristic to compare the memoized value to the newly
@@ -626,17 +626,6 @@ const EnvironmentConfigSchema = z.object({
626
627 export type EnvironmentConfig = z.infer<typeof EnvironmentConfigSchema>;
628
629 -export const MINIMAL_RETRY_CONFIG: PartialEnvironmentConfig = {
630 - validateHooksUsage: false,
631 - validateRefAccessDuringRender: false,
632 - validateNoSetStateInRender: false,
633 - validateNoSetStateInPassiveEffects: false,
634 - validateNoJSXInTryStatements: false,
635 - validateMemoizedEffectDependencies: false,
636 - validateNoCapitalizedCalls: null,
637 - validateBlocklistedImports: null,
638 - enableMinimalTransformsForRetry: true,
639 -};
629 /**
630 * For test fixtures and playground only.
631 *
@@ -851,6 +840,7 @@ export class Environment {
840 code: string | null;
841 config: EnvironmentConfig;
842 fnType: ReactFunctionType;
843 + compilerMode: CompilerMode;
844 useMemoCacheIdentifier: string;
845 hasLoweredContextAccess: boolean;
846 hasFireRewrite: boolean;
@@ -861,6 +851,7 @@ export class Environment {
851 constructor(
852 scope: BabelScope,
853 fnType: ReactFunctionType,
854 + compilerMode: CompilerMode,
855 config: EnvironmentConfig,
856 contextIdentifiers: Set<t.Identifier>,
857 logger: Logger | null,
@@ -870,6 +861,7 @@ export class Environment {
861 ) {
862 this.#scope = scope;
863 this.fnType = fnType;
864 + this.compilerMode = compilerMode;
865 this.config = config;
866 this.filename = filename;
867 this.code = code;
@@ -924,6 +916,10 @@ export class Environment {
916 this.#hoistedIdentifiers = new Set();
917 }
918
919 + get isInferredMemoEnabled(): boolean {
920 + return this.compilerMode !== 'no_inferred_memo';
921 + }
922 +
923 get nextIdentifierId(): IdentifierId {
924 return makeIdentifierId(this.#nextIdentifer++);
925 }
compiler/packages/babel-plugin-react-compiler/src/Inference/InferFunctionEffects.ts
+12 -7
@@ -5,7 +5,12 @@
5 * LICENSE file in the root directory of this source tree.
6 */
7
8 -import {CompilerError, ErrorSeverity, ValueKind} from '..';
8 +import {
9 + CompilerError,
10 + CompilerErrorDetailOptions,
11 + ErrorSeverity,
12 + ValueKind,
13 +} from '..';
14 import {
15 AbstractValue,
16 BasicBlock,
@@ -290,21 +295,21 @@ export function inferTerminalFunctionEffects(
295 return functionEffects;
296 }
297
293 -export function raiseFunctionEffectErrors(
298 +export function transformFunctionEffectErrors(
299 functionEffects: Array<FunctionEffect>,
295 -): void {
296 - functionEffects.forEach(eff => {
300 +): Array<CompilerErrorDetailOptions> {
301 + return functionEffects.map(eff => {
302 switch (eff.kind) {
303 case 'ReactMutation':
304 case 'GlobalMutation': {
300 - CompilerError.throw(eff.error);
305 + return eff.error;
306 }
307 case 'ContextMutation': {
303 - CompilerError.throw({
308 + return {
309 severity: ErrorSeverity.Invariant,
310 reason: `Unexpected ContextMutation in top-level function effects`,
311 loc: eff.loc,
307 - });
312 + };
313 }
314 default:
315 assertExhaustive(
compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts
+6 -5
@@ -5,7 +5,7 @@
5 * LICENSE file in the root directory of this source tree.
6 */
7
8 -import {CompilerError} from '../CompilerError';
8 +import {CompilerError, CompilerErrorDetailOptions} from '../CompilerError';
9 import {Environment} from '../HIR';
10 import {
11 AbstractValue,
@@ -49,7 +49,7 @@ import {assertExhaustive} from '../Utils/utils';
49 import {
50 inferTerminalFunctionEffects,
51 inferInstructionFunctionEffects,
52 - raiseFunctionEffectErrors,
52 + transformFunctionEffectErrors,
53 } from './InferFunctionEffects';
54
55 const UndefinedValue: InstructionValue = {
@@ -103,7 +103,7 @@ const UndefinedValue: InstructionValue = {
103 export default function inferReferenceEffects(
104 fn: HIRFunction,
105 options: {isFunctionExpression: boolean} = {isFunctionExpression: false},
106 -): void {
106 +): Array<CompilerErrorDetailOptions> {
107 /*
108 * Initial state contains function params
109 * TODO: include module declarations here as well
@@ -241,8 +241,9 @@ export default function inferReferenceEffects(
241
242 if (options.isFunctionExpression) {
243 fn.effects = functionEffects;
244 - } else if (!fn.env.config.enableMinimalTransformsForRetry) {
245 - raiseFunctionEffectErrors(functionEffects);
244 + return [];
245 + } else {
246 + return transformFunctionEffectErrors(functionEffects);
247 }
248 }
249
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-capitalized-fn-call.expect.md
+2 -2
@@ -2,7 +2,7 @@
2 ## Input
3
4 ```javascript
5 -// @validateNoCapitalizedCalls @enableFire
5 +// @validateNoCapitalizedCalls @enableFire @panicThreshold(none)
6 import {fire} from 'react';
7 const CapitalizedCall = require('shared-runtime').sum;
8
@@ -24,7 +24,7 @@ function Component({prop1, bar}) {
24 ## Code
25
26 ```javascript
27 -import { useFire } from "react/compiler-runtime"; // @validateNoCapitalizedCalls @enableFire
27 +import { useFire } from "react/compiler-runtime"; // @validateNoCapitalizedCalls @enableFire @panicThreshold(none)
28 import { fire } from "react";
29 const CapitalizedCall = require("shared-runtime").sum;
30
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-capitalized-fn-call.js
+1 -1
@@ -1,4 +1,4 @@
1 -// @validateNoCapitalizedCalls @enableFire
1 +// @validateNoCapitalizedCalls @enableFire @panicThreshold(none)
2 import {fire} from 'react';
3 const CapitalizedCall = require('shared-runtime').sum;
4
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-eslint-suppressions.expect.md
+2 -2
@@ -2,7 +2,7 @@
2 ## Input
3
4 ```javascript
5 -// @enableFire
5 +// @enableFire @panicThreshold(none)
6 import {useRef} from 'react';
7
8 function Component({props, bar}) {
@@ -26,7 +26,7 @@ function Component({props, bar}) {
26 ## Code
27
28 ```javascript
29 -import { useFire } from "react/compiler-runtime"; // @enableFire
29 +import { useFire } from "react/compiler-runtime"; // @enableFire @panicThreshold(none)
30 import { useRef } from "react";
31
32 function Component(t0) {
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-eslint-suppressions.js
+1 -1
@@ -1,4 +1,4 @@
1 -// @enableFire
1 +// @enableFire @panicThreshold(none)
2 import {useRef} from 'react';
3
4 function Component({props, bar}) {
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-preserve-memo.expect.md
+2 -2
@@ -2,7 +2,7 @@
2 ## Input
3
4 ```javascript
5 -// @validatePreserveExistingMemoizationGuarantees @enableFire
5 +// @validatePreserveExistingMemoizationGuarantees @enableFire @panicThreshold(none)
6 import {fire} from 'react';
7 import {sum} from 'shared-runtime';
8
@@ -24,7 +24,7 @@ function Component({prop1, bar}) {
24 ## Code
25
26 ```javascript
27 -import { useFire } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees @enableFire
27 +import { useFire } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees @enableFire @panicThreshold(none)
28 import { fire } from "react";
29 import { sum } from "shared-runtime";
30
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-preserve-memo.js
+1 -1
@@ -1,4 +1,4 @@
1 -// @validatePreserveExistingMemoizationGuarantees @enableFire
1 +// @validatePreserveExistingMemoizationGuarantees @enableFire @panicThreshold(none)
2 import {fire} from 'react';
3 import {sum} from 'shared-runtime';
4
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-prop-write.expect.md
+2 -2
@@ -2,7 +2,7 @@
2 ## Input
3
4 ```javascript
5 -// @enableFire
5 +// @enableFire @panicThreshold(none)
6 import {fire} from 'react';
7
8 function Component({prop1}) {
@@ -20,7 +20,7 @@ function Component({prop1}) {
20 ## Code
21
22 ```javascript
23 -import { useFire } from "react/compiler-runtime"; // @enableFire
23 +import { useFire } from "react/compiler-runtime"; // @enableFire @panicThreshold(none)
24 import { fire } from "react";
25
26 function Component(t0) {
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-prop-write.js
+1 -1
@@ -1,4 +1,4 @@
1 -// @enableFire
1 +// @enableFire @panicThreshold(none)
2 import {fire} from 'react';
3
4 function Component({prop1}) {
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-ref-current-access.expect.md
+1 -1
@@ -2,7 +2,7 @@
2 ## Input
3
4 ```javascript
5 -// @flow @enableFire
5 +// @flow @enableFire @panicThreshold(none)
6 import {fire} from 'react';
7 import {print} from 'shared-runtime';
8
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-ref-current-access.js
+1 -1
@@ -1,4 +1,4 @@
1 -// @flow @enableFire
1 +// @flow @enableFire @panicThreshold(none)
2 import {fire} from 'react';
3 import {print} from 'shared-runtime';
4
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-validate-conditional-hook.expect.md
+2 -2
@@ -2,7 +2,7 @@
2 ## Input
3
4 ```javascript
5 -// @enableFire
5 +// @enableFire @panicThreshold(none)
6 import {fire, useEffect} from 'react';
7 import {Stringify} from 'shared-runtime';
8
@@ -29,7 +29,7 @@ function Component(props) {
29 ## Code
30
31 ```javascript
32 -import { useFire } from "react/compiler-runtime"; // @enableFire
32 +import { useFire } from "react/compiler-runtime"; // @enableFire @panicThreshold(none)
33 import { fire, useEffect } from "react";
34 import { Stringify } from "shared-runtime";
35
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-validate-conditional-hook.js
+1 -1
@@ -1,4 +1,4 @@
1 -// @enableFire
1 +// @enableFire @panicThreshold(none)
2 import {fire, useEffect} from 'react';
3 import {Stringify} from 'shared-runtime';
4