From 7939d92fcc95ad5ee719c38272eaef14a3750fc0 Mon Sep 17 00:00:00 2001 From: mofeiZ <34200447+mofeiZ@users.noreply.github.com> Date: Thu, 13 Mar 2025 19:54:54 -0400 Subject: [PATCH 1/2] [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 --- .../scripts/jest/makeTransform.ts | 1 + .../src/Entrypoint/Pipeline.ts | 73 ++++++++++++------- .../src/Entrypoint/Program.ts | 48 ++++++------ .../src/HIR/Environment.ts | 22 +++--- .../src/Inference/InferFunctionEffects.ts | 19 +++-- .../src/Inference/InferReferenceEffects.ts | 11 +-- .../bailout-capitalized-fn-call.expect.md | 4 +- .../bailout-capitalized-fn-call.js | 2 +- .../bailout-eslint-suppressions.expect.md | 4 +- .../bailout-eslint-suppressions.js | 2 +- .../bailout-validate-preserve-memo.expect.md | 4 +- .../bailout-validate-preserve-memo.js | 2 +- .../bailout-validate-prop-write.expect.md | 4 +- .../bailout-validate-prop-write.js | 2 +- ...lout-validate-ref-current-access.expect.md | 2 +- .../bailout-validate-ref-current-access.js | 2 +- ...ailout-validate-conditional-hook.expect.md | 4 +- .../bailout-validate-conditional-hook.js | 2 +- 18 files changed, 115 insertions(+), 93 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 index 3ebd28f849..99291c2984 100644 --- 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 index 0953289c19..994aa8f18c 100644 --- 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,8 +200,13 @@ 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) { @@ -205,10 +214,6 @@ function runWithEnvironment( 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 fnEffectErrors = inferReferenceEffects(hir); + if (env.isInferredMemoEnabled) { + if (fnEffectErrors.length > 0) { + CompilerError.throw(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 index 34c1af955b..0865b50f84 100644 --- 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, } from '../HIR/Environment'; import {CodegenFunction} from '../ReactiveScopes'; import {isComponentDeclaration} from '../Utils/ComponentDeclaration'; @@ -407,6 +406,7 @@ export function compileProgram( fn, environment, fnType, + 'all_features', useMemoCacheIdentifier.name, pass.opts.logger, pass.filename, @@ -417,28 +417,7 @@ export function compileProgram( compileResult = {kind: 'error', error: err}; } } - // If non-memoization features are enabled, retry regardless of error kind - if (compileResult.kind === 'error' && environment.enableFire) { - try { - compileResult = { - kind: 'compile', - compiledFn: compileFn( - fn, - { - ...environment, - ...MINIMAL_RETRY_CONFIG, - }, - fnType, - useMemoCacheIdentifier.name, - pass.opts.logger, - pass.filename, - pass.code, - ), - }; - } catch (err) { - compileResult = {kind: 'error', error: err}; - } - } + if (compileResult.kind === 'error') { /** * If an opt out directive is present, log only instead of throwing and don't mark as @@ -449,7 +428,28 @@ export function compileProgram( } else { handleError(compileResult.error, pass, fn.node.loc ?? null); } - return null; + // If non-memoization features are enabled, retry regardless of error kind + if (!environment.enableFire) { + return null; + } + try { + compileResult = { + kind: 'compile', + compiledFn: compileFn( + fn, + environment, + fnType, + 'no_inferred_memo', + useMemoCacheIdentifier.name, + pass.opts.logger, + pass.filename, + pass.code, + ), + }; + } catch (err) { + // TODO: we might want to log error here, but this will also result in duplicate logging + return null; + } } pass.opts.logger?.logEvent(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 index 785240653e..4fce273b7a 100644 --- 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; export type MacroMethod = z.infer; @@ -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; -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, 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 index fe209924e4..a58ae44021 100644 --- 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, -): void { - functionEffects.forEach(eff => { +): Array { + 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 index bfa0825408..ae71da64b4 100644 --- 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,7 @@ const UndefinedValue: InstructionValue = { export default function inferReferenceEffects( fn: HIRFunction, options: {isFunctionExpression: boolean} = {isFunctionExpression: false}, -): void { +): Array { /* * Initial state contains function params * TODO: include module declarations here as well @@ -241,8 +241,9 @@ export default function inferReferenceEffects( if (options.isFunctionExpression) { fn.effects = functionEffects; - } else if (!fn.env.config.enableMinimalTransformsForRetry) { - raiseFunctionEffectErrors(functionEffects); + return []; + } else { + return transformFunctionEffectErrors(functionEffects); } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-capitalized-fn-call.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-capitalized-fn-call.expect.md index f7f413dedf..77bdd31275 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-capitalized-fn-call.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-capitalized-fn-call.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// @validateNoCapitalizedCalls @enableFire +// @validateNoCapitalizedCalls @enableFire @panicThreshold(none) import {fire} from 'react'; const CapitalizedCall = require('shared-runtime').sum; @@ -24,7 +24,7 @@ function Component({prop1, bar}) { ## Code ```javascript -import { useFire } from "react/compiler-runtime"; // @validateNoCapitalizedCalls @enableFire +import { useFire } from "react/compiler-runtime"; // @validateNoCapitalizedCalls @enableFire @panicThreshold(none) import { fire } from "react"; const CapitalizedCall = require("shared-runtime").sum; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-capitalized-fn-call.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-capitalized-fn-call.js index b872fd8670..679945eaab 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-capitalized-fn-call.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-capitalized-fn-call.js @@ -1,4 +1,4 @@ -// @validateNoCapitalizedCalls @enableFire +// @validateNoCapitalizedCalls @enableFire @panicThreshold(none) import {fire} from 'react'; const CapitalizedCall = require('shared-runtime').sum; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-eslint-suppressions.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-eslint-suppressions.expect.md index ad7f0ab467..30bd6d42e5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-eslint-suppressions.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-eslint-suppressions.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// @enableFire +// @enableFire @panicThreshold(none) import {useRef} from 'react'; function Component({props, bar}) { @@ -26,7 +26,7 @@ function Component({props, bar}) { ## Code ```javascript -import { useFire } from "react/compiler-runtime"; // @enableFire +import { useFire } from "react/compiler-runtime"; // @enableFire @panicThreshold(none) import { useRef } from "react"; function Component(t0) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-eslint-suppressions.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-eslint-suppressions.js index 7145fef03b..5312e5707c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-eslint-suppressions.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-eslint-suppressions.js @@ -1,4 +1,4 @@ -// @enableFire +// @enableFire @panicThreshold(none) import {useRef} from 'react'; function Component({props, bar}) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-preserve-memo.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-preserve-memo.expect.md index 9f8db6265f..6477a01126 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-preserve-memo.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-preserve-memo.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// @validatePreserveExistingMemoizationGuarantees @enableFire +// @validatePreserveExistingMemoizationGuarantees @enableFire @panicThreshold(none) import {fire} from 'react'; import {sum} from 'shared-runtime'; @@ -24,7 +24,7 @@ function Component({prop1, bar}) { ## Code ```javascript -import { useFire } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees @enableFire +import { useFire } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees @enableFire @panicThreshold(none) import { fire } from "react"; import { sum } from "shared-runtime"; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-preserve-memo.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-preserve-memo.js index b7c01e40ac..c3bb8b4216 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-preserve-memo.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-preserve-memo.js @@ -1,4 +1,4 @@ -// @validatePreserveExistingMemoizationGuarantees @enableFire +// @validatePreserveExistingMemoizationGuarantees @enableFire @panicThreshold(none) import {fire} from 'react'; import {sum} from 'shared-runtime'; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-prop-write.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-prop-write.expect.md index 74d9f679c4..e6ce051f10 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-prop-write.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-prop-write.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// @enableFire +// @enableFire @panicThreshold(none) import {fire} from 'react'; function Component({prop1}) { @@ -20,7 +20,7 @@ function Component({prop1}) { ## Code ```javascript -import { useFire } from "react/compiler-runtime"; // @enableFire +import { useFire } from "react/compiler-runtime"; // @enableFire @panicThreshold(none) import { fire } from "react"; function Component(t0) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-prop-write.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-prop-write.js index 4fac46f78e..b0cfd4fe39 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-prop-write.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-prop-write.js @@ -1,4 +1,4 @@ -// @enableFire +// @enableFire @panicThreshold(none) import {fire} from 'react'; function Component({prop1}) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-ref-current-access.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-ref-current-access.expect.md index af0fc7b5a2..79f5a2986d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-ref-current-access.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-ref-current-access.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// @flow @enableFire +// @flow @enableFire @panicThreshold(none) import {fire} from 'react'; import {print} from 'shared-runtime'; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-ref-current-access.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-ref-current-access.js index 96ccf0b161..f649fe5902 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-ref-current-access.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/bailout-validate-ref-current-access.js @@ -1,4 +1,4 @@ -// @flow @enableFire +// @flow @enableFire @panicThreshold(none) import {fire} from 'react'; import {print} from 'shared-runtime'; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-validate-conditional-hook.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-validate-conditional-hook.expect.md index c20f89d3af..dde2b692f4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-validate-conditional-hook.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-validate-conditional-hook.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// @enableFire +// @enableFire @panicThreshold(none) import {fire, useEffect} from 'react'; import {Stringify} from 'shared-runtime'; @@ -29,7 +29,7 @@ function Component(props) { ## Code ```javascript -import { useFire } from "react/compiler-runtime"; // @enableFire +import { useFire } from "react/compiler-runtime"; // @enableFire @panicThreshold(none) import { fire, useEffect } from "react"; import { Stringify } from "shared-runtime"; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-validate-conditional-hook.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-validate-conditional-hook.js index 0a29fd852c..fa8c034bfb 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-validate-conditional-hook.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-validate-conditional-hook.js @@ -1,4 +1,4 @@ -// @enableFire +// @enableFire @panicThreshold(none) import {fire, useEffect} from 'react'; import {Stringify} from 'shared-runtime'; From 63c1bc81f7a891e025cbbf21ceb8bc4cde489dd5 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Thu, 13 Mar 2025 19:55:27 -0400 Subject: [PATCH 2/2] [compiler] detect and throw on untransformed required features Traverse program after running compiler transform to find untransformed references to compiler features (e.g. `inferEffectDeps`, `fire`). Hard error to fail the babel pipeline when the compiler fails to transform these features to give predictable runtime semantics. Untransformed calls to functions like `fire` will throw at runtime anyways, so let's fail the build to catch these earlier. Note that with this fails the build *regardless of panicThreshold* --- .../src/Babel/BabelPlugin.ts | 8 +- .../src/Entrypoint/Program.ts | 23 +- .../ValidateNoUntransformedReferences.ts | 220 ++++++++++++++++++ .../src/HIR/Environment.ts | 1 + .../error.callsite-in-non-react-fn.expect.md | 26 +++ .../error.callsite-in-non-react-fn.js | 6 + .../error.non-inlined-effect-fn.expect.md | 41 ++++ .../error.non-inlined-effect-fn.js | 21 ++ ...mport-default-property-useEffect.expect.md | 27 +++ ...todo-import-default-property-useEffect.js} | 2 +- .../bailout-retry/error.todo-syntax.expect.md | 51 ++++ .../bailout-retry/error.todo-syntax.js | 18 ++ .../bailout-retry/error.use-no-memo.expect.md | 27 +++ .../bailout-retry/error.use-no-memo.js | 7 + .../infer-effect-dependencies.expect.md | 26 +-- .../infer-effect-dependencies.js | 7 - ...mport-default-property-useEffect.expect.md | 44 ---- .../error.todo-infer-deps-on-retry.expect.md | 42 ++++ .../error.todo-infer-deps-on-retry.js | 17 ++ .../bailout-retry/error.todo-syntax.expect.md | 20 +- .../bailout-retry/error.todo-syntax.js | 2 +- ...ror.untransformed-fire-reference.expect.md | 23 ++ .../error.untransformed-fire-reference.js | 4 + .../bailout-retry/error.use-no-memo.expect.md | 42 ++++ ...do-use-no-memo.js => error.use-no-memo.js} | 7 +- ...-fire-todo-syntax-shouldnt-throw.expect.md | 95 ++++++++ .../no-fire-todo-syntax-shouldnt-throw.js | 35 +++ .../bailout-retry/todo-use-no-memo.expect.md | 47 ---- 28 files changed, 743 insertions(+), 146 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/Entrypoint/ValidateNoUntransformedReferences.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.non-inlined-effect-fn.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.non-inlined-effect-fn.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-import-default-property-useEffect.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/{todo.import-default-property-useEffect.js => bailout-retry/error.todo-import-default-property-useEffect.js} (73%) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-syntax.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-syntax.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.use-no-memo.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.use-no-memo.js delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo.import-default-property-useEffect.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-infer-deps-on-retry.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-infer-deps-on-retry.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.untransformed-fire-reference.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.untransformed-fire-reference.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.use-no-memo.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/{todo-use-no-memo.js => error.use-no-memo.js} (51%) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/no-fire-todo-syntax-shouldnt-throw.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/no-fire-todo-syntax-shouldnt-throw.js delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/todo-use-no-memo.expect.md diff --git a/compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts b/compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts index aa49bda22b..431cbd8d41 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts @@ -11,6 +11,7 @@ import { injectReanimatedFlag, pipelineUsesReanimatedPlugin, } from '../Entrypoint/Reanimated'; +import validateNoUntransformedReferences from '../Entrypoint/ValidateNoUntransformedReferences'; const ENABLE_REACT_COMPILER_TIMINGS = process.env['ENABLE_REACT_COMPILER_TIMINGS'] === '1'; @@ -61,12 +62,17 @@ export default function BabelPluginReactCompiler( }, }; } - compileProgram(prog, { + const result = compileProgram(prog, { opts, filename: pass.filename ?? null, comments: pass.file.ast.comments ?? [], code: pass.file.code, }); + validateNoUntransformedReferences( + prog, + opts.environment, + result?.retryErrors ?? [], + ); if (ENABLE_REACT_COMPILER_TIMINGS === true) { performance.mark(`${filename}:end`, { detail: 'BabelPlugin:Program:end', 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 0865b50f84..72fa101260 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts @@ -271,6 +271,9 @@ function isFilePartOfSources( return false; } +type CompileProgramResult = { + retryErrors: Array<{fn: BabelFn; error: CompilerError}>; +}; /** * `compileProgram` is directly invoked by the react-compiler babel plugin, so * exceptions thrown by this function will fail the babel build. @@ -285,16 +288,16 @@ function isFilePartOfSources( export function compileProgram( program: NodePath, pass: CompilerPass, -): void { +): CompileProgramResult | null { if (shouldSkipCompilation(program, pass)) { - return; + return null; } const environment = pass.opts.environment; const restrictedImportsErr = validateRestrictedImports(program, environment); if (restrictedImportsErr) { handleError(restrictedImportsErr, pass, null); - return; + return null; } const useMemoCacheIdentifier = program.scope.generateUidIdentifier('c'); @@ -365,7 +368,7 @@ export function compileProgram( filename: pass.filename ?? null, }, ); - + const retryErrors: Array<{fn: BabelFn; error: CompilerError}> = []; const processFn = ( fn: BabelFn, fnType: ReactFunctionType, @@ -429,7 +432,9 @@ export function compileProgram( handleError(compileResult.error, pass, fn.node.loc ?? null); } // If non-memoization features are enabled, retry regardless of error kind - if (!environment.enableFire) { + if ( + !(environment.enableFire || environment.inferEffectDependencies != null) + ) { return null; } try { @@ -448,6 +453,9 @@ export function compileProgram( }; } 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; } } @@ -538,7 +546,7 @@ export function compileProgram( program.node.directives, ); if (moduleScopeOptOutDirectives.length > 0) { - return; + return null; } let gating: null | { gatingFn: ExternalFunction; @@ -596,7 +604,7 @@ export function compileProgram( } } catch (err) { handleError(err, pass, null); - return; + return null; } /* @@ -638,6 +646,7 @@ export function compileProgram( } addImportsToProgram(program, externalFunctions); } + return {retryErrors}; } function shouldSkipCompilation( diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/ValidateNoUntransformedReferences.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/ValidateNoUntransformedReferences.ts new file mode 100644 index 0000000000..194b6489ff --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/ValidateNoUntransformedReferences.ts @@ -0,0 +1,220 @@ +import {NodePath} from '@babel/core'; +import * as t from '@babel/types'; + +import {CompilerError, EnvironmentConfig} from '..'; +import {getOrInsertWith} from '../Utils/utils'; +import {Environment} from '../HIR'; + +export default function validateNoUntransformedReferences( + path: NodePath, + env: EnvironmentConfig, + transformErrors: Array<{fn: NodePath; error: CompilerError}>, +): void { + function assertValidEffectImportReference( + numArgs: number, + paths: Array>, + ): void { + for (const path of paths) { + const parent = path.parentPath; + if (parent != null && parent.isCallExpression()) { + const args = parent.get('arguments'); + /** + * Only error on untransformed references of the form `useMyEffect(...)` + * or `moduleNamespace.useMyEffect(...)`, with matching argument counts. + * TODO: do we also want a mode to also hard error on non-call references? + */ + if (args.length === numArgs) { + const maybeErrorDiagnostic = matchCompilerDiagnostic( + path, + transformErrors, + ); + /** + * Note that we cannot easily check the type of the first argument here, + * as it may have already been transformed by the compiler (and not + * memoized). + */ + CompilerError.throwInvalidReact({ + reason: + '[Fire] Untransformed reference to compiler-required feature. ' + + 'Either remove this call or ensure it is successfully transformed by the compiler', + description: maybeErrorDiagnostic + ? `(Bailout reason: ${maybeErrorDiagnostic})` + : null, + loc: parent.node.loc ?? null, + }); + } + } + } + } + + function assertValidFireImportReference( + paths: Array>, + ): void { + if (paths.length > 0) { + const maybeErrorDiagnostic = matchCompilerDiagnostic( + paths[0], + transformErrors, + ); + CompilerError.throwInvalidReact({ + reason: + '[Fire] Untransformed reference to compiler-required feature. ' + + 'Either remove this `fire` call or ensure it is successfully transformed by the compiler', + description: maybeErrorDiagnostic + ? `(Bailout reason: ${maybeErrorDiagnostic})` + : null, + loc: paths[0].node.loc ?? null, + }); + } + } + + const moduleLoadChecks = new Map< + string, + Map + >(); + if (env.enableFire) { + /** + * Error on any untransformed references to `fire` (e.g. including non-call + * expressions) + */ + for (const module of Environment.knownReactModules) { + const react = getOrInsertWith(moduleLoadChecks, module, () => new Map()); + react.set('fire', assertValidFireImportReference); + } + } + if (env.inferEffectDependencies) { + for (const { + function: {source, importSpecifierName}, + numRequiredArgs, + } of env.inferEffectDependencies) { + const module = getOrInsertWith(moduleLoadChecks, source, () => new Map()); + module.set( + importSpecifierName, + assertValidEffectImportReference.bind(null, numRequiredArgs), + ); + } + } + if (moduleLoadChecks.size > 0) { + transformProgram(path, moduleLoadChecks); + } +} + +type TraversalState = { + shouldInvalidateScopes: boolean; + program: NodePath; +}; +type CheckInvalidReferenceFn = (paths: Array>) => void; +function validateImportSpecifier( + specifier: NodePath, + importSpecifierChecks: Map, + state: TraversalState, +): void { + const imported = specifier.get('imported'); + const specifierName: string = + imported.node.type === 'Identifier' + ? imported.node.name + : imported.node.value; + const checkFn = importSpecifierChecks.get(specifierName); + if (checkFn == null) { + return; + } + if (state.shouldInvalidateScopes) { + state.shouldInvalidateScopes = false; + state.program.scope.crawl(); + } + + const local = specifier.get('local'); + const binding = local.scope.getBinding(local.node.name); + CompilerError.invariant(binding != null, { + reason: 'Expected binding to be found for import specifier', + loc: local.node.loc ?? null, + }); + checkFn(binding.referencePaths); +} + +function validateNamespacedImport( + specifier: NodePath, + importSpecifierChecks: Map, + state: TraversalState, +): void { + if (state.shouldInvalidateScopes) { + state.shouldInvalidateScopes = false; + state.program.scope.crawl(); + } + const local = specifier.get('local'); + const binding = local.scope.getBinding(local.node.name); + + CompilerError.invariant(binding != null, { + reason: 'Expected binding to be found for import specifier', + loc: local.node.loc ?? null, + }); + const filteredReferences = new Map< + CheckInvalidReferenceFn, + Array> + >(); + for (const reference of binding.referencePaths) { + const parent = reference.parentPath; + if ( + parent != null && + parent.isMemberExpression() && + parent.get('object') === reference + ) { + if (parent.node.computed || parent.node.property.type !== 'Identifier') { + continue; + } + const checkFn = importSpecifierChecks.get(parent.node.property.name); + if (checkFn != null) { + getOrInsertWith(filteredReferences, checkFn, () => []).push(parent); + } + } + } + + for (const [checkFn, references] of filteredReferences) { + checkFn(references); + } +} +function transformProgram( + path: NodePath, + moduleLoadChecks: Map>, +): void { + const traversalState = {shouldInvalidateScopes: true, program: path}; + path.traverse({ + ImportDeclaration(path: NodePath) { + const importSpecifierChecks = moduleLoadChecks.get( + path.node.source.value, + ); + if (importSpecifierChecks == null) { + return; + } + const specifiers = path.get('specifiers'); + for (const specifier of specifiers) { + if (specifier.isImportSpecifier()) { + validateImportSpecifier( + specifier, + importSpecifierChecks, + traversalState, + ); + } else { + validateNamespacedImport( + specifier as NodePath< + t.ImportNamespaceSpecifier | t.ImportDefaultSpecifier + >, + importSpecifierChecks, + traversalState, + ); + } + } + }, + }); +} + +function matchCompilerDiagnostic( + badReference: NodePath, + transformErrors: Array<{fn: NodePath; error: CompilerError}>, +): string | null { + for (const {fn, error} of transformErrors) { + if (fn.isAncestor(badReference)) { + return error.toString(); + } + } + return null; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index 4fce273b7a..1f64452e4b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -1121,6 +1121,7 @@ export class Environment { moduleName.toLowerCase() === 'react-dom' ); } + static knownReactModules: ReadonlyArray = ['react', 'react-dom']; getFallthroughPropertyType( receiver: Type, diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn.expect.md new file mode 100644 index 0000000000..2625cf9561 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn.expect.md @@ -0,0 +1,26 @@ + +## Input + +```javascript +// @inferEffectDependencies @compilationMode(infer) @panicThreshold(none) +import {useEffect} from 'react'; + +function nonReactFn(arg) { + useEffect(() => [1, 2, arg]); +} + +``` + + +## Error + +``` + 3 | + 4 | function nonReactFn(arg) { +> 5 | useEffect(() => [1, 2, arg]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: [Fire] Untransformed reference to compiler-required feature. Either remove this call or ensure it is successfully transformed by the compiler (5:5) + 6 | } + 7 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn.js new file mode 100644 index 0000000000..813c3b6c3d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.callsite-in-non-react-fn.js @@ -0,0 +1,6 @@ +// @inferEffectDependencies @compilationMode(infer) @panicThreshold(none) +import {useEffect} from 'react'; + +function nonReactFn(arg) { + useEffect(() => [1, 2, arg]); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.non-inlined-effect-fn.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.non-inlined-effect-fn.expect.md new file mode 100644 index 0000000000..0238c7428e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.non-inlined-effect-fn.expect.md @@ -0,0 +1,41 @@ + +## Input + +```javascript +// @inferEffectDependencies @panicThreshold(none) +import {useEffect} from 'react'; + +/** + * Error on non-inlined effect functions: + * 1. From the effect hook callee's perspective, it only makes sense + * to either + * (a) never hard error (i.e. failing to infer deps is acceptable) or + * (b) always hard error, + * regardless of whether the callback function is an inline fn. + * 2. (Technical detail) it's harder to support detecting cases in which + * function (pre-Forget transform) was inline but becomes memoized + */ +function Component({foo}) { + function f() { + console.log(foo); + } + + // No inferred dep array, the argument is not a lambda + useEffect(f); +} + +``` + + +## Error + +``` + 18 | + 19 | // No inferred dep array, the argument is not a lambda +> 20 | useEffect(f); + | ^^^^^^^^^^^^ InvalidReact: [Fire] Untransformed reference to compiler-required feature. Either remove this call or ensure it is successfully transformed by the compiler (20:20) + 21 | } + 22 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.non-inlined-effect-fn.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.non-inlined-effect-fn.js new file mode 100644 index 0000000000..a011e3bf14 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.non-inlined-effect-fn.js @@ -0,0 +1,21 @@ +// @inferEffectDependencies @panicThreshold(none) +import {useEffect} from 'react'; + +/** + * Error on non-inlined effect functions: + * 1. From the effect hook callee's perspective, it only makes sense + * to either + * (a) never hard error (i.e. failing to infer deps is acceptable) or + * (b) always hard error, + * regardless of whether the callback function is an inline fn. + * 2. (Technical detail) it's harder to support detecting cases in which + * function (pre-Forget transform) was inline but becomes memoized + */ +function Component({foo}) { + function f() { + console.log(foo); + } + + // No inferred dep array, the argument is not a lambda + useEffect(f); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-import-default-property-useEffect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-import-default-property-useEffect.expect.md new file mode 100644 index 0000000000..31cd5d6512 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-import-default-property-useEffect.expect.md @@ -0,0 +1,27 @@ + +## Input + +```javascript +// @inferEffectDependencies @panicThreshold(none) +import React from 'react'; + +function NonReactiveDepInEffect() { + const obj = makeObject_Primitives(); + React.useEffect(() => print(obj)); +} + +``` + + +## Error + +``` + 4 | function NonReactiveDepInEffect() { + 5 | const obj = makeObject_Primitives(); +> 6 | React.useEffect(() => print(obj)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: [Fire] Untransformed reference to compiler-required feature. Either remove this call or ensure it is successfully transformed by the compiler (6:6) + 7 | } + 8 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo.import-default-property-useEffect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-import-default-property-useEffect.js similarity index 73% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo.import-default-property-useEffect.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-import-default-property-useEffect.js index 0dbae754ec..237535fe5b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo.import-default-property-useEffect.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-import-default-property-useEffect.js @@ -1,4 +1,4 @@ -// @inferEffectDependencies +// @inferEffectDependencies @panicThreshold(none) import React from 'react'; function NonReactiveDepInEffect() { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-syntax.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-syntax.expect.md new file mode 100644 index 0000000000..8b349b2b81 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-syntax.expect.md @@ -0,0 +1,51 @@ + +## Input + +```javascript +// @inferEffectDependencies @panicThreshold(none) +import {useSpecialEffect} from 'shared-runtime'; + +/** + * Note that a react compiler-based transform still has limitations on JS syntax. + * We should surface these as actionable lint / build errors to devs. + */ +function Component({prop1}) { + 'use memo'; + useSpecialEffect(() => { + try { + console.log(prop1); + } finally { + console.log('exiting'); + } + }, [prop1]); + return
{prop1}
; +} + +``` + + +## Error + +``` + 8 | function Component({prop1}) { + 9 | 'use memo'; +> 10 | useSpecialEffect(() => { + | ^^^^^^^^^^^^^^^^^^^^^^^^ +> 11 | try { + | ^^^^^^^^^ +> 12 | console.log(prop1); + | ^^^^^^^^^ +> 13 | } finally { + | ^^^^^^^^^ +> 14 | console.log('exiting'); + | ^^^^^^^^^ +> 15 | } + | ^^^^^^^^^ +> 16 | }, [prop1]); + | ^^^^^^^^^^^^^^ InvalidReact: [Fire] Untransformed reference to compiler-required feature. Either remove this call or ensure it is successfully transformed by the compiler. (Bailout reason: Todo: (BuildHIR::lowerStatement) Handle TryStatement without a catch clause (11:15)) (10:16) + 17 | return
{prop1}
; + 18 | } + 19 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-syntax.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-syntax.js new file mode 100644 index 0000000000..d3c83c2562 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.todo-syntax.js @@ -0,0 +1,18 @@ +// @inferEffectDependencies @panicThreshold(none) +import {useSpecialEffect} from 'shared-runtime'; + +/** + * Note that a react compiler-based transform still has limitations on JS syntax. + * We should surface these as actionable lint / build errors to devs. + */ +function Component({prop1}) { + 'use memo'; + useSpecialEffect(() => { + try { + console.log(prop1); + } finally { + console.log('exiting'); + } + }, [prop1]); + return
{prop1}
; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.use-no-memo.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.use-no-memo.expect.md new file mode 100644 index 0000000000..2b6e0b2af7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.use-no-memo.expect.md @@ -0,0 +1,27 @@ + +## Input + +```javascript +// @inferEffectDependencies @panicThreshold(none) +import {useEffect} from 'react'; + +function Component({propVal}) { + 'use no memo'; + useEffect(() => [propVal]); +} + +``` + + +## Error + +``` + 4 | function Component({propVal}) { + 5 | 'use no memo'; +> 6 | useEffect(() => [propVal]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: [Fire] Untransformed reference to compiler-required feature. Either remove this call or ensure it is successfully transformed by the compiler (6:6) + 7 | } + 8 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.use-no-memo.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.use-no-memo.js new file mode 100644 index 0000000000..53eeda5050 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/error.use-no-memo.js @@ -0,0 +1,7 @@ +// @inferEffectDependencies @panicThreshold(none) +import {useEffect} from 'react'; + +function Component({propVal}) { + 'use no memo'; + useEffect(() => [propVal]); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/infer-effect-dependencies.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/infer-effect-dependencies.expect.md index 89da346a72..d0745f9bd3 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/infer-effect-dependencies.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/infer-effect-dependencies.expect.md @@ -34,13 +34,6 @@ function Component({foo, bar}) { console.log(bar.qux); }); - function f() { - console.log(foo); - } - - // No inferred dep array, the argument is not a lambda - useEffect(f); - useEffectWrapper(() => { console.log(foo); }); @@ -58,7 +51,7 @@ import useEffectWrapper from "useEffectWrapper"; const moduleNonReactive = 0; function Component(t0) { - const $ = _c(14); + const $ = _c(12); const { foo, bar } = t0; const ref = useRef(0); @@ -119,7 +112,7 @@ function Component(t0) { useEffect(t4, [bar.baz, bar.qux]); let t5; if ($[10] !== foo) { - t5 = function f() { + t5 = () => { console.log(foo); }; $[10] = foo; @@ -127,20 +120,7 @@ function Component(t0) { } else { t5 = $[11]; } - const f = t5; - - useEffect(f); - let t6; - if ($[12] !== foo) { - t6 = () => { - console.log(foo); - }; - $[12] = foo; - $[13] = t6; - } else { - t6 = $[13]; - } - useEffectWrapper(t6, [foo]); + useEffectWrapper(t5, [foo]); } ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/infer-effect-dependencies.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/infer-effect-dependencies.js index efdd416478..2bad5ee1cc 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/infer-effect-dependencies.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/infer-effect-dependencies.js @@ -30,13 +30,6 @@ function Component({foo, bar}) { console.log(bar.qux); }); - function f() { - console.log(foo); - } - - // No inferred dep array, the argument is not a lambda - useEffect(f); - useEffectWrapper(() => { console.log(foo); }); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo.import-default-property-useEffect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo.import-default-property-useEffect.expect.md deleted file mode 100644 index a5a576c830..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/todo.import-default-property-useEffect.expect.md +++ /dev/null @@ -1,44 +0,0 @@ - -## Input - -```javascript -// @inferEffectDependencies -import React from 'react'; - -function NonReactiveDepInEffect() { - const obj = makeObject_Primitives(); - React.useEffect(() => print(obj)); -} - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies -import React from "react"; - -function NonReactiveDepInEffect() { - const $ = _c(2); - let t0; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - t0 = makeObject_Primitives(); - $[0] = t0; - } else { - t0 = $[0]; - } - const obj = t0; - let t1; - if ($[1] === Symbol.for("react.memo_cache_sentinel")) { - t1 = () => print(obj); - $[1] = t1; - } else { - t1 = $[1]; - } - React.useEffect(t1); -} - -``` - -### Eval output -(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-infer-deps-on-retry.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-infer-deps-on-retry.expect.md new file mode 100644 index 0000000000..979311926f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-infer-deps-on-retry.expect.md @@ -0,0 +1,42 @@ + +## Input + +```javascript +// @inferEffectDependencies @panicThreshold(none) +import {useRef} from 'react'; +import {useSpecialEffect} from 'shared-runtime'; + +/** + * The retry pipeline disables memoization features, which means we need to + * provide an alternate implementation of effect dependencies which does not + * rely on memoization. + */ +function useFoo({cond}) { + const ref = useRef(); + const derived = cond ? ref.current : makeObject(); + useSpecialEffect(() => { + log(derived); + }, [derived]); + return ref; +} + +``` + + +## Error + +``` + 11 | const ref = useRef(); + 12 | const derived = cond ? ref.current : makeObject(); +> 13 | useSpecialEffect(() => { + | ^^^^^^^^^^^^^^^^^^^^^^^^ +> 14 | log(derived); + | ^^^^^^^^^^^^^^^^^ +> 15 | }, [derived]); + | ^^^^^^^^^^^^^^^^ InvalidReact: [Fire] Untransformed reference to compiler-required feature. Either remove this call or ensure it is successfully transformed by the compiler. (Bailout reason: Invariant: Expected function expression scope to exist (13:15)) (13:15) + 16 | return ref; + 17 | } + 18 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-infer-deps-on-retry.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-infer-deps-on-retry.js new file mode 100644 index 0000000000..f3a57bb912 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-infer-deps-on-retry.js @@ -0,0 +1,17 @@ +// @inferEffectDependencies @panicThreshold(none) +import {useRef} from 'react'; +import {useSpecialEffect} from 'shared-runtime'; + +/** + * The retry pipeline disables memoization features, which means we need to + * provide an alternate implementation of effect dependencies which does not + * rely on memoization. + */ +function useFoo({cond}) { + const ref = useRef(); + const derived = cond ? ref.current : makeObject(); + useSpecialEffect(() => { + log(derived); + }, [derived]); + return ref; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.expect.md index 86836d86b6..c5d7456d65 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// @enableFire +// @enableFire @panicThreshold(none) import {fire} from 'react'; /** @@ -29,21 +29,13 @@ function Component({prop1}) { ## Error ``` - 9 | function Component({prop1}) { - 10 | const foo = () => { -> 11 | try { - | ^^^^^ -> 12 | console.log(prop1); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ -> 13 | } finally { - | ^^^^^^^^^^^^^^^^^^^^^^^^^ -> 14 | console.log('jbrown215'); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ -> 15 | } - | ^^^^^^ Todo: (BuildHIR::lowerStatement) Handle TryStatement without a catch clause (11:15) 16 | }; 17 | useEffect(() => { - 18 | fire(foo()); +> 18 | fire(foo()); + | ^^^^ InvalidReact: [Fire] Untransformed reference to compiler-required feature. Either remove this `fire` call or ensure it is successfully transformed by the compiler. (Bailout reason: Todo: (BuildHIR::lowerStatement) Handle TryStatement without a catch clause (11:15)) (18:18) + 19 | }); + 20 | } + 21 | ``` \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.js index ec50ed84c5..b1ee459177 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-syntax.js @@ -1,4 +1,4 @@ -// @enableFire +// @enableFire @panicThreshold(none) import {fire} from 'react'; /** diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.untransformed-fire-reference.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.untransformed-fire-reference.expect.md new file mode 100644 index 0000000000..ddcc86ff00 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.untransformed-fire-reference.expect.md @@ -0,0 +1,23 @@ + +## Input + +```javascript +// @enableFire @panicThreshold(none) +import {fire} from 'react'; + +console.log(fire == null); + +``` + + +## Error + +``` + 2 | import {fire} from 'react'; + 3 | +> 4 | console.log(fire == null); + | ^^^^ InvalidReact: [Fire] Untransformed reference to compiler-required feature. Either remove this `fire` call or ensure it is successfully transformed by the compiler (4:4) + 5 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.untransformed-fire-reference.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.untransformed-fire-reference.js new file mode 100644 index 0000000000..25a60a9716 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.untransformed-fire-reference.js @@ -0,0 +1,4 @@ +// @enableFire @panicThreshold(none) +import {fire} from 'react'; + +console.log(fire == null); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.use-no-memo.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.use-no-memo.expect.md new file mode 100644 index 0000000000..84a27b43b8 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.use-no-memo.expect.md @@ -0,0 +1,42 @@ + +## Input + +```javascript +// @enableFire @panicThreshold(none) +import {fire} from 'react'; + +/** + * TODO: we should eventually distinguish between `use no memo` and `use no + * compiler` directives. The former should be used to *only* disable memoization + * features. + */ +function Component({props, bar}) { + 'use no memo'; + const foo = () => { + console.log(props); + }; + useEffect(() => { + fire(foo(props)); + fire(foo()); + fire(bar()); + }); + + return null; +} + +``` + + +## Error + +``` + 13 | }; + 14 | useEffect(() => { +> 15 | fire(foo(props)); + | ^^^^ InvalidReact: [Fire] Untransformed reference to compiler-required feature. Either remove this `fire` call or ensure it is successfully transformed by the compiler (15:15) + 16 | fire(foo()); + 17 | fire(bar()); + 18 | }); +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/todo-use-no-memo.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.use-no-memo.js similarity index 51% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/todo-use-no-memo.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.use-no-memo.js index 2587e24ee1..92e1ff211a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/todo-use-no-memo.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.use-no-memo.js @@ -1,6 +1,11 @@ -// @enableFire +// @enableFire @panicThreshold(none) import {fire} from 'react'; +/** + * TODO: we should eventually distinguish between `use no memo` and `use no + * compiler` directives. The former should be used to *only* disable memoization + * features. + */ function Component({props, bar}) { 'use no memo'; const foo = () => { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/no-fire-todo-syntax-shouldnt-throw.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/no-fire-todo-syntax-shouldnt-throw.expect.md new file mode 100644 index 0000000000..fecc28bb00 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/no-fire-todo-syntax-shouldnt-throw.expect.md @@ -0,0 +1,95 @@ + +## Input + +```javascript +// @enableFire @panicThreshold(none) +import {fire} from 'react'; + +/** + * Compilation of this file should succeed. + */ +function NonFireComponent({prop1}) { + /** + * This component bails out but does not use fire + */ + const foo = () => { + try { + console.log(prop1); + } finally { + console.log('jbrown215'); + } + }; + useEffect(() => { + foo(); + }); +} + +function FireComponent(props) { + /** + * This component uses fire and compiles successfully + */ + const foo = props => { + console.log(props); + }; + useEffect(() => { + fire(foo(props)); + }); + + return null; +} + +``` + +## Code + +```javascript +import { useFire } from "react/compiler-runtime"; +import { c as _c } from "react/compiler-runtime"; // @enableFire @panicThreshold(none) +import { fire } from "react"; + +/** + * Compilation of this file should succeed. + */ +function NonFireComponent({ prop1 }) { + /** + * This component bails out but does not use fire + */ + const foo = () => { + try { + console.log(prop1); + } finally { + console.log("jbrown215"); + } + }; + useEffect(() => { + foo(); + }); +} + +function FireComponent(props) { + const $ = _c(3); + + const foo = _temp; + const t0 = useFire(foo); + let t1; + if ($[0] !== props || $[1] !== t0) { + t1 = () => { + t0(props); + }; + $[0] = props; + $[1] = t0; + $[2] = t1; + } else { + t1 = $[2]; + } + useEffect(t1); + return null; +} +function _temp(props_0) { + console.log(props_0); +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/no-fire-todo-syntax-shouldnt-throw.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/no-fire-todo-syntax-shouldnt-throw.js new file mode 100644 index 0000000000..899fa33376 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/no-fire-todo-syntax-shouldnt-throw.js @@ -0,0 +1,35 @@ +// @enableFire @panicThreshold(none) +import {fire} from 'react'; + +/** + * Compilation of this file should succeed. + */ +function NonFireComponent({prop1}) { + /** + * This component bails out but does not use fire + */ + const foo = () => { + try { + console.log(prop1); + } finally { + console.log('jbrown215'); + } + }; + useEffect(() => { + foo(); + }); +} + +function FireComponent(props) { + /** + * This component uses fire and compiles successfully + */ + const foo = props => { + console.log(props); + }; + useEffect(() => { + fire(foo(props)); + }); + + return null; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/todo-use-no-memo.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/todo-use-no-memo.expect.md deleted file mode 100644 index 907501228b..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/todo-use-no-memo.expect.md +++ /dev/null @@ -1,47 +0,0 @@ - -## Input - -```javascript -// @enableFire -import {fire} from 'react'; - -function Component({props, bar}) { - 'use no memo'; - const foo = () => { - console.log(props); - }; - useEffect(() => { - fire(foo(props)); - fire(foo()); - fire(bar()); - }); - - return null; -} - -``` - -## Code - -```javascript -// @enableFire -import { fire } from "react"; - -function Component({ props, bar }) { - "use no memo"; - const foo = () => { - console.log(props); - }; - useEffect(() => { - fire(foo(props)); - fire(foo()); - fire(bar()); - }); - - return null; -} - -``` - -### Eval output -(kind: exception) Fixture not implemented \ No newline at end of file