From d5358f8af6b1d8a313a4dcaf411afbee198da330 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Thu, 8 May 2025 11:27:18 -0400 Subject: [PATCH] [compiler][entrypoint] Fix edgecases for noEmit and opt-outs Title --- .../src/Entrypoint/Imports.ts | 9 +++- .../src/Entrypoint/Program.ts | 47 ++++++++++++------- ...bailout-nopanic-shouldnt-outline.expect.md | 3 -- .../compiler/use-memo-noemit.expect.md | 15 +----- 4 files changed, 40 insertions(+), 34 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Imports.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Imports.ts index d5cac921e9..d8f3ad0904 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Imports.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Imports.ts @@ -19,7 +19,11 @@ import {getOrInsertWith} from '../Utils/utils'; import {ExternalFunction, isHookName} from '../HIR/Environment'; import {Err, Ok, Result} from '../Utils/Result'; import {LoggerEvent, PluginOptions} from './Options'; -import {BabelFn, getReactCompilerRuntimeModule} from './Program'; +import { + BabelFn, + findDirectiveDisablingMemoization, + getReactCompilerRuntimeModule, +} from './Program'; import {SuppressionRange} from './Suppression'; export function validateRestrictedImports( @@ -70,6 +74,7 @@ export class ProgramContext { code: string | null; reactRuntimeModule: string; suppressions: Array; + hasModuleScopeOptOut: boolean; /* * This is a hack to work around what seems to be a Babel bug. Babel doesn't @@ -101,6 +106,8 @@ export class ProgramContext { this.code = code; this.reactRuntimeModule = getReactCompilerRuntimeModule(opts.target); this.suppressions = suppressions; + this.hasModuleScopeOptOut = + findDirectiveDisablingMemoization(program.node.directives) != null; } isHookName(name: string): boolean { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts index 58e3c7de01..6ed9295bc8 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts @@ -368,7 +368,19 @@ export function compileProgram( } // Avoid modifying the program if we find a program level opt-out - if (findDirectiveDisablingMemoization(program.node.directives) != null) { + if (programContext.hasModuleScopeOptOut) { + if (compiledFns.length > 0) { + const error = new CompilerError(); + error.pushErrorDetail( + new CompilerErrorDetail({ + reason: + 'Unexpected compiled functions when module scope opt-out is present', + severity: ErrorSeverity.Invariant, + loc: null, + }), + ); + handleError(error, programContext, null); + } return null; } @@ -513,21 +525,6 @@ function processFn( prunedMemoValues: compiledFn.prunedMemoValues, }); - /** - * Always compile functions with opt in directives. - */ - if (directives.optIn != null) { - return compiledFn; - } else if (programContext.opts.compilationMode === 'annotation') { - /** - * If no opt-in directive is found and the compiler is configured in - * annotation mode, don't insert the compiled function. - */ - return null; - } else if (!programContext.opts.noEmit) { - return compiledFn; - } - /** * inferEffectDependencies + noEmit is currently only used for linting. In * this mode, add source locations for where the compiler *can* infer effect @@ -538,7 +535,23 @@ function processFn( programContext.inferredEffectLocations.add(loc); } } - return null; + + /** + * Always compile functions with opt in directives. + */ + if (programContext.hasModuleScopeOptOut || programContext.opts.noEmit) { + return null; + } else if (directives.optIn != null) { + return compiledFn; + } else if (programContext.opts.compilationMode === 'annotation') { + /** + * If no opt-in directive is found and the compiler is configured in + * annotation mode, don't insert the compiled function. + */ + return null; + } else { + return compiledFn; + } } function tryCompileFunction( diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-bailout-nopanic-shouldnt-outline.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-bailout-nopanic-shouldnt-outline.expect.md index f24d949205..cfbaa34568 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-bailout-nopanic-shouldnt-outline.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-bailout-nopanic-shouldnt-outline.expect.md @@ -20,9 +20,6 @@ function Foo() { function Foo() { return ; } -function _temp() { - return alert("hello!"); -} ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-memo-noemit.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-memo-noemit.expect.md index dfc831555f..c47501945b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-memo-noemit.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-memo-noemit.expect.md @@ -19,22 +19,11 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; // @noEmit +// @noEmit function Foo() { "use memo"; - const $ = _c(1); - let t0; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - t0 = ; - $[0] = t0; - } else { - t0 = $[0]; - } - return t0; -} -function _temp() { - return alert("hello!"); + return ; } export const FIXTURE_ENTRYPOINT = {