From 1b658717e901b18050fe647ac18bcbc099380219 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Fri, 9 May 2025 13:30:42 -0400 Subject: [PATCH] [compiler][entrypoint] Fix edgecases for noEmit and opt-outs Title --- .../src/Entrypoint/Imports.ts | 4 ++ .../src/Entrypoint/Program.ts | 43 +++++++++++++------ .../no-emit/retry-opt-in--no-emit.expect.md | 9 ++-- ...bailout-nopanic-shouldnt-outline.expect.md | 3 -- .../compiler/use-memo-noemit.expect.md | 15 +------ 5 files changed, 39 insertions(+), 35 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 d5cac921e980c..24ce37cf72c29 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Imports.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Imports.ts @@ -59,6 +59,7 @@ type ProgramContextOptions = { opts: PluginOptions; filename: string | null; code: string | null; + hasModuleScopeOptOut: boolean; }; export class ProgramContext { /** @@ -70,6 +71,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 @@ -94,6 +96,7 @@ export class ProgramContext { opts, filename, code, + hasModuleScopeOptOut, }: ProgramContextOptions) { this.scope = program.scope; this.opts = opts; @@ -101,6 +104,7 @@ export class ProgramContext { this.code = code; this.reactRuntimeModule = getReactCompilerRuntimeModule(opts.target); this.suppressions = suppressions; + this.hasModuleScopeOptOut = hasModuleScopeOptOut; } 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 1a3445531dfcf..64abc110ea12f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts @@ -325,6 +325,8 @@ export function compileProgram( filename: pass.filename, code: pass.code, suppressions, + hasModuleScopeOptOut: + findDirectiveDisablingMemoization(program.node.directives) != null, }); const queue: Array = findFunctionsToCompile( @@ -368,7 +370,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; } @@ -491,9 +505,10 @@ function processFn( } /** - * Otherwise if 'use no forget/memo' is present, we still run the code through the compiler - * for validation but we don't mutate the babel AST. This allows us to flag if there is an - * unused 'use no forget/memo' directive. + * If 'use no forget/memo' is present and we still ran the code through the + * compiler for validation, log a skip event and don't mutate the babel AST. + * This allows us to flag if there is an unused 'use no forget/memo' + * directive. */ if ( programContext.opts.ignoreUseNoForget === false && @@ -518,16 +533,7 @@ 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. - */ + if (programContext.hasModuleScopeOptOut) { return null; } else if (programContext.opts.noEmit) { /** @@ -541,6 +547,15 @@ function processFn( } } return null; + } else if ( + programContext.opts.compilationMode === 'annotation' && + directives.optIn == null + ) { + /** + * 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; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/retry-opt-in--no-emit.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/retry-opt-in--no-emit.expect.md index 8d2e5c7c31fe4..6e4ddeeb2b59e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/retry-opt-in--no-emit.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/retry-opt-in--no-emit.expect.md @@ -33,16 +33,15 @@ export const FIXTURE_ENTRYPOINT = { import { print } from "shared-runtime"; import useEffectWrapper from "useEffectWrapper"; -function Foo(t0) { +function Foo({ propVal }) { "use memo"; - const { propVal } = t0; - const arr = [propVal]; - useEffectWrapper(() => print(arr), [arr]); + useEffectWrapper(() => print(arr)); const arr2 = []; - useEffectWrapper(() => arr2.push(propVal), [arr2, propVal]); + useEffectWrapper(() => arr2.push(propVal)); arr2.push(2); + return { arr, arr2 }; } 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 f24d9492058fa..cfbaa34568245 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 dfc831555f026..c47501945b345 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 = {