Skip to content

refactor: add EnvConfig for centralized env var management#556

Merged
fengmk2 merged 8 commits intomainfrom
refactor/env-config
Feb 9, 2026
Merged

refactor: add EnvConfig for centralized env var management#556
fengmk2 merged 8 commits intomainfrom
refactor/env-config

Conversation

@fengmk2
Copy link
Copy Markdown
Member

@fengmk2 fengmk2 commented Feb 8, 2026

Summary

Phase 1+2 of env var refactoring. Introduces EnvConfig struct that reads all known environment variables once at startup, enabling testable, parallel-safe code.

Problem

  • 235 direct std::env calls across 7 crates, 140 are set_var/remove_var
  • env::set_var is unsafe since Rust 1.83
  • Tests need #[serial] or #[ignore] due to global env mutation
  • No single source of truth for supported env vars

Changes

New: vite_shared::EnvConfig

  • Central struct documenting all 11 known env vars
  • EnvConfig::from_env() — read from real environment (production)
  • EnvConfig::for_test() — sensible defaults, zero env access
  • EnvConfig::for_test_with_home(path) — convenience for temp dir tests
  • Supports struct update syntax: EnvConfig { is_ci: true, ..EnvConfig::for_test() }

Updated: home.rs

  • New get_vite_plus_home_with_config(&EnvConfig) — config-based, no env access
  • Legacy get_vite_plus_home() preserved for backward compat
  • Removed #[ignore] test, replaced with parallel-safe config test

Updated: tracing.rs

  • New init_tracing_with_config(&EnvConfig) — config-based
  • Legacy init_tracing() preserved
  • Extracted shared init_tracing_inner()

Test Results

  • cargo test -p vite_shared: 19 passed + 2 doc-tests ✅
  • cargo check (full project): compiles ✅

Next Phases

  • Phase 3: Thread through vite_install + vite_js_runtime
  • Phase 4: Thread through vite_global_cli (biggest change)
  • Phase 5: Remove remaining unsafe env::set_var

@fengmk2 fengmk2 force-pushed the refactor/env-config branch 2 times, most recently from 4232806 to 9c1c7e8 Compare February 8, 2026 09:54
@fengmk2 fengmk2 added the test: e2e Auto run e2e tests label Feb 8, 2026
@fengmk2 fengmk2 force-pushed the refactor/env-config branch 3 times, most recently from 7ad3717 to b948a4a Compare February 9, 2026 14:12
@fengmk2 fengmk2 marked this pull request as ready for review February 9, 2026 14:16
Introduce EnvConfig for centralized environment variable management:

- EnvConfig::init() in main(), EnvConfig::get() anywhere (OnceLock)
- EnvConfig::test_scope() for thread-local test overrides (parallel-safe)
- No function signature changes needed

Migrated crates:
- vite_shared: home.rs, tracing.rs use EnvConfig::get()
- vite_install: config.rs removes LazyLock<NPM_REGISTRY>, uses EnvConfig
- vite_js_runtime: download.rs (CI check), node.rs (dist mirror)
- vite_global_cli: config.rs (version resolution), use.rs (eval enable),
  registry.rs (npm_registry)

Remaining: vite_global_cli tests (22 #[serial] tests in config.rs need
EnvConfig::test_scope migration), shim/dispatch.rs env mutations
…g::test_guard

Remove #[serial] and unsafe env::set_var/remove_var from:
- config.rs: all 22 serial tests converted
- package_metadata.rs: 3 serial tests converted

Remaining serial tests in other files (~40) follow the same pattern
and can be converted in follow-up commits.
…stall tests to EnvConfig

- Add user_home, fish_version, ps_module_path fields to EnvConfig
- Export TestEnvGuard from vite_shared
- Convert setup.rs: 11 tests, no more unsafe HOME mutation
- Convert use.rs: detect_shell() reads from EnvConfig, 3 tests converted
- Convert bin_config.rs: 4 tests using test_guard(for_test_with_home)
- Convert global_install.rs: 1 test using test_guard
- Convert package_metadata.rs: 2 remaining tests

Removed 218 lines of unsafe env var manipulation. All #[serial] removed
from converted tests. Remaining #[serial] tests are in dispatch.rs (deferred),
doctor.rs (own EnvGuard pattern), exec.rs, and js_executor.rs.
…cing

VITE_LOG is only read once during tracing initialization via OnceLock,
so there's no benefit from caching it in EnvConfig. Restored the original
direct std::env::var("VITE_LOG") read in init_tracing().
Add vite_shared::env_vars module with constants for all vite-plus
environment variable names. Replace string literals across all crates:

- env_config.rs: 10 var names → env_vars constants
- tracing.rs: VITE_LOG → env_vars::VITE_LOG
- dispatch.rs: VITE_PLUS_BYPASS (13x), RECURSION_ENV_VAR, DEBUG_SHIM,
  ACTIVE_NODE, RESOLVE_SOURCE → env_vars constants
- doctor.rs: VITE_PLUS_BYPASS (5x), VITE_PLUS_HOME, NODE_VERSION
- exec.rs: VITE_PLUS_TOOL_RECURSION (2x)
- config.rs: VERSION_ENV_VAR → env_vars::VITE_PLUS_NODE_VERSION
- mod.rs: SHIM_TOOL_ENV_VAR → env_vars::VITE_PLUS_SHIM_TOOL
- js_executor.rs: JS_SCRIPTS_DIR, CLI_BIN

Standard system vars (PATH, HOME, CI, etc.) intentionally excluded.
@fengmk2 fengmk2 force-pushed the refactor/env-config branch from b948a4a to 4ef8184 Compare February 9, 2026 14:29
@fengmk2 fengmk2 merged commit 20c168a into main Feb 9, 2026
30 checks passed
@fengmk2 fengmk2 deleted the refactor/env-config branch February 9, 2026 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test: e2e Auto run e2e tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants