[compiler] clean up retry pipeline: fireRetry flag -> compileMode

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 #32164 (comment)) > 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.
facebook · Mar 4, 2025 · 48c873b · 48c873b
1 parent e0fe347
commit 48c873b
Show file tree

Hide file tree

Showing 6 changed files with 83 additions and 57 deletions.
diff --git a/compiler/packages/babel-plugin-react-compiler/scripts/jest/makeTransform.ts b/compiler/packages/babel-plugin-react-compiler/scripts/jest/makeTransform.ts
@@ -181,6 +181,7 @@ function ReactForgetFunctionTransform() {
         fn,
         forgetOptions,
         'Other',
+        'all_features',
         '_c',
         null,
         null,

diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts
@@ -24,6 +24,7 @@ import {
   pruneUnusedLabelsHIR,
 } from '../HIR';
 import {
+  CompilerMode,
   Environment,
   EnvironmentConfig,
   ReactFunctionType,
@@ -100,6 +101,7 @@ import {outlineJSX} from '../Optimization/OutlineJsx';
 import {optimizePropsMethodCalls} from '../Optimization/OptimizePropsMethodCalls';
 import {transformFire} from '../Transform';
 import {validateNoImpureFunctionsInRender} from '../Validation/ValiateNoImpureFunctionsInRender';
+import {CompilerError} from '..';
 
 export type CompilerPipelineValue =
   | {kind: 'ast'; name: string; value: CodegenFunction}
@@ -113,6 +115,7 @@ function run(
   >,
   config: EnvironmentConfig,
   fnType: ReactFunctionType,
+  mode: CompilerMode,
   useMemoCacheIdentifier: string,
   logger: Logger | null,
   filename: string | null,
@@ -122,6 +125,7 @@ function run(
   const env = new Environment(
     func.scope,
     fnType,
+    mode,
     config,
     contextIdentifiers,
     logger,
@@ -160,10 +164,10 @@ function runWithEnvironment(
   validateUseMemo(hir);
 
   if (
+    env.isInferredMemoEnabled &&
     !env.config.enablePreserveExistingManualUseMemo &&
     !env.config.disableMemoizationForDebugging &&
-    !env.config.enableChangeDetectionForDebugging &&
-    !env.config.enableMinimalTransformsForRetry
+    !env.config.enableChangeDetectionForDebugging
   ) {
     dropManualMemoization(hir);
     log({kind: 'hir', name: 'DropManualMemoization', value: hir});
@@ -196,19 +200,20 @@ function runWithEnvironment(
   inferTypes(hir);
   log({kind: 'hir', name: 'InferTypes', value: hir});
 
-  if (env.config.validateHooksUsage) {
-    validateHooksUsage(hir);
+  if (env.isInferredMemoEnabled) {
+    if (env.config.validateHooksUsage) {
+      validateHooksUsage(hir);
+    }
+    if (env.config.validateNoCapitalizedCalls) {
+      validateNoCapitalizedCalls(hir);
+    }
   }
 
   if (env.config.enableFire) {
     transformFire(hir);
     log({kind: 'hir', name: 'TransformFire', value: hir});
   }
 
-  if (env.config.validateNoCapitalizedCalls) {
-    validateNoCapitalizedCalls(hir);
-  }
-
   if (env.config.lowerContextAccess) {
     lowerContextAccess(hir, env.config.lowerContextAccess);
   }
@@ -219,7 +224,12 @@ function runWithEnvironment(
   analyseFunctions(hir);
   log({kind: 'hir', name: 'AnalyseFunctions', value: hir});
 
-  inferReferenceEffects(hir);
+  const referenceEffectsResult = inferReferenceEffects(hir);
+  if (env.isInferredMemoEnabled) {
+    if (referenceEffectsResult.fnEffectErrors.length > 0) {
+      CompilerError.throw(referenceEffectsResult.fnEffectErrors[0]);
+    }
+  }
   log({kind: 'hir', name: 'InferReferenceEffects', value: hir});
 
   validateLocalsNotReassignedAfterRender(hir);
@@ -239,28 +249,30 @@ function runWithEnvironment(
   inferMutableRanges(hir);
   log({kind: 'hir', name: 'InferMutableRanges', value: hir});
 
-  if (env.config.assertValidMutableRanges) {
-    assertValidMutableRanges(hir);
-  }
+  if (env.isInferredMemoEnabled) {
+    if (env.config.assertValidMutableRanges) {
+      assertValidMutableRanges(hir);
+    }
 
-  if (env.config.validateRefAccessDuringRender) {
-    validateNoRefAccessInRender(hir);
-  }
+    if (env.config.validateRefAccessDuringRender) {
+      validateNoRefAccessInRender(hir);
+    }
 
-  if (env.config.validateNoSetStateInRender) {
-    validateNoSetStateInRender(hir);
-  }
+    if (env.config.validateNoSetStateInRender) {
+      validateNoSetStateInRender(hir);
+    }
 
-  if (env.config.validateNoSetStateInPassiveEffects) {
-    validateNoSetStateInPassiveEffects(hir);
-  }
+    if (env.config.validateNoSetStateInPassiveEffects) {
+      validateNoSetStateInPassiveEffects(hir);
+    }
 
-  if (env.config.validateNoJSXInTryStatements) {
-    validateNoJSXInTryStatement(hir);
-  }
+    if (env.config.validateNoJSXInTryStatements) {
+      validateNoJSXInTryStatement(hir);
+    }
 
-  if (env.config.validateNoImpureFunctionsInRender) {
-    validateNoImpureFunctionsInRender(hir);
+    if (env.config.validateNoImpureFunctionsInRender) {
+      validateNoImpureFunctionsInRender(hir);
+    }
   }
 
   inferReactivePlaces(hir);
@@ -280,7 +292,12 @@ function runWithEnvironment(
     value: hir,
   });
 
-  if (!env.config.enableMinimalTransformsForRetry) {
+  if (env.isInferredMemoEnabled) {
+    /**
+     * Only create reactive scopes (which directly map to generated memo blocks)
+     * if inferred memoization is enabled. This makes all later passes which
+     * transform reactive-scope labeled instructions no-ops.
+     */
     inferReactiveScopeVariables(hir);
     log({kind: 'hir', name: 'InferReactiveScopeVariables', value: hir});
   }
@@ -529,6 +546,7 @@ export function compileFn(
   >,
   config: EnvironmentConfig,
   fnType: ReactFunctionType,
+  mode: CompilerMode,
   useMemoCacheIdentifier: string,
   logger: Logger | null,
   filename: string | null,
@@ -538,6 +556,7 @@ export function compileFn(
     func,
     config,
     fnType,
+    mode,
     useMemoCacheIdentifier,
     logger,
     filename,

diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts
@@ -16,7 +16,6 @@ import {
   EnvironmentConfig,
   ExternalFunction,
   ReactFunctionType,
-  MINIMAL_RETRY_CONFIG,
   tryParseExternalFunction,
 } from '../HIR/Environment';
 import {CodegenFunction} from '../ReactiveScopes';
@@ -408,6 +407,7 @@ export function compileProgram(
             fn,
             environment,
             fnType,
+            'all_features',
             useMemoCacheIdentifier.name,
             pass.opts.logger,
             pass.filename,
@@ -425,11 +425,9 @@ export function compileProgram(
           kind: 'compile',
           compiledFn: compileFn(
             fn,
-            {
-              ...environment,
-              ...MINIMAL_RETRY_CONFIG,
-            },
+            environment,
             fnType,
+            'no_inferred_memo',
             useMemoCacheIdentifier.name,
             pass.opts.logger,
             pass.filename,

diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts
@@ -96,6 +96,8 @@ export const MacroSchema = z.union([
   z.tuple([z.string(), z.array(MacroMethodSchema)]),
 ]);
 
+export type CompilerMode = 'all_features' | 'no_inferred_memo';
+
 export type Macro = z.infer<typeof MacroSchema>;
 export type MacroMethod = z.infer<typeof MacroMethodSchema>;
 
@@ -550,8 +552,6 @@ const EnvironmentConfigSchema = z.object({
    */
   disableMemoizationForDebugging: z.boolean().default(false),
 
-  enableMinimalTransformsForRetry: z.boolean().default(false),
-
   /**
    * When true, rather using memoized values, the compiler will always re-compute
    * values, and then use a heuristic to compare the memoized value to the newly
@@ -626,17 +626,6 @@ const EnvironmentConfigSchema = z.object({
 
 export type EnvironmentConfig = z.infer<typeof EnvironmentConfigSchema>;
 
-export const MINIMAL_RETRY_CONFIG: PartialEnvironmentConfig = {
-  validateHooksUsage: false,
-  validateRefAccessDuringRender: false,
-  validateNoSetStateInRender: false,
-  validateNoSetStateInPassiveEffects: false,
-  validateNoJSXInTryStatements: false,
-  validateMemoizedEffectDependencies: false,
-  validateNoCapitalizedCalls: null,
-  validateBlocklistedImports: null,
-  enableMinimalTransformsForRetry: true,
-};
 /**
  * For test fixtures and playground only.
  *
@@ -851,6 +840,7 @@ export class Environment {
   code: string | null;
   config: EnvironmentConfig;
   fnType: ReactFunctionType;
+  compilerMode: CompilerMode;
   useMemoCacheIdentifier: string;
   hasLoweredContextAccess: boolean;
   hasFireRewrite: boolean;
@@ -861,6 +851,7 @@ export class Environment {
   constructor(
     scope: BabelScope,
     fnType: ReactFunctionType,
+    compilerMode: CompilerMode,
     config: EnvironmentConfig,
     contextIdentifiers: Set<t.Identifier>,
     logger: Logger | null,
@@ -870,6 +861,7 @@ export class Environment {
   ) {
     this.#scope = scope;
     this.fnType = fnType;
+    this.compilerMode = compilerMode;
     this.config = config;
     this.filename = filename;
     this.code = code;
@@ -924,6 +916,10 @@ export class Environment {
     this.#hoistedIdentifiers = new Set();
   }
 
+  get isInferredMemoEnabled(): boolean {
+    return this.compilerMode !== 'no_inferred_memo';
+  }
+
   get nextIdentifierId(): IdentifierId {
     return makeIdentifierId(this.#nextIdentifer++);
   }

diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferFunctionEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferFunctionEffects.ts
@@ -5,7 +5,12 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import {CompilerError, ErrorSeverity, ValueKind} from '..';
+import {
+  CompilerError,
+  CompilerErrorDetailOptions,
+  ErrorSeverity,
+  ValueKind,
+} from '..';
 import {
   AbstractValue,
   BasicBlock,
@@ -290,21 +295,21 @@ export function inferTerminalFunctionEffects(
   return functionEffects;
 }
 
-export function raiseFunctionEffectErrors(
+export function transformFunctionEffectErrors(
   functionEffects: Array<FunctionEffect>,
-): void {
-  functionEffects.forEach(eff => {
+): Array<CompilerErrorDetailOptions> {
+  return functionEffects.map(eff => {
     switch (eff.kind) {
       case 'ReactMutation':
       case 'GlobalMutation': {
-        CompilerError.throw(eff.error);
+        return eff.error;
       }
       case 'ContextMutation': {
-        CompilerError.throw({
+        return {
           severity: ErrorSeverity.Invariant,
           reason: `Unexpected ContextMutation in top-level function effects`,
           loc: eff.loc,
-        });
+        };
       }
       default:
         assertExhaustive(

diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts
@@ -5,7 +5,7 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import {CompilerError} from '../CompilerError';
+import {CompilerError, CompilerErrorDetailOptions} from '../CompilerError';
 import {Environment} from '../HIR';
 import {
   AbstractValue,
@@ -49,7 +49,7 @@ import {assertExhaustive} from '../Utils/utils';
 import {
   inferTerminalFunctionEffects,
   inferInstructionFunctionEffects,
-  raiseFunctionEffectErrors,
+  transformFunctionEffectErrors,
 } from './InferFunctionEffects';
 
 const UndefinedValue: InstructionValue = {
@@ -103,7 +103,9 @@ const UndefinedValue: InstructionValue = {
 export default function inferReferenceEffects(
   fn: HIRFunction,
   options: {isFunctionExpression: boolean} = {isFunctionExpression: false},
-): void {
+): {
+  fnEffectErrors: Array<CompilerErrorDetailOptions>;
+} {
   /*
    * Initial state contains function params
    * TODO: include module declarations here as well
@@ -241,8 +243,13 @@ export default function inferReferenceEffects(
 
   if (options.isFunctionExpression) {
     fn.effects = functionEffects;
-  } else if (!fn.env.config.enableMinimalTransformsForRetry) {
-    raiseFunctionEffectErrors(functionEffects);
+    return {
+      fnEffectErrors: [],
+    };
+  } else {
+    return {
+      fnEffectErrors: transformFunctionEffectErrors(functionEffects),
+    };
   }
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -181,6 +181,7 @@ function ReactForgetFunctionTransform() { @@
             fn,
             forgetOptions,
             'Other',
+            'all_features',
             '_c',
             null,
             null,
@@ Expand Down @@