From 34890556eca6751735344a8f41b911344668a67d Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Tue, 3 Oct 2023 13:58:05 -0700 Subject: [PATCH] [be] Improve feature flags setup Updates `Environment` to store all feature flags on a single `config` object. We now also define an object with all the default config values, and use this to populate defaults for any missing values in the user-provided config. --- .../src/Entrypoint/Options.ts | 4 +- .../src/Entrypoint/Pipeline.ts | 28 +++--- .../src/HIR/BuildHIR.ts | 2 +- .../src/HIR/Environment.ts | 87 +++++++++---------- .../src/Inference/InferReferenceEffects.ts | 4 +- .../ReactiveScopes/CodegenReactiveFunction.ts | 6 +- .../InferReactiveScopeVariables.ts | 2 +- .../ReactiveScopes/PruneNonEscapingScopes.ts | 6 +- 8 files changed, 67 insertions(+), 72 deletions(-) diff --git a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Options.ts b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Options.ts index 549fe9c9a3..fb9a62a1c9 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Options.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Options.ts @@ -7,7 +7,7 @@ import * as t from "@babel/types"; import { CompilerErrorDetailOptions } from "../CompilerError"; -import { EnvironmentConfig } from "../HIR/Environment"; +import { PartialEnvironmentConfig } from "../HIR/Environment"; export type ExternalFunction = { /** @@ -39,7 +39,7 @@ export type PanicThresholdOptions = | "NONE"; export type PluginOptions = { - environment: EnvironmentConfig | null; + environment: PartialEnvironmentConfig | null; logger: Logger | null; diff --git a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts index 547783769d..79a1ee5b98 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts @@ -17,7 +17,7 @@ import { lower, mergeConsecutiveBlocks, } from "../HIR"; -import { Environment, EnvironmentConfig } from "../HIR/Environment"; +import { Environment, PartialEnvironmentConfig } from "../HIR/Environment"; import { findContextIdentifiers } from "../HIR/FindContextIdentifiers"; import { analyseFunctions, @@ -82,7 +82,7 @@ export function* run( func: NodePath< t.FunctionDeclaration | t.ArrowFunctionExpression | t.FunctionExpression >, - config?: EnvironmentConfig | null + config?: PartialEnvironmentConfig | null ): Generator { const contextIdentifiers = findContextIdentifiers(func); const env = new Environment(config ?? null, contextIdentifiers); @@ -106,7 +106,7 @@ function* runWithEnvironment( pruneMaybeThrows(hir); yield log({ kind: "hir", name: "PruneMaybeThrows", value: hir }); - if (env.inlineUseMemo) { + if (env.config.inlineUseMemo) { inlineUseMemo(hir); yield log({ kind: "hir", name: "RewriteUseMemo", value: hir }); } @@ -131,7 +131,7 @@ function* runWithEnvironment( inferTypes(hir); yield log({ kind: "hir", name: "InferTypes", value: hir }); - if (env.validateHooksUsage) { + if (env.config.validateHooksUsage) { validateHooksUsage(hir); const conditionalHooksResult = validateUnconditionalHooks(hir).unwrap(); yield log({ @@ -150,7 +150,7 @@ function* runWithEnvironment( inferReferenceEffects(hir); yield log({ kind: "hir", name: "InferReferenceEffects", value: hir }); - if (env.validateFrozenLambdas) { + if (env.config.validateFrozenLambdas) { validateFrozenLambdas(hir); } @@ -164,15 +164,15 @@ function* runWithEnvironment( inferMutableRanges(hir); yield log({ kind: "hir", name: "InferMutableRanges", value: hir }); - if (env.assertValidMutableRanges) { + if (env.config.assertValidMutableRanges) { assertValidMutableRanges(hir); } - if (env.validateRefAccessDuringRender) { + if (env.config.validateRefAccessDuringRender) { validateNoRefAccessInRender(hir); } - if (env.validateNoSetStateInRender) { + if (env.config.validateNoSetStateInRender) { const noSetStateInRenderResult = validateNoSetStateInRender(hir).unwrap(); yield log({ kind: "debug", @@ -222,7 +222,7 @@ function* runWithEnvironment( value: reactiveFunction, }); - if (env.disableAllMemoization) { + if (env.config.disableAllMemoization) { pruneAllReactiveScopes(reactiveFunction); yield log({ kind: "reactive", @@ -259,8 +259,8 @@ function* runWithEnvironment( value: reactiveFunction, }); - let memoizeJsxElements = env.memoizeJsxElements; - if (env.enableForest) { + let memoizeJsxElements = env.config.memoizeJsxElements; + if (env.config.enableForest) { memoizeJsxElements = false; } pruneNonEscapingScopes(reactiveFunction, { memoizeJsxElements }); @@ -284,7 +284,7 @@ function* runWithEnvironment( value: reactiveFunction, }); - if (env.enableMergeConsecutiveScopes) { + if (env.config.enableMergeConsecutiveScopes) { mergeConsecutiveScopes(reactiveFunction); yield log({ kind: "reactive", @@ -293,7 +293,7 @@ function* runWithEnvironment( }); } - if (env.enableForest) { + if (env.config.enableForest) { yield* lowerToForest(reactiveFunction); } @@ -349,7 +349,7 @@ export function compileFn( func: NodePath< t.FunctionDeclaration | t.ArrowFunctionExpression | t.FunctionExpression >, - options?: Partial | null + options?: Partial | null ): CodegenFunction { let generator = run(func, options); while (true) { diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts index 44883a8b28..6fc5ecb3ad 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts @@ -1436,7 +1436,7 @@ function lowerExpression( elements.push({ kind: "Hole", }); - if (builder.environment.bailoutOnHoleyArrays) { + if (builder.environment.config.bailoutOnHoleyArrays) { builder.errors.push({ reason: `(BuildHIR::lower) Fix babel holey array backward compatibility.`, severity: ErrorSeverity.Todo, diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts index c0909c2b13..e4ec4f1537 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts @@ -84,8 +84,9 @@ export type Hook = { // i.e. // missing required shapes (BuiltInArray for [] and BuiltInObject for {}) // missing some recursive Object / Function shapeIds -export type EnvironmentConfig = Partial<{ - customHooks: Map; + +export type CompleteEnvironmentConfig = { + customHooks: Map | null; // 🌲 enableForest: boolean; @@ -243,42 +244,59 @@ export type EnvironmentConfig = Partial<{ * https://github.com/babel/babel/pull/10917/files#diff-19b555d2f3904c206af406540d9df200b1e16befedb83ff39ebfcbd876f7fa8aL52-R56 */ bailoutOnHoleyArrays: boolean; -}>; +}; + +const DEFAULT_ENVIRONMENT_CONFIG: CompleteEnvironmentConfig = { + customHooks: null, + + enableTreatHooksAsFunctions: true, + memoizeJsxElements: true, + + assertValidMutableRanges: false, + bailoutOnHoleyArrays: false, + disableAllMemoization: false, + enableAssumeHooksFollowRulesOfReact: false, + enableEmitFreeze: null, + enableForest: false, + enableFunctionCallSignatureOptimizations: false, + enableMergeConsecutiveScopes: false, + enableNoAliasOptimizations: false, + inlineUseMemo: false, + + validateFrozenLambdas: false, + validateHooksUsage: false, + validateNoSetStateInRender: false, + validateRefAccessDuringRender: false, +}; + +export type PartialEnvironmentConfig = Partial; export class Environment { #globals: GlobalRegistry; #shapes: ShapeRegistry; #nextIdentifer: number = 0; #nextBlock: number = 0; - validateHooksUsage: boolean; - validateRefAccessDuringRender: boolean; - validateFrozenLambdas: boolean; - validateNoSetStateInRender: boolean; - enableFunctionCallSignatureOptimizations: boolean; - enableAssumeHooksFollowRulesOfReact: boolean; - enableTreatHooksAsFunctions: boolean; - enableNoAliasOptimizations: boolean; - inlineUseMemo: boolean; - memoizeJsxElements: boolean; - disableAllMemoization: boolean; - enableEmitFreeze: ExternalFunction | null; - enableMergeConsecutiveScopes: boolean; - assertValidMutableRanges: boolean; - bailoutOnHoleyArrays: boolean; - enableForest: boolean; + config: CompleteEnvironmentConfig; #contextIdentifiers: Set; #hoistedIdentifiers: Set; constructor( - config: EnvironmentConfig | null, + partialConfig: PartialEnvironmentConfig | null, contextIdentifiers: Set ) { this.#shapes = new Map(DEFAULT_SHAPES); + const config: CompleteEnvironmentConfig = { ...DEFAULT_ENVIRONMENT_CONFIG }; + for (const rawKey in DEFAULT_ENVIRONMENT_CONFIG) { + const key = rawKey as keyof CompleteEnvironmentConfig; + const value = partialConfig?.[key] ?? DEFAULT_ENVIRONMENT_CONFIG[key]; + config[key] = value as any; + } + this.config = config; - if (config?.customHooks) { + if (this.config.customHooks != null && this.config.customHooks.size > 0) { this.#globals = new Map(DEFAULT_GLOBALS); - for (const [hookName, hook] of config.customHooks) { + for (const [hookName, hook] of this.config.customHooks) { CompilerError.invariant(!this.#globals.has(hookName), { reason: `[Globals] Found existing definition in global registry for custom hook ${hookName}`, description: null, @@ -303,29 +321,6 @@ export class Environment { } else { this.#globals = DEFAULT_GLOBALS; } - this.validateHooksUsage = config?.validateHooksUsage ?? false; - this.validateRefAccessDuringRender = - config?.validateRefAccessDuringRender ?? false; - this.validateFrozenLambdas = config?.validateFrozenLambdas ?? false; - this.enableFunctionCallSignatureOptimizations = - config?.enableFunctionCallSignatureOptimizations ?? false; - this.enableNoAliasOptimizations = - config?.enableNoAliasOptimizations ?? false; - this.enableAssumeHooksFollowRulesOfReact = - config?.enableAssumeHooksFollowRulesOfReact ?? false; - this.enableTreatHooksAsFunctions = - config?.enableTreatHooksAsFunctions ?? true; - this.disableAllMemoization = config?.disableAllMemoization ?? false; - this.enableEmitFreeze = config?.enableEmitFreeze ?? null; - this.enableMergeConsecutiveScopes = - config?.enableMergeConsecutiveScopes ?? false; - this.assertValidMutableRanges = config?.assertValidMutableRanges ?? false; - this.validateNoSetStateInRender = - config?.validateNoSetStateInRender ?? false; - this.inlineUseMemo = config?.inlineUseMemo ?? false; - this.memoizeJsxElements = config?.memoizeJsxElements ?? true; - this.bailoutOnHoleyArrays = config?.bailoutOnHoleyArrays ?? false; - this.enableForest = config?.enableForest ?? false; this.#contextIdentifiers = contextIdentifiers; this.#hoistedIdentifiers = new Set(); @@ -412,7 +407,7 @@ export class Environment { } #getCustomHookType(): Global { - if (this.enableAssumeHooksFollowRulesOfReact) { + if (this.config.enableAssumeHooksFollowRulesOfReact) { return DefaultNonmutatingHook; } else { return DefaultMutatingHook; diff --git a/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts b/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts index 0a3953093d..dd9d25a7ea 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts @@ -747,7 +747,7 @@ function inferBlock( instrValue.callee.identifier.type ); signature = - env.enableFunctionCallSignatureOptimizations || + env.config.enableFunctionCallSignatureOptimizations || signature?.hookKind != null ? signature : null; @@ -755,7 +755,7 @@ function inferBlock( if ( signature && signature.hookKind != null && - !env.enableTreatHooksAsFunctions + !env.config.enableTreatHooksAsFunctions ) { effectKind = signature.restParam; valueKind = signature.returnValueKind; diff --git a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts index 8e9eb97993..b2d5262032 100644 --- a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -184,12 +184,12 @@ function codegenBlock(cx: Context, block: ReactiveBlock): t.BlockStatement { } function wrapCacheDep(cx: Context, value: t.Expression): t.Expression { - if (cx.env.enableEmitFreeze != null) { + if (cx.env.config.enableEmitFreeze != null) { // The import declaration for emitFreeze is inserted in the Babel plugin return t.conditionalExpression( t.identifier("__DEV__"), t.callExpression( - t.identifier(cx.env.enableEmitFreeze.importSpecifierName), + t.identifier(cx.env.config.enableEmitFreeze.importSpecifierName), [value, t.stringLiteral(cx.fnName)] ), value @@ -383,7 +383,7 @@ function codegenReactiveScope( scope: ReactiveScope, block: ReactiveBlock ): void { - if (cx.env.enableForest) { + if (cx.env.config.enableForest) { codegenSignalBlockForReactiveScope(cx, statements, scope, block); } else { codegenMemoBlockForReactiveScope(cx, statements, scope, block); diff --git a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/InferReactiveScopeVariables.ts b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/InferReactiveScopeVariables.ts index 317d4d151a..62c87c4f65 100644 --- a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/InferReactiveScopeVariables.ts +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/InferReactiveScopeVariables.ts @@ -250,7 +250,7 @@ function mayAllocate(env: Environment, instruction: Instruction): boolean { } case "CallExpression": case "MethodCall": { - if (env.enableFunctionCallSignatureOptimizations) { + if (env.config.enableFunctionCallSignatureOptimizations) { return instruction.lvalue.identifier.type.kind !== "Primitive"; } return true; diff --git a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PruneNonEscapingScopes.ts b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PruneNonEscapingScopes.ts index 3242489ad6..5dfd64bde2 100644 --- a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PruneNonEscapingScopes.ts +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PruneNonEscapingScopes.ts @@ -611,7 +611,7 @@ function computeMemoizationInputs( }; } case "CallExpression": { - const signature = env.enableNoAliasOptimizations + const signature = env.config.enableNoAliasOptimizations ? getFunctionCallSignature(env, value.callee.identifier.type) : null; const operands = [...eachReactiveValueOperand(value)]; @@ -636,7 +636,7 @@ function computeMemoizationInputs( }; } case "MethodCall": { - const signature = env.enableNoAliasOptimizations + const signature = env.config.enableNoAliasOptimizations ? getFunctionCallSignature(env, value.property.identifier.type) : null; const operands = [...eachReactiveValueOperand(value)]; @@ -808,7 +808,7 @@ class CollectDependenciesVisitor extends ReactiveFunctionVisitor { } else if (instruction.value.kind === "CallExpression") { const callee = instruction.value.callee; if (getHookKind(state.env, callee.identifier) != null) { - const signature = this.env.enableNoAliasOptimizations + const signature = this.env.config.enableNoAliasOptimizations ? getFunctionCallSignature( this.env, instruction.value.callee.identifier.type