feat(config): add NestJS-inspired configuration system(rikta config similarity)#26
Merged
Merged
Conversation
…lidated environment loading This commit introduces a comprehensive configuration system for NestForge applications, following the philosophy of NestJS. ## Core Features Implemented ### nestforge-config crate - Added for environment variable loading from .env files - Implemented with support for custom paths, process env inclusion, and prefix filtering - Added and for modular configuration - Created for detailed error reporting at startup - Implemented trait for automatic type parsing from environment variables - Supported primitives: String, bool, u8-u64, i8-i64, f32-f64, usize, isize ### nestforge-macros crate - Developed procedural macro for automatic config struct generation - Macro automatically implements and traits - Generates type-safe field access with automatic env var key mapping ### nestforge-core crate - Added function for DI container integration - Implemented trait for automatic config loading and validation - Exported config types for easy access via ### nestforge-cli crate - Updated app scaffolding to generate with example config struct - Added proper env var support in .env templates ## Technical Details - Uses for .env file parsing - Integrates crate for schema validation - Leverages crate for static registration (prepared for future use) - Zero runtime overhead after validation - configs are singletons in DI container ## Breaking Changes None - this is a new optional module that does not affect existing functionality. ## Performance Impact - Startup: Slight increase due to .env parsing and validation (runs once) - Runtime: Zero overhead after injection - Binary size: Minimal increase due to serde and validator dependencies
…n and auto-deriving, fixes #23 Removes the validator-based validation system, prefix support, ConfigField trait, and automatic derive implementation from nestforge-config. The config system now requires manual FromEnv implementation instead of using derive macros. This simplifies the API and removes external validation dependencies. BREAKING CHANGE: The #[derive(Config)] macro no longer auto-generates FromEnv implementations. Users must manually implement FromEnv for their config structs. The `prefix` option in ConfigOptions has been removed. ConfigField trait and its helper methods (from_env, from_env_or, etc.) have been removed.
…ed approach Replaced the trait-based configuration pattern (FromEnv, Configurable) with a simpler service-based approach (ConfigService). Removed validation dependencies (validator, serde, inventory) and streamlined error handling. The new ConfigService uses Arc for thread-safe sharing and provides a cleaner API with get, get_or_panic, get_or, and get_as methods. BREAKING CHANGE: Removed FromEnv trait - configs must now be loaded via ConfigService instead of implementing FromEnv BREAKING CHANGE: EnvStore replaced with ConfigService - update all references from EnvStore to ConfigService BREAKING CHANGE: Configurable trait removed - use ConfigService::load_with_options() directly instead of register_config<T: Configurable>
…d getters Refactored ConfigService to use HashMap directly instead of Arc-wrapped HashMap, changed get() to return Option<&str> for zero-copy access, and added typed getters (get_i32, get_u16, get_bool) for more ergonomic configuration access. Removed generic get_as methods in favor of specific typed methods.
…config loading Add new typed getter methods (get_u32, get_usize, get_f64, get_isize, get_i64, get_u64) and has() method to ConfigService. Introduce Config<T> and ConfigRegistration<T> for type-safe config registration. Refactor config loading to use ConfigModule::for_root_with_options instead of ConfigService::load_with_options.
- Add EnvSchema for defining required environment variables - Add EnvStore for storing and retrieving env values - Add FromEnv trait for typed config conversion - Add Config<T> wrapper for DI integration - Add load_config() helper function - Update all examples to use new simplified API - Update README and documentation with new API reference
vernonthedev
added
enhancement
github-actions
Bot
added
documentation
Code Review by Qodo
1. Config wrapper unusable
|
4 tasks
📝 WalkthroughWalkthroughRefactored NestForge configuration system from schema-based validation to a Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant CM as ConfigModule
participant CS as ConfigService
participant Env as Process Env +<br/>Dotenvy File
participant DI as DI Container
App->>CM: for_root_with_options(options)
CM->>CS: load_with_options()
CS->>Env: env::vars() + dotenvy::from_path_iter()
Env-->>CS: key-value pairs
CS-->>CM: ConfigService instance
CM-->>App: ConfigService (ready)
App->>DI: register_config(container, options)
DI->>CS: load()
CS->>Env: env::vars() + dotenvy::from_path_iter()
Env-->>CS: key-value pairs
CS-->>DI: ConfigService singleton
DI-->>App: registration complete
App->>CS: get_string("APP_NAME")
CS-->>App: typed value + default
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
vernonthedev
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56d0f39ddd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+103
to
+105
| .for_each(|result| { | ||
| if let Ok((key, value)) = result { | ||
| values.insert(key, value); |
There was a problem hiding this comment.
.env values
When include_process_env is true, values is preloaded from env::vars(), but this loop unconditionally overwrites those entries with .env contents. The previous loader preserved runtime overrides via entry(...).or_insert_with(...), so deployments that inject PORT, DATABASE_URL, etc. through the process environment and also ship a default .env will now silently boot with the file's values instead of the intended runtime configuration.
Useful? React with 👍 / 👎.
| fn load_app_config() -> anyhow::Result<AppConfig> {{ | ||
| Ok(ConfigModule::for_root::<AppConfig>(ConfigOptions::new().env_file(".env"))?) | ||
| }} | ||
| pub type AppConfig = ConfigService; |
There was a problem hiding this comment.
AppConfig injectable
This changes the generated AppConfig provider into a plain alias of ConfigService, but template_app_module_rs still emits providers = [AppConfig] (crates/nestforge-cli/src/main.rs:2571-2600), and the module macro expands bare type names to register_injectable::<T> (crates/nestforge-macros/src/lib.rs:1749-1755). Any project created with nestforge new will therefore fail to compile because ConfigService does not implement Injectable.
Useful? React with 👍 / 👎.
Comment on lines
+275
to
+277
| pub struct Config<T> { | ||
| _phantom: std::marker::PhantomData<T>, | ||
| } |
There was a problem hiding this comment.
Config<T> instead of a marker
Config<T> is only a PhantomData marker here, and neither load_config() nor nestforge_core::register_config() ever registers that wrapper type—only ConfigService. Because the container resolves exact TypeIds (crates/nestforge-core/src/container.rs:259-297), any provider that follows the advertised Config<AppConfig> pattern and resolves Config<AppConfig> will fail with TypeNotRegistered at startup.
Useful? React with 👍 / 👎.
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+275
to
291
| pub struct Config<T> { | ||
| _phantom: std::marker::PhantomData<T>, | ||
| } | ||
|
|
||
| if issues.is_empty() { | ||
| Ok(()) | ||
| } else { | ||
| Err(ConfigError::Validation { issues }) | ||
| impl<T> Config<T> { | ||
| pub fn new() -> Self { | ||
| Self { | ||
| _phantom: std::marker::PhantomData, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * EnvStore | ||
| * | ||
| * A store for environment variable key-value pairs. | ||
| * | ||
| * # Loading | ||
| * - Loads from .env file if present | ||
| * - Includes process environment variables by default | ||
| * - Supports custom loading options | ||
| */ | ||
| #[derive(Clone, Debug, Default)] | ||
| pub struct EnvStore { | ||
| values: HashMap<String, String>, | ||
| impl<T> Default for Config<T> { | ||
| fn default() -> Self { | ||
| Self::new() | ||
| } | ||
| } |
There was a problem hiding this comment.
1. Config wrapper unusable 🐞 Bug ✓ Correctness
Config<T> is documented as the injectable config handle, but it is only a PhantomData wrapper and exposes none of ConfigService’s getter APIs, so documented usage like Config<AppConfig>.get_string_or(...) cannot compile.
Agent Prompt
### Issue description
`Config<T>` is advertised/used as the DI-friendly config handle, but it has no functionality. Any code following the README/docs (`self.config.get_string_or(...)`) will not compile.
### Issue Context
- `ConfigService` contains the getter methods.
- `Config<T>` currently contains only `PhantomData<T>`.
### Fix Focus Areas
- Implement `Config<T>` as a real wrapper around `ConfigService` (e.g., `struct Config<T>(Arc<ConfigService>, PhantomData<T>)` or `struct Config<T>(ConfigService, PhantomData<T>)`) and forward/Deref getter methods.
- Alternatively, remove `Config<T>` from public API and update docs/README to inject/resolve `ConfigService` (or `AppConfig = ConfigService`) directly.
### Fix Focus Areas (locations)
- crates/nestforge-config/src/lib.rs[75-223]
- crates/nestforge-config/src/lib.rs[275-291]
- docs/config-module.md[54-75]
- crates/nestforge-config/README.md[25-41]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+2027
to
+2052
| #[proc_macro_derive(Config)] | ||
| pub fn derive_config(item: TokenStream) -> TokenStream { | ||
| let input = parse_macro_input!(item as DeriveInput); | ||
| let name = &input.ident; | ||
| let (impl_generics, ty_generics, where_clause) = &input.generics.split_for_impl(); | ||
|
|
||
| let Data::Struct(_data) = &input.data else { | ||
| return syn::Error::new(input.ident.span(), "Config can only be derived on structs") | ||
| .to_compile_error() | ||
| .into(); | ||
| }; | ||
|
|
||
| let expanded = quote! { | ||
| impl #impl_generics nestforge_config::FromEnv for #name #ty_generics #where_clause { | ||
| fn from_env(env: &nestforge_config::EnvStore) -> Result<Self, nestforge_config::ConfigError> { | ||
| std::compile_error!( | ||
| "Config derive requires manual FromEnv implementation. \ | ||
| Use `impl FromEnv for YourConfig` with `env.get(\"KEY\")` to read values." | ||
| ); | ||
| } | ||
|
|
||
| fn config_key() -> &'static str { | ||
| stringify!(#name) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
2. Broken derive config 🐞 Bug ✓ Correctness
#[derive(Config)] generates an impl of nestforge_config::FromEnv that declares a non-existent trait method (config_key) and intentionally emits compile_error! inside function bodies, so any use of the derive fails compilation.
Agent Prompt
### Issue description
`#[derive(Config)]` currently expands to code that cannot compile against the current `FromEnv` trait (extra `config_key()` method) and contains deliberate `compile_error!`.
### Issue Context
If this derive is intended as a placeholder, it should not be shipped in a way that breaks builds when used. If it is intended to work, it must match the `FromEnv` trait and generate a valid `from_env` implementation.
### Fix Focus Areas
- Either delete the derive macro entirely until implemented.
- Or update the macro to:
- Only implement existing trait methods (`from_env`).
- Remove `compile_error!` bodies.
- (Optional) generate `from_env` by reading fields via attributes.
### Fix Focus Areas (locations)
- crates/nestforge-macros/src/lib.rs[2027-2063]
- crates/nestforge-config/src/lib.rs[71-73]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+2727
to
+2734
| fn template_app_config_rs(_transport: AppTransport) -> String { | ||
| r#"use nestforge_config::{ConfigService, ConfigModule}; | ||
|
|
||
| fn load_app_config() -> anyhow::Result<AppConfig> {{ | ||
| Ok(ConfigModule::for_root::<AppConfig>(ConfigOptions::new().env_file(".env"))?) | ||
| }} | ||
| pub type AppConfig = ConfigService; | ||
|
|
||
| impl nestforge::FromEnv for AppConfig {{ | ||
| fn from_env(env: &nestforge::EnvStore) -> Result<Self, nestforge::ConfigError> {{ | ||
| let app_name = env.get("APP_NAME").unwrap_or("{default_app_name}").to_string(); | ||
| Ok(Self {{ app_name }}) | ||
| }} | ||
| }} | ||
| pub fn load_config() -> AppConfig { | ||
| ConfigModule::for_root_with_options(ConfigModule::for_root().env_file(".env")) | ||
| } |
There was a problem hiding this comment.
3. Scaffolded app won't compile 🐞 Bug ✓ Correctness
The CLI now generates app_config.rs that imports nestforge_config directly and defines AppConfig = ConfigService, but the generated Cargo.toml doesn’t add a nestforge-config dependency and other templates still register/consume AppConfig as if it were a struct (register_injectable::<AppConfig>, config.app_name), causing immediate compilation failures in new projects.
Agent Prompt
### Issue description
`nestforge new` scaffolding is internally inconsistent after switching `AppConfig` to a `ConfigService` alias. The generated project will not compile.
### Issue Context
- `#[module]` providers accept expressions; a bare path like `AppConfig` is treated as an injectable type and becomes `register_injectable::<AppConfig>`.
- With `pub type AppConfig = ConfigService;`, this is not injectable.
### Fix Focus Areas
- In `template_app_config_rs`, import from `nestforge` (re-export) instead of `nestforge_config`, OR add an explicit `nestforge-config` dependency in generated Cargo.toml.
- In `template_app_module_rs`, register config as a value/factory expression, e.g. `providers = [load_config()]` or `providers = [Provider::value(load_config())]`.
- Update `template_main_rs` (GraphQL) to use getters (e.g. `config.get_string_or("APP_NAME", ...)`) instead of `config.app_name`.
- Update `template_app_service_rs` to use the new API (resolve `AppConfig`/`ConfigService` from container and call getters), removing `FromEnv` + `EnvStore::load_with_options`.
### Fix Focus Areas (locations)
- crates/nestforge-cli/src/main.rs[2289-2342]
- crates/nestforge-cli/src/main.rs[2560-2604]
- crates/nestforge-cli/src/main.rs[2680-2705]
- crates/nestforge-cli/src/main.rs[2390-2405]
- crates/nestforge-cli/src/main.rs[2727-2736]
- crates/nestforge-macros/src/lib.rs[1749-1756]
- crates/nestforge-macros/src/lib.rs[1783-1792]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+262
to
264
| pub fn for_root_with_options(options: ConfigOptions) -> ConfigService { | ||
| ConfigService::load_with_options(&options).expect("Failed to load configuration") | ||
| } |
There was a problem hiding this comment.
4. Config loading panics 🐞 Bug ⛯ Reliability
ConfigModule::for_root_with_options calls expect, so failures to load/parse the .env file crash the process; nestforge-core::register_config returns Result<()> but cannot report config load failures because it will panic first.
Agent Prompt
### Issue description
Config loading panics via `expect`, bypassing callers’ `Result`-based error handling.
### Issue Context
`nestforge-core::register_config` returns `anyhow::Result<()>` but cannot surface config load failures while `for_root_with_options` panics.
### Fix Focus Areas
- Change `ConfigModule::for_root_with_options` to return `Result<ConfigService, ConfigError>`.
- Update `load_config()` and `nestforge-core::register_config` to propagate errors instead of panicking.
### Fix Focus Areas (locations)
- crates/nestforge-config/src/lib.rs[85-112]
- crates/nestforge-config/src/lib.rs[257-273]
- crates/nestforge-core/src/config.rs[5-8]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (1)
examples/hello-nestforge-grpc/src/app_config.rs (1)
10-23:FromEnvimplementation appears unused.The
load_app_configfunction creates aConfigModuleand reads values directly viaget_string_or, but theFromEnvtrait implementation on lines 18-23 is never called. This creates confusion about the intended pattern.Consider either:
- Removing the unused
FromEnvimpl- Using
FromEnvwithinload_app_configto demonstrate the trait-based approach- Adding a comment explaining when each approach should be used
Option 1: Remove unused impl
fn load_app_config() -> anyhow::Result<AppConfig> { let options = ConfigOptions::new().env_file(".env"); let config = ConfigModule::for_root_with_options(options); Ok(AppConfig { app_name: config.get_string_or("APP_NAME", "NestForge gRPC"), }) } - -impl FromEnv for AppConfig { - fn from_env(env: &EnvStore) -> Result<Self, ConfigError> { - Ok(Self { - app_name: env.get("APP_NAME").unwrap_or("NestForge gRPC").to_string(), - }) - } -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/hello-nestforge-grpc/src/app_config.rs` around lines 10 - 23, The FromEnv impl for AppConfig is unused; either remove the impl block (impl FromEnv for AppConfig { ... }) to avoid dead code, or update load_app_config to construct AppConfig via the trait (e.g., obtain the EnvStore from ConfigModule and call AppConfig::from_env(env) or equivalent) so FromEnv is actually invoked; pick one and apply consistently to the AppConfig, load_app_config, FromEnv and EnvStore symbols.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Cargo.toml`:
- Line 48: Remove the unused workspace dependency declaration for the validator
crate (the line declaring validator = { version = "0.16", features = ["derive"]
} in Cargo.toml) since the project uses its own Validate trait and custom
#[derive(Validate)] implementation; delete that dependency entry from Cargo.toml
and run cargo update / cargo metadata to ensure the workspace lockfile and
dependency tree are cleaned up.
In `@crates/nestforge-cli/src/main.rs`:
- Around line 2727-2736: The generated app_config.rs template
(template_app_config_rs) now imports nestforge_config and exposes
load_config/ConfigService, which is inconsistent with template_app_service_rs
and template_app_module_rs that expect AppConfig as a struct/FromEnv with fields
like config.app_name and do not call load_config; fix by making the templates
consistent: either (A) revert template_app_config_rs to generate the original
AppConfig struct implementing FromEnv with the expected fields (e.g., app_name)
so it matches template_app_service_rs and keep template_app_cargo_toml depending
on nestforge, or (B) update template_app_cargo_toml to add nestforge_config and
modify template_app_service_rs and template_app_module_rs to call load_config()
and use the ConfigService API; pick one approach and apply it across
template_app_config_rs, template_app_cargo_toml, template_app_service_rs, and
template_app_module_rs so all references to AppConfig/load_config/nestforge vs
nestforge_config are consistent.
In `@crates/nestforge-config/README.md`:
- Around line 20-22: Update the README's example to match the project's standard
initialization pattern by replacing the use of
ConfigModule::for_root().env_file(".env") with the
ConfigOptions::new().env_file(".env") pattern shown in examples; specifically,
in the load_config() example (returning AppConfig) use ConfigOptions::new() to
build the options (instead of calling ConfigModule::for_root()) so the docs
align with examples and the code paths that ultimately call
ConfigOptions::new().
In `@crates/nestforge-config/src/lib.rs`:
- Around line 275-290: Config<T> currently only stores PhantomData and thus
holds no underlying value; change Config<T> to actually contain the wrapped
configuration (e.g., a field like inner: T or cfg: T) and update the constructor
and Default impl to initialize that field (for Default require T: Default). Also
implement or forward the accessor methods used in docs (e.g., get_string_or) by
either impl Deref<Target=T> for Config<T> or by adding matching wrapper methods
on Config that call through to the inner T (referencing Config<T>, Config::new,
and the Default impl to locate where to change).
- Around line 14-15: The accessors currently swallow missing keys and parse
failures (returning ""/0/false) instead of surfacing the error; change each
getter (e.g., the string/uint/bool accessors referenced around lines 117–217) to
return Result<T, ConfigError> and: 1) return Err(ConfigError::MissingKey { key:
key.to_string() }) when the environment key is absent, 2) return a distinct
parse error (or extend ConfigError with a Parse variant) when conversion fails,
and 3) remove the silent defaulting to empty/0/false so callers must handle the
Result; update call sites to propagate or handle these errors.
- Around line 18-32: EnvSchema currently accumulates required keys in the
requirements Vec but never enforces them; add a validation step that fails
startup when a required env var is missing. Implement a method on EnvSchema like
validate_required(&self) -> Result<(), anyhow::Error> (or suitable error type)
that iterates self.requirements and checks std::env::var(key).map_err(...) for
missing keys, returning an error listing all missing keys, and then invoke this
method from the code path that constructs/applies the schema (where
EnvSchema::new() / .required(...) is used) so required-key validation runs
during startup.
- Around line 71-73: The trait FromEnv currently only declares fn
from_env(&EnvStore) -> Result<Self, ConfigError> but the derive macro for Config
generates an impl that also defines fn config_key() -> &'static str; update the
trait by adding the missing signature fn config_key() -> &'static str; so that
implementations produced by the macro match the trait (i.e., modify trait
FromEnv to include the config_key method), or alternatively remove the generated
config_key method from the derive macro in crates/nestforge-macros if you prefer
keeping the trait unchanged; reference the trait name FromEnv, the existing
method from_env and the generated method config_key to locate and reconcile the
mismatch.
- Around line 89-110: In load_with_options, do not silently drop per-entry parse
errors from dotenvy::from_path_iter; replace the current .for_each block with
.try_for_each and map any per-item error into a ConfigError variant (e.g.,
create/return ConfigError::ParseEnvEntry with the path and source) so malformed
.env lines cause load_with_options to return Err instead of being ignored;
ensure the mapping happens after dotenvy::from_path_iter(...) and before merging
into values.
In `@crates/nestforge-core/Cargo.toml`:
- Around line 22-23: Remove the unused dependency declaration "validator = {
workspace = true }" from the Cargo.toml so only the actively used path
dependency "nestforge-config = { path = \"../nestforge-config\" }" remains;
update the Cargo.toml by deleting the validator line and then run cargo metadata
or cargo build to verify no other references to the validator crate exist.
In `@crates/nestforge-core/src/config.rs`:
- Around line 14-29: The test test_register_config_loads currently mutates
process-wide environment via std::env::set_var/remove_var which is not
thread-safe; instead modify the test to avoid touching std::env by constructing
ConfigOptions with test-specific values (e.g., use
ConfigOptions::new().env_file("tests/fixtures/test.env") or a builder that sets
APP_NAME directly) and pass that into register_config(&container, options), then
assert via container.resolve::<ConfigService>() without calling
std::env::set_var/remove_var; alternatively mark the test to run serially if
changing ConfigOptions is not possible.
- Around line 5-8: The current register_config function calls
ConfigModule::for_root_with_options which panics on load failure; change to use
a fallible loader instead: add a try_for_root_with_options on ConfigModule that
delegates to ConfigService::load_with_options returning Result<ConfigService,
ConfigError>, then update register_config to call
ConfigModule::try_for_root_with_options(options) and propagate the error (return
Err) instead of allowing a panic; ensure the signature of register_config stays
Result<()> and that ConfigOptions and ConfigError types are used for the
fallible path.
In `@crates/nestforge-macros/src/lib.rs`:
- Around line 2027-2063: The public proc-macro derive Config (function
derive_config and the #[proc_macro_derive(Config)] export) is incomplete and
emits compile-time errors; remove the public derive by deleting the
derive_config function and its #[proc_macro_derive(Config)] attribute (or if you
prefer to keep it temporarily, remove the proc-macro export and mark it hidden
by removing #[proc_macro_derive(Config)] and adding a non-exported helper),
ensuring there are no remaining references to Config derive in the crate;
alternatively, fully implement derive_config to generate valid FromEnv and
Default impls if you intend to keep the API — in either case update Cargo export
so the public proc-macro is no longer exposed in its broken form.
In `@docs/config-module.md`:
- Around line 23-31: The examples import crate symbols directly from
nestforge_config (ConfigService, ConfigModule) which aren’t provided by the
scaffolded Cargo.toml; update the snippets to either re-export these symbols
from the top-level crate (use nestforge::{ConfigService, ConfigModule}) or
explicitly document/add the nestforge_config dependency in the manifest;
specifically change the import lines used by AppConfig and load_config (and the
other occurrences around the examples that reference ConfigService/ConfigModule)
so the examples compile when copy-pasted.
- Around line 214-219: The example function load_config incorrectly calls the
infallible helper
ConfigModule::for_root_with_options(ConfigModule::for_root().env_file(".env"))
while declaring a fallible signature Result<AppConfig, ConfigError>; update the
snippet to call the fallible API instead—invoke
ConfigService::load_with_options(...) (passing the options from
ConfigModule::for_root().env_file(".env")) so the return type matches
Result<AppConfig, ConfigError>, or alternatively change the example to show an
infallible signature that returns ConfigService if you intend to keep
ConfigModule::for_root_with_options; reference
ConfigModule::for_root_with_options, ConfigModule::for_root,
ConfigService::load_with_options, AppConfig, and ConfigError when making the
fix.
- Around line 129-147: The example using static DATABASE_CONFIG is invalid
because Rust statics can't call non-const functions or allocate Strings at
compile time; replace the static with a lazy-initialized value using
once_cell::sync::Lazy or std::sync::OnceLock (e.g., create a Lazy/OnceLock named
DATABASE_CONFIG that calls register_config("database", || DatabaseSettings { ...
}) so the allocation happens at runtime), or alternatively show a non-static
local variable example; update the docs to reference the DatabaseSettings struct
and the register_config call and mention using Lazy/OnceLock (not a plain
static) to make the snippet compile.
---
Nitpick comments:
In `@examples/hello-nestforge-grpc/src/app_config.rs`:
- Around line 10-23: The FromEnv impl for AppConfig is unused; either remove the
impl block (impl FromEnv for AppConfig { ... }) to avoid dead code, or update
load_app_config to construct AppConfig via the trait (e.g., obtain the EnvStore
from ConfigModule and call AppConfig::from_env(env) or equivalent) so FromEnv is
actually invoked; pick one and apply consistently to the AppConfig,
load_app_config, FromEnv and EnvStore symbols.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 175d8bdb-0201-4b6d-8eab-9d2410eaba9d
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
Cargo.tomlcrates/nestforge-cli/src/main.rscrates/nestforge-config/Cargo.tomlcrates/nestforge-config/README.mdcrates/nestforge-config/src/lib.rscrates/nestforge-core/Cargo.tomlcrates/nestforge-core/src/config.rscrates/nestforge-core/src/lib.rscrates/nestforge-macros/src/lib.rscrates/nestforge/src/lib.rsdocs/config-module.mdexamples/hello-nestforge-graphql/src/app_config.rsexamples/hello-nestforge-grpc/src/app_config.rsexamples/hello-nestforge-microservices/src/app_config.rsexamples/hello-nestforge-websockets/src/app_config.rsexamples/hello-nestforge/src/app_config.rs
| http = "1" | ||
| anyhow = "1" | ||
| thiserror = "2" | ||
| validator = { version = "0.16", features = ["derive"] } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if validator is actually used in nestforge-config or nestforge-core
rg -l 'validator' --type=toml -g '*/nestforge-config/*' -g '*/nestforge-core/*'
rg 'use.*validator|validator::' --type=rustRepository: vernonthedev/nestforge
Length of output: 98
🏁 Script executed:
# Check Cargo.toml files to see where validator is declared
cat crates/nestforge-core/Cargo.toml | grep -A 2 -B 2 validator
# Also search for any validator references in the entire Rust codebase
rg 'validator' --type=rust -iRepository: vernonthedev/nestforge
Length of output: 179
🏁 Script executed:
# Search for any usage of validator in the entire Rust codebase
rg 'validator' --type=rust
# Also check if it's only declared but not used (check for imports more broadly)
rg -i 'validate|validator' --type=rust -A 2Repository: vernonthedev/nestforge
Length of output: 14611
🏁 Script executed:
# Confirm by checking the workspace Cargo.toml to see if validator is truly a workspace dependency
cat Cargo.toml | grep -A 5 "\[workspace"Repository: vernonthedev/nestforge
Length of output: 639
🏁 Script executed:
# Check if validator is in workspace.dependencies section
cat Cargo.toml | grep -A 50 "\[workspace.dependencies\]" | grep -i validator
# Also check the full nestforge-core Cargo.toml dependencies section
cat crates/nestforge-core/Cargo.toml | grep -A 30 "\[dependencies\]"Repository: vernonthedev/nestforge
Length of output: 432
Remove unused validator dependency.
The validator crate is declared as a workspace dependency but is never used. The codebase implements a custom validation system with the Validate trait and #[derive(Validate)] macro instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Cargo.toml` at line 48, Remove the unused workspace dependency declaration
for the validator crate (the line declaring validator = { version = "0.16",
features = ["derive"] } in Cargo.toml) since the project uses its own Validate
trait and custom #[derive(Validate)] implementation; delete that dependency
entry from Cargo.toml and run cargo update / cargo metadata to ensure the
workspace lockfile and dependency tree are cleaned up.
Comment on lines
+2727
to
+2736
| fn template_app_config_rs(_transport: AppTransport) -> String { | ||
| r#"use nestforge_config::{ConfigService, ConfigModule}; | ||
|
|
||
| fn load_app_config() -> anyhow::Result<AppConfig> {{ | ||
| Ok(ConfigModule::for_root::<AppConfig>(ConfigOptions::new().env_file(".env"))?) | ||
| }} | ||
| pub type AppConfig = ConfigService; | ||
|
|
||
| impl nestforge::FromEnv for AppConfig {{ | ||
| fn from_env(env: &nestforge::EnvStore) -> Result<Self, nestforge::ConfigError> {{ | ||
| let app_name = env.get("APP_NAME").unwrap_or("{default_app_name}").to_string(); | ||
| Ok(Self {{ app_name }}) | ||
| }} | ||
| }} | ||
| pub fn load_config() -> AppConfig { | ||
| ConfigModule::for_root_with_options(ConfigModule::for_root().env_file(".env")) | ||
| } | ||
| "# | ||
| ) | ||
| .to_string() |
There was a problem hiding this comment.
This generated app_config.rs no longer matches the rest of the scaffold.
Line 2728 switches the template to nestforge_config, but template_app_cargo_toml() still only adds nestforge as a dependency. More importantly, the other generated files in this same source file still treat AppConfig as a struct/FromEnv type (template_app_service_rs reads config.app_name, and template_app_module_rs never calls load_config()), so fresh projects will not compile or load .env through this path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/nestforge-cli/src/main.rs` around lines 2727 - 2736, The generated
app_config.rs template (template_app_config_rs) now imports nestforge_config and
exposes load_config/ConfigService, which is inconsistent with
template_app_service_rs and template_app_module_rs that expect AppConfig as a
struct/FromEnv with fields like config.app_name and do not call load_config; fix
by making the templates consistent: either (A) revert template_app_config_rs to
generate the original AppConfig struct implementing FromEnv with the expected
fields (e.g., app_name) so it matches template_app_service_rs and keep
template_app_cargo_toml depending on nestforge, or (B) update
template_app_cargo_toml to add nestforge_config and modify
template_app_service_rs and template_app_module_rs to call load_config() and use
the ConfigService API; pick one approach and apply it across
template_app_config_rs, template_app_cargo_toml, template_app_service_rs, and
template_app_module_rs so all references to AppConfig/load_config/nestforge vs
nestforge_config are consistent.
Comment on lines
+20
to
+22
| pub fn load_config() -> AppConfig { | ||
| ConfigModule::for_root_with_options(ConfigModule::for_root().env_file(".env")) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the actual for_root method signature and usage
ast-grep --pattern 'fn for_root($$$) -> $_ {
$$$
}'
# Check how ConfigOptions is created
rg 'ConfigModule::for_root\(' --type=rust
rg 'ConfigOptions::new\(\)' --type=rustRepository: vernonthedev/nestforge
Length of output: 1225
🏁 Script executed:
# Check the ConfigModule::for_root() method signature and what it returns
ast-grep --pattern 'impl ConfigModule {
$$$
fn for_root($$$) {
$$$
}
$$$
}'
# Also check for ConfigModule::for_root_with_options signature
ast-grep --pattern 'fn for_root_with_options($$$) {
$$$
}'Repository: vernonthedev/nestforge
Length of output: 48
🏁 Script executed:
# Get the README.md content around lines 20-22 for full context
sed -n '15,30p' crates/nestforge-config/README.mdRepository: vernonthedev/nestforge
Length of output: 373
🏁 Script executed:
# Check what ConfigModule::for_root() actually returns by looking at the implementation
rg -A 5 'fn for_root\(' crates/nestforge-config/src/lib.rs --type=rustRepository: vernonthedev/nestforge
Length of output: 308
Standardize configuration initialization pattern across documentation and examples.
The example code is correct—ConfigModule::for_root() returns ConfigOptions, and calling .env_file(".env") on it works as expected. However, the README uses ConfigModule::for_root().env_file(".env") while all example projects use ConfigOptions::new().env_file(".env") directly (both patterns are functionally equivalent since for_root() internally calls ConfigOptions::new()). For consistency and clarity, align the README to use the same pattern as the actual examples.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/nestforge-config/README.md` around lines 20 - 22, Update the README's
example to match the project's standard initialization pattern by replacing the
use of ConfigModule::for_root().env_file(".env") with the
ConfigOptions::new().env_file(".env") pattern shown in examples; specifically,
in the load_config() example (returning AppConfig) use ConfigOptions::new() to
build the options (instead of calling ConfigModule::for_root()) so the docs
align with examples and the code paths that ultimately call
ConfigOptions::new().
Comment on lines
+14
to
15
| #[error("Missing config key: {key}")] | ||
| MissingKey { key: String }, |
There was a problem hiding this comment.
These getters hide bad config instead of surfacing it.
Line 14 introduces MissingKey, but none of the accessors use it: missing keys and parse failures both collapse to empty string/0/false. That makes values like PORT=abc indistinguishable from a missing key and defeats the fail-fast startup validation this module is supposed to provide.
Also applies to: 117-217
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/nestforge-config/src/lib.rs` around lines 14 - 15, The accessors
currently swallow missing keys and parse failures (returning ""/0/false) instead
of surfacing the error; change each getter (e.g., the string/uint/bool accessors
referenced around lines 117–217) to return Result<T, ConfigError> and: 1) return
Err(ConfigError::MissingKey { key: key.to_string() }) when the environment key
is absent, 2) return a distinct parse error (or extend ConfigError with a Parse
variant) when conversion fails, and 3) remove the silent defaulting to
empty/0/false so callers must handle the Result; update call sites to propagate
or handle these errors.
Comment on lines
+18
to
+32
| #[derive(Clone, Debug, Default)] | ||
| pub struct EnvSchema { | ||
| requirements: Vec<String>, | ||
| } | ||
|
|
||
| impl EnvSchema { | ||
| pub fn new() -> Self { | ||
| Self::default() | ||
| } | ||
|
|
||
| pub fn required(&mut self, key: &str) -> &mut Self { | ||
| self.requirements.push(key.to_string()); | ||
| self | ||
| } | ||
| } |
There was a problem hiding this comment.
EnvSchema is a no-op in the new flow.
Lines 28-30 collect required keys, but nothing in Lines 257-268 or the rest of this file ever consumes requirements. Existing code can still build a schema, yet required-key validation never runs, which drops the startup-validation/backward-compatibility contract instead of preserving it.
Also applies to: 255-269
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/nestforge-config/src/lib.rs` around lines 18 - 32, EnvSchema currently
accumulates required keys in the requirements Vec but never enforces them; add a
validation step that fails startup when a required env var is missing. Implement
a method on EnvSchema like validate_required(&self) -> Result<(), anyhow::Error>
(or suitable error type) that iterates self.requirements and checks
std::env::var(key).map_err(...) for missing keys, returning an error listing all
missing keys, and then invoke this method from the code path that
constructs/applies the schema (where EnvSchema::new() / .required(...) is used)
so required-key validation runs during startup.
Comment on lines
+14
to
+29
| #[test] | ||
| fn test_register_config_loads() { | ||
| std::env::set_var("APP_NAME", "TestApp"); | ||
|
|
||
| let container = Container::new(); | ||
| let options = ConfigOptions::new(); | ||
|
|
||
| let result = register_config(&container, options); | ||
| assert!(result.is_ok()); | ||
|
|
||
| let resolved = container.resolve::<ConfigService>(); | ||
| assert!(resolved.is_ok()); | ||
| assert_eq!(resolved.unwrap().get("APP_NAME"), Some("TestApp")); | ||
|
|
||
| std::env::remove_var("APP_NAME"); | ||
| } |
There was a problem hiding this comment.
Test uses std::env::set_var/remove_var which is not thread-safe.
Modifying environment variables in tests can cause flaky behavior when tests run in parallel, as std::env is process-global. Consider using a test-specific ConfigOptions that doesn't rely on process environment, or use a serial test harness.
Alternative: use ConfigOptions with explicit values
If ConfigOptions supports setting values directly (or via a test .env file), that would be more robust:
#[test]
fn test_register_config_loads() {
let options = ConfigOptions::new()
.env_file("tests/fixtures/test.env"); // contains APP_NAME=TestApp
// ...
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/nestforge-core/src/config.rs` around lines 14 - 29, The test
test_register_config_loads currently mutates process-wide environment via
std::env::set_var/remove_var which is not thread-safe; instead modify the test
to avoid touching std::env by constructing ConfigOptions with test-specific
values (e.g., use ConfigOptions::new().env_file("tests/fixtures/test.env") or a
builder that sets APP_NAME directly) and pass that into
register_config(&container, options), then assert via
container.resolve::<ConfigService>() without calling
std::env::set_var/remove_var; alternatively mark the test to run serially if
changing ConfigOptions is not possible.
Comment on lines
+2027
to
+2063
| #[proc_macro_derive(Config)] | ||
| pub fn derive_config(item: TokenStream) -> TokenStream { | ||
| let input = parse_macro_input!(item as DeriveInput); | ||
| let name = &input.ident; | ||
| let (impl_generics, ty_generics, where_clause) = &input.generics.split_for_impl(); | ||
|
|
||
| let Data::Struct(_data) = &input.data else { | ||
| return syn::Error::new(input.ident.span(), "Config can only be derived on structs") | ||
| .to_compile_error() | ||
| .into(); | ||
| }; | ||
|
|
||
| let expanded = quote! { | ||
| impl #impl_generics nestforge_config::FromEnv for #name #ty_generics #where_clause { | ||
| fn from_env(env: &nestforge_config::EnvStore) -> Result<Self, nestforge_config::ConfigError> { | ||
| std::compile_error!( | ||
| "Config derive requires manual FromEnv implementation. \ | ||
| Use `impl FromEnv for YourConfig` with `env.get(\"KEY\")` to read values." | ||
| ); | ||
| } | ||
|
|
||
| fn config_key() -> &'static str { | ||
| stringify!(#name) | ||
| } | ||
| } | ||
|
|
||
| impl #impl_generics std::default::Default for #name #ty_generics #where_clause { | ||
| fn default() -> Self { | ||
| std::compile_error!( | ||
| "Config derive requires manual Default implementation or provide default values." | ||
| ); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| TokenStream::from(expanded) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if Config derive is actually used anywhere
rg '#\[derive\(.*Config.*\)\]' --type=rustRepository: vernonthedev/nestforge
Length of output: 48
🏁 Script executed:
# Check visibility and documentation of Config derive macro
head -n 2100 crates/nestforge-macros/src/lib.rs | tail -n 100Repository: vernonthedev/nestforge
Length of output: 3110
🏁 Script executed:
# Search for test files or tests related to Config
find . -type f -name "*.rs" -path "*/tests/*" | head -20Repository: vernonthedev/nestforge
Length of output: 959
🏁 Script executed:
# Check if there's any documentation or examples for Config derive
rg -i "config.*derive|derive.*config" --type=rust -A 2 -B 2 | head -50Repository: vernonthedev/nestforge
Length of output: 1722
🏁 Script executed:
# Check module structure to see what's exported
rg "^pub " crates/nestforge-macros/src/lib.rs | head -20Repository: vernonthedev/nestforge
Length of output: 1503
🏁 Script executed:
# Look for any documentation files mentioning Config derive
find . -type f \( -name "*.md" -o -name "*.txt" \) | xargs grep -l "Config" 2>/dev/null | head -10Repository: vernonthedev/nestforge
Length of output: 282
🏁 Script executed:
# Check the config module documentation
cat crates/nestforge-config/README.mdRepository: vernonthedev/nestforge
Length of output: 1802
🏁 Script executed:
# Check the config-module documentation
cat docs/config-module.mdRepository: vernonthedev/nestforge
Length of output: 5038
🏁 Script executed:
# Look for any TODO/FIXME comments related to Config derive
rg -i "todo|fixme" crates/nestforge-macros/src/lib.rs | grep -i configRepository: vernonthedev/nestforge
Length of output: 48
🏁 Script executed:
# Check what's exported from the macros crate in Cargo.toml
head -30 crates/nestforge-macros/Cargo.tomlRepository: vernonthedev/nestforge
Length of output: 598
🏁 Script executed:
# Check if Config derive is mentioned in main crate docs or examples
rg -i "derive.*config|config.*derive" docs/ README.md 2>/dev/nullRepository: vernonthedev/nestforge
Length of output: 48
Remove or implement the unused Config derive macro.
The #[derive(Config)] macro is public and exported, but completely undocumented, unused in the codebase, and generates std::compile_error! in both required method implementations. This appears to be incomplete/unfinished work accidentally exposed as a public API.
The documented configuration approach (shown in docs/config-module.md) uses ConfigModule::for_root() and Config<AppConfig> injection, not a derive macro. For typed configs, the docs recommend register_config() instead.
Since the macro is neither used internally nor documented, and it's non-functional as written, either remove it or mark it as private while completing the implementation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/nestforge-macros/src/lib.rs` around lines 2027 - 2063, The public
proc-macro derive Config (function derive_config and the
#[proc_macro_derive(Config)] export) is incomplete and emits compile-time
errors; remove the public derive by deleting the derive_config function and its
#[proc_macro_derive(Config)] attribute (or if you prefer to keep it temporarily,
remove the proc-macro export and mark it hidden by removing
#[proc_macro_derive(Config)] and adding a non-exported helper), ensuring there
are no remaining references to Config derive in the crate; alternatively, fully
implement derive_config to generate valid FromEnv and Default impls if you
intend to keep the API — in either case update Cargo export so the public
proc-macro is no longer exposed in its broken form.
Comment on lines
23
to
31
| ```rust | ||
| fn load_app_config() -> anyhow::Result<AppConfig> { | ||
| let levels = vec!["trace", "debug", "info", "warn", "error"]; | ||
| let schema = nestforge::EnvSchema::new() | ||
| .required("APP_NAME") | ||
| .min_len("APP_NAME", 2) | ||
| .one_of("LOG_LEVEL", &levels); | ||
| // src/app_config.rs | ||
| use nestforge_config::{ConfigService, ConfigModule}; | ||
|
|
||
| pub type AppConfig = ConfigService; | ||
|
|
||
| Ok(nestforge::ConfigModule::for_root::<AppConfig>( | ||
| nestforge::ConfigOptions::new().env_file(".env").validate_with(schema), | ||
| )?) | ||
| pub fn load_config() -> AppConfig { | ||
| ConfigModule::for_root_with_options(ConfigModule::for_root().env_file(".env")) | ||
| } |
There was a problem hiding this comment.
These snippets use a crate the documented install path never adds.
Line 25, Line 116, and Line 132 import nestforge_config, but the page says config support is included through nestforge, and the scaffolded manifest only depends on nestforge. Copy-pasting these examples into a generated app yields unresolved imports unless you either document the extra dependency or switch the snippets to the nestforge re-exports.
Also applies to: 115-123, 131-133
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/config-module.md` around lines 23 - 31, The examples import crate
symbols directly from nestforge_config (ConfigService, ConfigModule) which
aren’t provided by the scaffolded Cargo.toml; update the snippets to either
re-export these symbols from the top-level crate (use nestforge::{ConfigService,
ConfigModule}) or explicitly document/add the nestforge_config dependency in the
manifest; specifically change the import lines used by AppConfig and load_config
(and the other occurrences around the examples that reference
ConfigService/ConfigModule) so the examples compile when copy-pasted.
Comment on lines
+129
to
+147
| For strongly-typed configuration structures, use `register_config`: | ||
|
|
||
| You can customize this with `ConfigOptions`. | ||
| ```rust | ||
| use nestforge_config::register_config; | ||
|
|
||
| #[derive(Debug, Clone)] | ||
| pub struct DatabaseSettings { | ||
| pub host: String, | ||
| pub port: u16, | ||
| pub username: String, | ||
| pub password: String, | ||
| } | ||
|
|
||
| pub static DATABASE_CONFIG = register_config("database", || DatabaseSettings { | ||
| host: "localhost".to_string(), | ||
| port: 5432, | ||
| username: "postgres".to_string(), | ||
| password: "password".to_string(), | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Can a Rust staticinitializer call a non-const function or allocateStrings?
💡 Result:
No, a Rust static initializer cannot call a non-const function or allocate Strings. Static initializers must be constant expressions evaluated at compile time. They can only call const fns (constant functions), not regular non-const functions. This is explicitly stated in the Rust Reference: "The static initializer is a constant expression evaluated at compile time." (Result 16,17). Compiler errors confirm this, e.g., "cannot call non-const fn Mutex::new in statics" with note "calls in statics are limited to constant functions" (Result 1). String::new requires heap allocation and is not const until Rust 1.39 for the empty case (Result 7,8), but even then, non-empty Strings from non-const operations fail. String allocation violates compile-time evaluation requirements. Use &'static str for static strings instead (Result 6,12). Workarounds like lazy_static, once_cell, or static mut with runtime init exist but are not true static initializers (Results 1,2,4). const items share similar restrictions but are inlined, not single-address like static (Result 12,16).
Citations:
- 1: Suggest lazy initialization when calling a non-const function in a static initializer rust-lang/rust#100410
- 2: https://stackoverflow.com/questions/65120456/how-to-initialize-immutable-globals-with-non-const-initializer-in-rust
- 3: https://stackoverflow.com/questions/55977067/how-can-i-create-a-static-string-in-rust
- 4: https://doc.rust-lang.org/beta/alloc/string/struct.String.html
- 5: https://doc.rust-lang.org/stable/alloc/string/struct.String.html
- 6: https://doc.rust-lang.org/reference/items/constant-items.html
- 7: http://doc.rust-lang.org/stable/reference/items/static-items.html
- 8: https://doc.rust-lang.org/1.73.0/reference/items/static-items.html
The static DATABASE_CONFIG example does not compile as written.
Rust static initializers cannot call non-const functions like register_config(...), and this example allocates Strings via .to_string(). Static initializers require compile-time constant expressions. Use LazyLock/once_cell instead, or provide a non-static example.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/config-module.md` around lines 129 - 147, The example using static
DATABASE_CONFIG is invalid because Rust statics can't call non-const functions
or allocate Strings at compile time; replace the static with a lazy-initialized
value using once_cell::sync::Lazy or std::sync::OnceLock (e.g., create a
Lazy/OnceLock named DATABASE_CONFIG that calls register_config("database", ||
DatabaseSettings { ... }) so the allocation happens at runtime), or
alternatively show a non-static local variable example; update the docs to
reference the DatabaseSettings struct and the register_config call and mention
using Lazy/OnceLock (not a plain static) to make the snippet compile.
Comment on lines
+214
to
+219
| For custom error handling: | ||
|
|
||
| ```rust | ||
| pub fn load_config() -> Result<AppConfig, ConfigError> { | ||
| ConfigModule::for_root_with_options(ConfigModule::for_root().env_file(".env")) | ||
| } |
There was a problem hiding this comment.
This fallible-loading example is calling the infallible helper.
ConfigModule::for_root_with_options(...) returns ConfigService, not Result<_, ConfigError>, so Lines 217-219 do not type-check. If you want to document custom error handling, the snippet needs to call ConfigService::load_with_options(&...) instead, or the crate needs a fallible ConfigModule helper before this example is documented.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/config-module.md` around lines 214 - 219, The example function
load_config incorrectly calls the infallible helper
ConfigModule::for_root_with_options(ConfigModule::for_root().env_file(".env"))
while declaring a fallible signature Result<AppConfig, ConfigError>; update the
snippet to call the fallible API instead—invoke
ConfigService::load_with_options(...) (passing the options from
ConfigModule::for_root().env_file(".env")) so the return type matches
Result<AppConfig, ConfigError>, or alternatively change the example to show an
infallible signature that returns ConfigService if you intend to keep
ConfigModule::for_root_with_options; reference
ConfigModule::for_root_with_options, ConfigModule::for_root,
ConfigService::load_with_options, AppConfig, and ConfigError when making the
fix.
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
1 participant
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
This PR introduces a centralized, NestJS-inspired configuration system for NestForge. It provides type-safe, injectable, and easy-to-use configuration management that ensures applications load configuration from
.envfiles at startup.Changes
nestforge-config crate (enhanced)
ConfigService- like NestJS ConfigService for loading.envfiles and accessing environment variablesConfigModule::for_root()andConfigModule::for_feature()for modular configurationget_string(),get_string_or(),get_u16_or(),get_bool_or(), etc.Config<T>wrapper for DI-compatible injectionregister_config()for typed configuration registrationload_config()convenience functionEnvSchemafor backward compatibility with existing examplesFromEnv,EnvStore,EnvValidationIssuenestforge-core crate (updated)
register_config()function to integrate ConfigService with the DI containernestforge-cli crate (updated)
nestforge newscaffolding to generateapp_config.rswith ConfigService setupexamples (updated)
docs (updated)
docs/config-module.mdnestforge-config/README.mdValidation
cargo test --lib- all workspace tests passcargo clippy -- -D warningson nestforge-config - passesPerformance Impact
dotenvydependencyDocs
docs/config-module.mdwith comprehensive usage guidecrates/nestforge-config/README.mdArchitecture
Usage Examples
Basic Setup (via CLI - automatic)
nestforge new my-app # Generates app_config.rs automaticallyManual Setup
In Services - NestJS-like Ergonomics
ConfigService Direct Usage
Typed Configs (like NestJS registerAs)
NestJS Comparison
ConfigModule.forRoot({ isGlobal: true })ConfigModule::for_root()configService.get('KEY')config.get_string("KEY")configService.getOrThrow('KEY')config.get("KEY").unwrap()registerAs('config', () => ({ ... }))register_config("name", || Config { ... })@Inject(ConfigService)Config<AppConfig>Breaking Change Analysis
None. This is a new, optional module. Existing code is unaffected.
Linked Issues
nestforge-configfor Type-Safe, Modular Configuration #23Summary by CodeRabbit
New Features
get_string,get_u16,get_bool, etc.) for simplified config value retrievalConfigServiceandConfig<T>.envfile support for environment-based configurationDocumentation