From 5069e18060e00d7c07b2b04ebc8a3fa21e2d810a Mon Sep 17 00:00:00 2001 From: mofeiZ <34200447+mofeiZ@users.noreply.github.com> Date: Fri, 9 May 2025 13:23:08 -0400 Subject: [PATCH 1/2] [compiler][be] Make program traversal more readable (#33147) React Compiler's program traversal logic is pretty lengthy and complex as we've added a lot of features piecemeal. `compileProgram` is 300+ lines long and has confusing control flow (defining helpers inline, invoking visitors, mutating-asts-while-iterating, mutating global `ALREADY_COMPILED` state). - Moved more stuff to `ProgramContext` - Separated `compileProgram` into a bunch of helpers Tested by syncing this stack to a Meta codebase and observing no compilation output changes (D74487851, P1806855669, P1806855379) --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33147). * #33149 * #33148 * __->__ #33147 --- .../src/Entrypoint/Imports.ts | 62 +- .../src/Entrypoint/Program.ts | 539 ++++++++++-------- .../ValidateNoUntransformedReferences.ts | 6 +- .../no-emit-lint-repro.expect.md | 0 .../{ => no-emit}/no-emit-lint-repro.js | 0 .../no-emit/retry-no-emit.expect.md | 64 +++ .../no-emit/retry-no-emit.js | 19 + .../no-emit/retry-opt-in--no-emit.expect.md | 60 ++ .../no-emit/retry-opt-in--no-emit.js | 21 + 9 files changed, 521 insertions(+), 250 deletions(-) rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/{ => no-emit}/no-emit-lint-repro.expect.md (100%) rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/{ => no-emit}/no-emit-lint-repro.js (100%) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/retry-no-emit.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/retry-no-emit.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/retry-opt-in--no-emit.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/retry-opt-in--no-emit.js 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 704f696b3b86f..d5cac921e980c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Imports.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Imports.ts @@ -18,8 +18,9 @@ import { import {getOrInsertWith} from '../Utils/utils'; import {ExternalFunction, isHookName} from '../HIR/Environment'; import {Err, Ok, Result} from '../Utils/Result'; -import {CompilerReactTarget} from './Options'; -import {getReactCompilerRuntimeModule} from './Program'; +import {LoggerEvent, PluginOptions} from './Options'; +import {BabelFn, getReactCompilerRuntimeModule} from './Program'; +import {SuppressionRange} from './Suppression'; export function validateRestrictedImports( path: NodePath, @@ -52,32 +53,61 @@ export function validateRestrictedImports( } } +type ProgramContextOptions = { + program: NodePath; + suppressions: Array; + opts: PluginOptions; + filename: string | null; + code: string | null; +}; export class ProgramContext { - /* Program and environment context */ + /** + * Program and environment context + */ scope: BabelScope; + opts: PluginOptions; + filename: string | null; + code: string | null; reactRuntimeModule: string; - hookPattern: string | null; + suppressions: Array; + /* + * This is a hack to work around what seems to be a Babel bug. Babel doesn't + * consistently respect the `skip()` function to avoid revisiting a node within + * a pass, so we use this set to track nodes that we have compiled. + */ + alreadyCompiled: WeakSet | Set = new (WeakSet ?? Set)(); // known generated or referenced identifiers in the program knownReferencedNames: Set = new Set(); // generated imports imports: Map> = new Map(); - constructor( - program: NodePath, - reactRuntimeModule: CompilerReactTarget, - hookPattern: string | null, - ) { - this.hookPattern = hookPattern; + /** + * Metadata from compilation + */ + retryErrors: Array<{fn: BabelFn; error: CompilerError}> = []; + inferredEffectLocations: Set = new Set(); + + constructor({ + program, + suppressions, + opts, + filename, + code, + }: ProgramContextOptions) { this.scope = program.scope; - this.reactRuntimeModule = getReactCompilerRuntimeModule(reactRuntimeModule); + this.opts = opts; + this.filename = filename; + this.code = code; + this.reactRuntimeModule = getReactCompilerRuntimeModule(opts.target); + this.suppressions = suppressions; } isHookName(name: string): boolean { - if (this.hookPattern == null) { + if (this.opts.environment.hookPattern == null) { return isHookName(name); } else { - const match = new RegExp(this.hookPattern).exec(name); + const match = new RegExp(this.opts.environment.hookPattern).exec(name); return ( match != null && typeof match[1] === 'string' && isHookName(match[1]) ); @@ -179,6 +209,12 @@ export class ProgramContext { }); return Err(error); } + + logEvent(event: LoggerEvent): void { + if (this.opts.logger != null) { + this.opts.logger.logEvent(this.filename, event); + } + } } function getExistingImports( 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 34347f10b8fdf..1a3445531dfcf 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts @@ -12,7 +12,7 @@ import { CompilerErrorDetail, ErrorSeverity, } from '../CompilerError'; -import {EnvironmentConfig, ReactFunctionType} from '../HIR/Environment'; +import {ReactFunctionType} from '../HIR/Environment'; import {CodegenFunction} from '../ReactiveScopes'; import {isComponentDeclaration} from '../Utils/ComponentDeclaration'; import {isHookDeclaration} from '../Utils/HookDeclaration'; @@ -43,17 +43,21 @@ export const OPT_OUT_DIRECTIVES = new Set(['use no forget', 'use no memo']); export function findDirectiveEnablingMemoization( directives: Array, -): Array { - return directives.filter(directive => - OPT_IN_DIRECTIVES.has(directive.value.value), +): t.Directive | null { + return ( + directives.find(directive => + OPT_IN_DIRECTIVES.has(directive.value.value), + ) ?? null ); } export function findDirectiveDisablingMemoization( directives: Array, -): Array { - return directives.filter(directive => - OPT_OUT_DIRECTIVES.has(directive.value.value), +): t.Directive | null { + return ( + directives.find(directive => + OPT_OUT_DIRECTIVES.has(directive.value.value), + ) ?? null ); } @@ -88,13 +92,16 @@ export type CompileResult = { function logError( err: unknown, - pass: CompilerPass, + context: { + opts: PluginOptions; + filename: string | null; + }, fnLoc: t.SourceLocation | null, ): void { - if (pass.opts.logger) { + if (context.opts.logger) { if (err instanceof CompilerError) { for (const detail of err.details) { - pass.opts.logger.logEvent(pass.filename, { + context.opts.logger.logEvent(context.filename, { kind: 'CompileError', fnLoc, detail: detail.options, @@ -108,7 +115,7 @@ function logError( stringifiedError = err?.toString() ?? '[ null ]'; } - pass.opts.logger.logEvent(pass.filename, { + context.opts.logger.logEvent(context.filename, { kind: 'PipelineError', fnLoc, data: stringifiedError, @@ -118,13 +125,17 @@ function logError( } function handleError( err: unknown, - pass: CompilerPass, + context: { + opts: PluginOptions; + filename: string | null; + }, fnLoc: t.SourceLocation | null, ): void { - logError(err, pass, fnLoc); + logError(err, context, fnLoc); if ( - pass.opts.panicThreshold === 'all_errors' || - (pass.opts.panicThreshold === 'critical_errors' && isCriticalError(err)) || + context.opts.panicThreshold === 'all_errors' || + (context.opts.panicThreshold === 'critical_errors' && + isCriticalError(err)) || isConfigError(err) // Always throws regardless of panic threshold ) { throw err; @@ -187,7 +198,6 @@ export function createNewFunctionNode( } } // Avoid visiting the new transformed version - ALREADY_COMPILED.add(transformedFn); return transformedFn; } @@ -239,13 +249,6 @@ function insertNewOutlinedFunctionNode( } } -/* - * This is a hack to work around what seems to be a Babel bug. Babel doesn't - * consistently respect the `skip()` function to avoid revisiting a node within - * a pass, so we use this set to track nodes that we have compiled. - */ -const ALREADY_COMPILED: WeakSet | Set = new (WeakSet ?? Set)(); - const DEFAULT_ESLINT_SUPPRESSIONS = [ 'react-hooks/exhaustive-deps', 'react-hooks/rules-of-hooks', @@ -268,41 +271,43 @@ function isFilePartOfSources( return false; } -export type CompileProgramResult = { +export type CompileProgramMetadata = { retryErrors: Array<{fn: BabelFn; error: CompilerError}>; inferredEffectLocations: Set; }; /** - * `compileProgram` is directly invoked by the react-compiler babel plugin, so - * exceptions thrown by this function will fail the babel build. - * - call `handleError` if your error is recoverable. - * Unless the error is a warning / info diagnostic, compilation of a function - * / entire file should also be skipped. - * - throw an exception if the error is fatal / not recoverable. - * Examples of this are invalid compiler configs or failure to codegen outlined - * functions *after* already emitting optimized components / hooks that invoke - * the outlined functions. + * Main entrypoint for React Compiler. + * + * @param program The Babel program node to compile + * @param pass Compiler configuration and context + * @returns Compilation results or null if compilation was skipped */ export function compileProgram( program: NodePath, pass: CompilerPass, -): CompileProgramResult | null { +): CompileProgramMetadata | null { + /** + * This is directly invoked by the react-compiler babel plugin, so exceptions + * thrown by this function will fail the babel build. + * - call `handleError` if your error is recoverable. + * Unless the error is a warning / info diagnostic, compilation of a function + * / entire file should also be skipped. + * - throw an exception if the error is fatal / not recoverable. + * Examples of this are invalid compiler configs or failure to codegen outlined + * functions *after* already emitting optimized components / hooks that invoke + * the outlined functions. + */ if (shouldSkipCompilation(program, pass)) { return null; } - - const environment = pass.opts.environment; - const restrictedImportsErr = validateRestrictedImports(program, environment); + const restrictedImportsErr = validateRestrictedImports( + program, + pass.opts.environment, + ); if (restrictedImportsErr) { handleError(restrictedImportsErr, pass, null); return null; } - - const programContext = new ProgramContext( - program, - pass.opts.target, - environment.hookPattern, - ); /* * Record lint errors and critical errors as depending on Forget's config, * we may still need to run Forget's analysis on every function (even if we @@ -313,16 +318,88 @@ export function compileProgram( pass.opts.eslintSuppressionRules ?? DEFAULT_ESLINT_SUPPRESSIONS, pass.opts.flowSuppressions, ); - const queue: Array<{ - kind: 'original' | 'outlined'; - fn: BabelFn; - fnType: ReactFunctionType; - }> = []; + + const programContext = new ProgramContext({ + program: program, + opts: pass.opts, + filename: pass.filename, + code: pass.code, + suppressions, + }); + + const queue: Array = findFunctionsToCompile( + program, + pass, + programContext, + ); const compiledFns: Array = []; + while (queue.length !== 0) { + const current = queue.shift()!; + const compiled = processFn(current.fn, current.fnType, programContext); + + if (compiled != null) { + for (const outlined of compiled.outlined) { + CompilerError.invariant(outlined.fn.outlined.length === 0, { + reason: 'Unexpected nested outlined functions', + loc: outlined.fn.loc, + }); + const fn = insertNewOutlinedFunctionNode( + program, + current.fn, + outlined.fn, + ); + fn.skip(); + programContext.alreadyCompiled.add(fn.node); + if (outlined.type !== null) { + queue.push({ + kind: 'outlined', + fn, + fnType: outlined.type, + }); + } + } + compiledFns.push({ + kind: current.kind, + originalFn: current.fn, + compiledFn: compiled, + }); + } + } + + // Avoid modifying the program if we find a program level opt-out + if (findDirectiveDisablingMemoization(program.node.directives) != null) { + return null; + } + + // Insert React Compiler generated functions into the Babel AST + applyCompiledFunctions(program, compiledFns, pass, programContext); + + return { + retryErrors: programContext.retryErrors, + inferredEffectLocations: programContext.inferredEffectLocations, + }; +} + +type CompileSource = { + kind: 'original' | 'outlined'; + fn: BabelFn; + fnType: ReactFunctionType; +}; +/** + * Find all React components and hooks that need to be compiled + * + * @returns An array of React functions from @param program to transform + */ +function findFunctionsToCompile( + program: NodePath, + pass: CompilerPass, + programContext: ProgramContext, +): Array { + const queue: Array = []; const traverseFunction = (fn: BabelFn, pass: CompilerPass): void => { - const fnType = getReactFunctionType(fn, pass, environment); - if (fnType === null || ALREADY_COMPILED.has(fn.node)) { + const fnType = getReactFunctionType(fn, pass); + if (fnType === null || programContext.alreadyCompiled.has(fn.node)) { return; } @@ -331,7 +408,7 @@ export function compileProgram( * traversal will loop infinitely. * Ensure we avoid visiting the original function again. */ - ALREADY_COMPILED.add(fn.node); + programContext.alreadyCompiled.add(fn.node); fn.skip(); queue.push({kind: 'original', fn, fnType}); @@ -346,7 +423,6 @@ export function compileProgram( * can reference `this` which is unsafe for compilation */ node.skip(); - return; }, ClassExpression(node: NodePath) { @@ -355,7 +431,6 @@ export function compileProgram( * can reference `this` which is unsafe for compilation */ node.skip(); - return; }, FunctionDeclaration: traverseFunction, @@ -370,205 +445,205 @@ export function compileProgram( filename: pass.filename ?? null, }, ); - const retryErrors: Array<{fn: BabelFn; error: CompilerError}> = []; - const inferredEffectLocations = new Set(); - const processFn = ( - fn: BabelFn, - fnType: ReactFunctionType, - ): null | CodegenFunction => { - let optInDirectives: Array = []; - let optOutDirectives: Array = []; - if (fn.node.body.type === 'BlockStatement') { - optInDirectives = findDirectiveEnablingMemoization( - fn.node.body.directives, - ); - optOutDirectives = findDirectiveDisablingMemoization( - fn.node.body.directives, - ); - } + return queue; +} - /** - * Note that Babel does not attach comment nodes to nodes; they are dangling off of the - * Program node itself. We need to figure out whether an eslint suppression range - * applies to this function first. - */ - const suppressionsInFunction = filterSuppressionsThatAffectFunction( - suppressions, - fn, - ); - let compileResult: - | {kind: 'compile'; compiledFn: CodegenFunction} - | {kind: 'error'; error: unknown}; - if (suppressionsInFunction.length > 0) { - compileResult = { - kind: 'error', - error: suppressionsToCompilerError(suppressionsInFunction), - }; - } else { - try { - compileResult = { - kind: 'compile', - compiledFn: compileFn( - fn, - environment, - fnType, - 'all_features', - programContext, - pass.opts.logger, - pass.filename, - pass.code, - ), - }; - } catch (err) { - compileResult = {kind: 'error', error: err}; - } - } +/** + * Try to compile a source function, taking into account all local suppressions, + * opt-ins, and opt-outs. + * + * Errors encountered during compilation are either logged (if recoverable) or + * thrown (if non-recoverable). + * + * @returns the compiled function or null if the function was skipped (due to + * config settings and/or outputs) + */ +function processFn( + fn: BabelFn, + fnType: ReactFunctionType, + programContext: ProgramContext, +): null | CodegenFunction { + let directives; + if (fn.node.body.type !== 'BlockStatement') { + directives = {optIn: null, optOut: null}; + } else { + directives = { + optIn: findDirectiveEnablingMemoization(fn.node.body.directives), + optOut: findDirectiveDisablingMemoization(fn.node.body.directives), + }; + } - if (compileResult.kind === 'error') { - /** - * If an opt out directive is present, log only instead of throwing and don't mark as - * containing a critical error. - */ - if (optOutDirectives.length > 0) { - logError(compileResult.error, pass, fn.node.loc ?? null); - } else { - handleError(compileResult.error, pass, fn.node.loc ?? null); - } - // If non-memoization features are enabled, retry regardless of error kind - if ( - !(environment.enableFire || environment.inferEffectDependencies != null) - ) { - return null; - } - try { - compileResult = { - kind: 'compile', - compiledFn: compileFn( - fn, - environment, - fnType, - 'no_inferred_memo', - programContext, - pass.opts.logger, - pass.filename, - pass.code, - ), - }; - if ( - !compileResult.compiledFn.hasFireRewrite && - !compileResult.compiledFn.hasInferredEffect - ) { - return null; - } - } catch (err) { - // TODO: we might want to log error here, but this will also result in duplicate logging - if (err instanceof CompilerError) { - retryErrors.push({fn, error: err}); - } - return null; - } + let compiledFn: CodegenFunction; + const compileResult = tryCompileFunction(fn, fnType, programContext); + if (compileResult.kind === 'error') { + if (directives.optOut != null) { + logError(compileResult.error, programContext, fn.node.loc ?? null); + } else { + handleError(compileResult.error, programContext, fn.node.loc ?? null); } - - /** - * 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 (pass.opts.ignoreUseNoForget === false && optOutDirectives.length > 0) { - for (const directive of optOutDirectives) { - pass.opts.logger?.logEvent(pass.filename, { - kind: 'CompileSkip', - fnLoc: fn.node.body.loc ?? null, - reason: `Skipped due to '${directive.value.value}' directive.`, - loc: directive.loc ?? null, - }); - } + const retryResult = retryCompileFunction(fn, fnType, programContext); + if (retryResult == null) { return null; } + compiledFn = retryResult; + } else { + compiledFn = compileResult.compiledFn; + } - pass.opts.logger?.logEvent(pass.filename, { - kind: 'CompileSuccess', - fnLoc: fn.node.loc ?? null, - fnName: compileResult.compiledFn.id?.name ?? null, - memoSlots: compileResult.compiledFn.memoSlotsUsed, - memoBlocks: compileResult.compiledFn.memoBlocks, - memoValues: compileResult.compiledFn.memoValues, - prunedMemoBlocks: compileResult.compiledFn.prunedMemoBlocks, - prunedMemoValues: compileResult.compiledFn.prunedMemoValues, + /** + * 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 ( + programContext.opts.ignoreUseNoForget === false && + directives.optOut != null + ) { + programContext.logEvent({ + kind: 'CompileSkip', + fnLoc: fn.node.body.loc ?? null, + reason: `Skipped due to '${directives.optOut.value}' directive.`, + loc: directives.optOut.loc ?? null, }); + return null; + } + programContext.logEvent({ + kind: 'CompileSuccess', + fnLoc: fn.node.loc ?? null, + fnName: compiledFn.id?.name ?? null, + memoSlots: compiledFn.memoSlotsUsed, + memoBlocks: compiledFn.memoBlocks, + memoValues: compiledFn.memoValues, + prunedMemoBlocks: compiledFn.prunedMemoBlocks, + prunedMemoValues: compiledFn.prunedMemoValues, + }); + /** + * Always compile functions with opt in directives. + */ + if (directives.optIn != null) { + return compiledFn; + } else if (programContext.opts.compilationMode === 'annotation') { /** - * Always compile functions with opt in directives. + * If no opt-in directive is found and the compiler is configured in + * annotation mode, don't insert the compiled function. */ - if (optInDirectives.length > 0) { - return compileResult.compiledFn; - } else if (pass.opts.compilationMode === 'annotation') { - /** - * No opt-in directive in annotation mode, so don't insert the compiled function. - */ - return null; - } - - if (!pass.opts.noEmit) { - return compileResult.compiledFn; - } + return null; + } else if (programContext.opts.noEmit) { /** * inferEffectDependencies + noEmit is currently only used for linting. In * this mode, add source locations for where the compiler *can* infer effect * dependencies. */ - for (const loc of compileResult.compiledFn.inferredEffectLocations) { - if (loc !== GeneratedSource) inferredEffectLocations.add(loc); - } - return null; - }; - - while (queue.length !== 0) { - const current = queue.shift()!; - const compiled = processFn(current.fn, current.fnType); - if (compiled === null) { - continue; - } - for (const outlined of compiled.outlined) { - CompilerError.invariant(outlined.fn.outlined.length === 0, { - reason: 'Unexpected nested outlined functions', - loc: outlined.fn.loc, - }); - const fn = insertNewOutlinedFunctionNode( - program, - current.fn, - outlined.fn, - ); - fn.skip(); - ALREADY_COMPILED.add(fn.node); - if (outlined.type !== null) { - queue.push({ - kind: 'outlined', - fn, - fnType: outlined.type, - }); + for (const loc of compiledFn.inferredEffectLocations) { + if (loc !== GeneratedSource) { + programContext.inferredEffectLocations.add(loc); } } - compiledFns.push({ - kind: current.kind, - compiledFn: compiled, - originalFn: current.fn, - }); + return null; + } else { + return compiledFn; } +} +function tryCompileFunction( + fn: BabelFn, + fnType: ReactFunctionType, + programContext: ProgramContext, +): + | {kind: 'compile'; compiledFn: CodegenFunction} + | {kind: 'error'; error: unknown} { /** - * Do not modify source if there is a module scope level opt out directive. + * Note that Babel does not attach comment nodes to nodes; they are dangling off of the + * Program node itself. We need to figure out whether an eslint suppression range + * applies to this function first. */ - const moduleScopeOptOutDirectives = findDirectiveDisablingMemoization( - program.node.directives, + const suppressionsInFunction = filterSuppressionsThatAffectFunction( + programContext.suppressions, + fn, ); - if (moduleScopeOptOutDirectives.length > 0) { + if (suppressionsInFunction.length > 0) { + return { + kind: 'error', + error: suppressionsToCompilerError(suppressionsInFunction), + }; + } + + try { + return { + kind: 'compile', + compiledFn: compileFn( + fn, + programContext.opts.environment, + fnType, + 'all_features', + programContext, + programContext.opts.logger, + programContext.filename, + programContext.code, + ), + }; + } catch (err) { + return {kind: 'error', error: err}; + } +} + +/** + * If non-memo feature flags are enabled, retry compilation with a more minimal + * feature set. + * + * @returns a CodegenFunction if retry was successful + */ +function retryCompileFunction( + fn: BabelFn, + fnType: ReactFunctionType, + programContext: ProgramContext, +): CodegenFunction | null { + const environment = programContext.opts.environment; + if ( + !(environment.enableFire || environment.inferEffectDependencies != null) + ) { return null; } - /* - * Only insert Forget-ified functions if we have not encountered a critical - * error elsewhere in the file, regardless of bailout mode. + /** + * Note that function suppressions are not checked in the retry pipeline, as + * they only affect auto-memoization features. */ + try { + const retryResult = compileFn( + fn, + environment, + fnType, + 'no_inferred_memo', + programContext, + programContext.opts.logger, + programContext.filename, + programContext.code, + ); + + if (!retryResult.hasFireRewrite && !retryResult.hasInferredEffect) { + return null; + } + return retryResult; + } catch (err) { + // TODO: we might want to log error here, but this will also result in duplicate logging + if (err instanceof CompilerError) { + programContext.retryErrors.push({fn, error: err}); + } + return null; + } +} + +/** + * Applies React Compiler generated functions to the babel AST by replacing + * existing functions in place or inserting new declarations. + */ +function applyCompiledFunctions( + program: NodePath, + compiledFns: Array, + pass: CompilerPass, + programContext: ProgramContext, +): void { const referencedBeforeDeclared = pass.opts.gating != null ? getFunctionReferencedBeforeDeclarationAtTopLevel(program, compiledFns) @@ -576,6 +651,7 @@ export function compileProgram( for (const result of compiledFns) { const {kind, originalFn, compiledFn} = result; const transformedFn = createNewFunctionNode(originalFn, compiledFn); + programContext.alreadyCompiled.add(transformedFn); if (referencedBeforeDeclared != null && kind === 'original') { CompilerError.invariant(pass.opts.gating != null, { @@ -598,7 +674,6 @@ export function compileProgram( if (compiledFns.length > 0) { addImportsToProgram(program, programContext); } - return {retryErrors, inferredEffectLocations}; } function shouldSkipCompilation( @@ -640,14 +715,10 @@ function shouldSkipCompilation( function getReactFunctionType( fn: BabelFn, pass: CompilerPass, - /** - * TODO(mofeiZ): remove once we validate PluginOptions with Zod - */ - environment: EnvironmentConfig, ): ReactFunctionType | null { - const hookPattern = environment.hookPattern; + const hookPattern = pass.opts.environment.hookPattern; if (fn.node.body.type === 'BlockStatement') { - if (findDirectiveEnablingMemoization(fn.node.body.directives).length > 0) + if (findDirectiveEnablingMemoization(fn.node.body.directives) != null) return getComponentOrHookLike(fn, hookPattern) ?? 'Other'; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/ValidateNoUntransformedReferences.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/ValidateNoUntransformedReferences.ts index 5f6c6986e04fd..e288c227ad25c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/ValidateNoUntransformedReferences.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/ValidateNoUntransformedReferences.ts @@ -18,7 +18,7 @@ import { import {getOrInsertWith} from '../Utils/utils'; import {Environment} from '../HIR'; import {DEFAULT_EXPORT} from '../HIR/Environment'; -import {CompileProgramResult} from './Program'; +import {CompileProgramMetadata} from './Program'; function throwInvalidReact( options: Omit, @@ -109,7 +109,7 @@ export default function validateNoUntransformedReferences( filename: string | null, logger: Logger | null, env: EnvironmentConfig, - compileResult: CompileProgramResult | null, + compileResult: CompileProgramMetadata | null, ): void { const moduleLoadChecks = new Map< string, @@ -236,7 +236,7 @@ function transformProgram( moduleLoadChecks: Map>, filename: string | null, logger: Logger | null, - compileResult: CompileProgramResult | null, + compileResult: CompileProgramMetadata | null, ): void { const traversalState: TraversalState = { shouldInvalidateScopes: true, diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit-lint-repro.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/no-emit-lint-repro.expect.md similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit-lint-repro.expect.md rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/no-emit-lint-repro.expect.md diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit-lint-repro.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/no-emit-lint-repro.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit-lint-repro.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/no-emit-lint-repro.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/retry-no-emit.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/retry-no-emit.expect.md new file mode 100644 index 0000000000000..bd70c0138d7e2 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/retry-no-emit.expect.md @@ -0,0 +1,64 @@ + +## Input + +```javascript +// @inferEffectDependencies @noEmit @panicThreshold:"none" @loggerTestOnly +import {print} from 'shared-runtime'; +import useEffectWrapper from 'useEffectWrapper'; + +function Foo({propVal}) { + const arr = [propVal]; + useEffectWrapper(() => print(arr)); + + const arr2 = []; + useEffectWrapper(() => arr2.push(propVal)); + arr2.push(2); + return {arr, arr2}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{propVal: 1}], + sequentialRenders: [{propVal: 1}, {propVal: 2}], +}; + +``` + +## Code + +```javascript +// @inferEffectDependencies @noEmit @panicThreshold:"none" @loggerTestOnly +import { print } from "shared-runtime"; +import useEffectWrapper from "useEffectWrapper"; + +function Foo({ propVal }) { + const arr = [propVal]; + useEffectWrapper(() => print(arr)); + + const arr2 = []; + useEffectWrapper(() => arr2.push(propVal)); + arr2.push(2); + return { arr, arr2 }; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{ propVal: 1 }], + sequentialRenders: [{ propVal: 1 }, { propVal: 2 }], +}; + +``` + +## Logs + +``` +{"kind":"CompileError","fnLoc":{"start":{"line":5,"column":0,"index":163},"end":{"line":13,"column":1,"index":357},"filename":"retry-no-emit.ts"},"detail":{"reason":"This mutates a variable that React considers immutable","description":null,"loc":{"start":{"line":11,"column":2,"index":320},"end":{"line":11,"column":6,"index":324},"filename":"retry-no-emit.ts","identifierName":"arr2"},"suggestions":null,"severity":"InvalidReact"}} +{"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":7,"column":2,"index":216},"end":{"line":7,"column":36,"index":250},"filename":"retry-no-emit.ts"},"decorations":[{"start":{"line":7,"column":31,"index":245},"end":{"line":7,"column":34,"index":248},"filename":"retry-no-emit.ts","identifierName":"arr"}]} +{"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":10,"column":2,"index":274},"end":{"line":10,"column":44,"index":316},"filename":"retry-no-emit.ts"},"decorations":[{"start":{"line":10,"column":25,"index":297},"end":{"line":10,"column":29,"index":301},"filename":"retry-no-emit.ts","identifierName":"arr2"},{"start":{"line":10,"column":25,"index":297},"end":{"line":10,"column":29,"index":301},"filename":"retry-no-emit.ts","identifierName":"arr2"},{"start":{"line":10,"column":35,"index":307},"end":{"line":10,"column":42,"index":314},"filename":"retry-no-emit.ts","identifierName":"propVal"}]} +{"kind":"CompileSuccess","fnLoc":{"start":{"line":5,"column":0,"index":163},"end":{"line":13,"column":1,"index":357},"filename":"retry-no-emit.ts"},"fnName":"Foo","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0} +``` + +### Eval output +(kind: ok) {"arr":[1],"arr2":[2]} +{"arr":[2],"arr2":[2]} +logs: [[ 1 ],[ 2 ]] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/retry-no-emit.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/retry-no-emit.js new file mode 100644 index 0000000000000..d1dda06a044a2 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/retry-no-emit.js @@ -0,0 +1,19 @@ +// @inferEffectDependencies @noEmit @panicThreshold:"none" @loggerTestOnly +import {print} from 'shared-runtime'; +import useEffectWrapper from 'useEffectWrapper'; + +function Foo({propVal}) { + const arr = [propVal]; + useEffectWrapper(() => print(arr)); + + const arr2 = []; + useEffectWrapper(() => arr2.push(propVal)); + arr2.push(2); + return {arr, arr2}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{propVal: 1}], + sequentialRenders: [{propVal: 1}, {propVal: 2}], +}; 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 new file mode 100644 index 0000000000000..8d2e5c7c31fe4 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/retry-opt-in--no-emit.expect.md @@ -0,0 +1,60 @@ + +## Input + +```javascript +// @compilationMode:"all" @inferEffectDependencies @panicThreshold:"none" @noEmit +import {print} from 'shared-runtime'; +import useEffectWrapper from 'useEffectWrapper'; + +function Foo({propVal}) { + 'use memo'; + const arr = [propVal]; + useEffectWrapper(() => print(arr)); + + const arr2 = []; + useEffectWrapper(() => arr2.push(propVal)); + arr2.push(2); + + return {arr, arr2}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{propVal: 1}], + sequentialRenders: [{propVal: 1}, {propVal: 2}], +}; + +``` + +## Code + +```javascript +// @compilationMode:"all" @inferEffectDependencies @panicThreshold:"none" @noEmit +import { print } from "shared-runtime"; +import useEffectWrapper from "useEffectWrapper"; + +function Foo(t0) { + "use memo"; + const { propVal } = t0; + + const arr = [propVal]; + useEffectWrapper(() => print(arr), [arr]); + + const arr2 = []; + useEffectWrapper(() => arr2.push(propVal), [arr2, propVal]); + arr2.push(2); + return { arr, arr2 }; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{ propVal: 1 }], + sequentialRenders: [{ propVal: 1 }, { propVal: 2 }], +}; + +``` + +### Eval output +(kind: ok) {"arr":[1],"arr2":[2]} +{"arr":[2],"arr2":[2]} +logs: [[ 1 ],[ 2 ]] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/retry-opt-in--no-emit.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/retry-opt-in--no-emit.js new file mode 100644 index 0000000000000..2650cbe5d7f38 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/retry-opt-in--no-emit.js @@ -0,0 +1,21 @@ +// @compilationMode:"all" @inferEffectDependencies @panicThreshold:"none" @noEmit +import {print} from 'shared-runtime'; +import useEffectWrapper from 'useEffectWrapper'; + +function Foo({propVal}) { + 'use memo'; + const arr = [propVal]; + useEffectWrapper(() => print(arr)); + + const arr2 = []; + useEffectWrapper(() => arr2.push(propVal)); + arr2.push(2); + + return {arr, arr2}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{propVal: 1}], + sequentialRenders: [{propVal: 1}, {propVal: 2}], +}; From 3820740a7fbfc3b27a5127b43bdad44382ff3ce0 Mon Sep 17 00:00:00 2001 From: mofeiZ <34200447+mofeiZ@users.noreply.github.com> Date: Fri, 9 May 2025 13:37:49 -0400 Subject: [PATCH 2/2] [compiler][entrypoint] Fix edgecases for noEmit and opt-outs (#33148) Title --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33148). * #33149 * __->__ #33148 --- .../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 = {