fix(hir): perry.define of process.env.* folds at lowering, not just tree-shake (#5009)#5014
Merged
Merged
Conversation
…ree-shake (#5009) perry.define of process.env.NODE_ENV was ignored in every default build: the define was only consulted by the env_fold branch pruner, which is gated on tree_shake — and tree-shaking is off by default. So process.env.NODE_ENV stayed a live runtime EnvGet lookup (undefined when unset), and React / react-reconciler / scheduler selected their development builds (crashing ink #348 with 'value is not a function'). Fold a defined process.env.<NAME> read to its literal at the single HIR lowering point that would otherwise emit Expr::EnvGet — independent of tree-shaking and in every context (branch conditions, ternaries, closures). esbuild-style define semantics: the define wins over the runtime environment. - perry-hir/ir/constants.rs: EnvDefine enum + ENV_DEFINES thread-local (mirrors COMPILE_PACKAGES_OVERRIDE; rayon-safe) with set/clear/lookup. - perry-hir/lower/expr_member.rs: env_define_literal() substitutes the literal in both the process.env.X and process.env["X"] arms before EnvGet. - perry/compile/collect_modules.rs: env_defines_for_lowering() strips the process.env. prefix; set before each lower_module_full, cleared after. Composes with env_fold: substitution yields a string literal that try_const already folds, so branch-elimination still works under tree-shake; the tree-shake-only node_modules NODE_ENV->production default is unchanged. Tests: issue_5009_define_process_env.rs (react-reconciler-shaped fixture: production selected, define wins over runtime env; no-define still reads runtime env) + unit test for env_defines_for_lowering mapping.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #5009 —
perry.defineofprocess.env.NODE_ENV(and anyprocess.env.*) was silently ignored in every default build. As a result, compiled packages that branch onprocess.env.NODE_ENV(React, react-reconciler, scheduler, …) selected their development builds, and react-reconciler's dev build then threwTypeError: value is not a functionduring render — the last wall before yoga for ink (#348).Root cause (not #4993 as the issue guessed)
#4993 only touched runtime files;
process.env.Xstill lowered toExpr::EnvGet. The real bug is broader: thedefinewas only consulted by theenv_foldbranch pruner, which is gated ontree_shake— and tree-shaking is off by default (PERRY_TREE_SHAKE=1/perry.experiments.treeShake: trueonly). So in a normal build the define did nothing andprocess.env.NODE_ENVstayed a live runtimeEnvGet → js_getenv_valuelookup (undefinedwhen unset).Fix
Fold a defined
process.env.<NAME>read to its literal at the single HIR lowering point that would otherwise emitExpr::EnvGet— independent of tree-shaking and in every context (branch conditions, ternaries, closures). esbuild-styledefinesemantics: the define wins over the runtime environment.perry-hir/ir/constants.rs—EnvDefineenum +ENV_DEFINESthread-local (mirrors the existingCOMPILE_PACKAGES_OVERRIDEpattern; rayon-safe per worker) withset_env_defines/clear_env_defines/env_define_lookup.perry-hir/lower/expr_member.rs—env_define_literal()substitutes the literal in both theprocess.env.Xandprocess.env["X"]arms before emittingEnvGet.perry/compile/collect_modules.rs—env_defines_for_lowering()strips theprocess.env.prefix fromctx.define; installed before eachlower_module_full, cleared after.Composes cleanly with
env_fold: the substitution yields a string literal thattry_constalready folds, so branch-elimination still works under tree-shake; the tree-shake-only node_modulesNODE_ENV → "production"default is unchanged. Keys without an explicit define still produce a runtimeEnvGet.Verification
compilePackagesfixture; after the fix the production build is selected and the define wins over a conflicting runtimeNODE_ENV=development.test_gap_process_import_4987.tsis still byte-identical tonode --experimental-strip-types.crates/perry/tests/issue_5009_define_process_env.rs(2 integration) + a unit test forenv_defines_for_lowering. All pass.The secondary item in the issue (react-reconciler's development build hits a
value is not a functioncodegen gap) is out of scope here — production is the target build and now selected.🤖 Generated with Claude Code