Implement TypeScript-style import transpiler with CLI commands and config updates#27
Open
vernonthedev wants to merge 11 commits into
Open
Implement TypeScript-style import transpiler with CLI commands and config updates#27vernonthedev wants to merge 11 commits into
vernonthedev wants to merge 11 commits into
Conversation
## Bug Fixes ## Critical 1. CLI scaffolded app won't compile - Added nestforge-config dependency to scaffolded Cargo.toml, fixed template imports to use nestforge instead of nestforge_config, updated module templates to use load_config() factory pattern 2. Config<T> wrapper unusable - Now wraps Arc<ConfigService> with full getter forwarding and Deref impl 3. Process env precedence reversed - Changed to or_insert_with so runtime env takes precedence over .env file ## Major 4. Config loading panics - Added try_for_root_with_options() returning Result<ConfigService, ConfigError> 5. Dotenv errors silently dropped - Malformed .env lines now propagate errors 6. Broken #[derive(Config)] macro - Removed broken macro that generated invalid code ## Updates - Updated all 5 examples to use new consistent config API - Added load_config() function to examples for DI registration - Removed unused FromEnv implementations from examples All workspace tests pass.
This module provides the common NestForge types and macros that users
can import using TypeScript-style syntax. When combined with the
transpiler, users can write:
import { Module, Controller, Get } from "nestforge/common";
Which gets transpiled to Rust's:
use nestforge::common::{Module, Controller, Get};
The common module re-exports:
- Core framework types (ModuleDefinition, Container, Controller, Guard, etc.)
- Decorator macros (#[module], #[controller], #[injectable], #[get], etc.)
- Configuration types when the config feature is enabled
This enables 1:1 parity with the NestJS developer experience.
This adds a source-to-source transpiler that converts TypeScript-style
imports to valid Rust use statements, enabling 1:1 parity with NestJS.
Transpiler Features
Named imports: import { Module, Controller } from "nestforge/common"
Default imports: import MyService from "./my.service"
NestForge imports: import { Get, Post } from "nestforge/common"
Relative imports: import { UsersService } from "./users/users.service"
Parent imports: import { Config } from "../config"
Path Transformations
- "nestforge/common" -> nestforge::common
- "./file.service" -> self::file_service
- "../folder/file" -> super::folder::file
Case Conversion
- Automatically converts kebab-case/camelCase to snake_case
- users.service -> users_service
- auth-controller -> auth_controller
Shadow Build Process
- Scans src/ directory for files containing "import" keyword
- Transpiles code and writes to .nestforge/cache directory
- Auto-generates mod.rs files with pub mod declarations
CLI Commands Added
nestforge start - Transpiles imports then runs the application with cargo run.
nestforge dev (alias: nestforge run) - Transpiles imports then runs in development mode with watch.
Technical Details
- Uses regex for import pattern matching
- Processes entire directory tree recursively
- Preserves original source files unchanged
- Zero runtime overhead (final output is standard Rust)
Breaking Change
Standard cargo commands will fail on files using import syntax.
Users must use "nestforge start" or "nestforge dev" to trigger
the transpilation layer.
Adds regex crate dependency for the transpiler module.
- Added documentation for nestforge start and nestforge dev commands - Added TypeScript-style imports guide with supported patterns - Documented path transformations and case conversion - Added transpiler unit tests - Added tempfile dev dependency for tests Commands documented: - nestforge start: runs app with import transpilation - nestforge dev: runs app in development mode with transpilation
- README.md: Added TypeScript-style imports section with example - quick-start.md: Updated run commands to use nestforge start/dev - core-concepts.md: Added TypeScript-Style Imports section - example-app.md: Added transpiler info and running instructions
…FromEnv - Add FromEnv implementation for AppConfig - Refactor load_app_config to use ConfigService and EnvStore - Update load_config to use ConfigService - Remove ConfigService from AppModule exports - Add .nestforge.toml configuration file This change modernizes the example's configuration handling to leverage the new common module features, improving consistency with the library's API.
- Adjust indentation in code examples - Add blank line in transpilation instructions - Remove Contributing and License sections
vernonthedev
added
enhancement
github-actions
Bot
added
documentation
📝 WalkthroughWalkthroughThis PR implements a TypeScript-style import transpiler for NestForge, allowing developers to write Changes
Sequence DiagramsequenceDiagram
actor Developer
participant CLI as nestforge CLI
participant FileSystem as File System
participant Transpiler as Transpiler
participant ModGen as Module Generator
participant Cargo as cargo build
Developer->>CLI: nestforge start/dev
CLI->>FileSystem: scan src/ directory
FileSystem-->>CLI: file list
loop for each .rs file with import
CLI->>Transpiler: transpile_file(content)
Transpiler->>Transpiler: parse import patterns
Transpiler->>Transpiler: transform paths<br/>(nestforge/common→nestforge::common)
Transpiler->>Transpiler: apply case conversion<br/>(file.service→file_service)
Transpiler-->>CLI: transpiled code
end
CLI->>FileSystem: write .rs to .nestforge/cache/
loop for each directory
CLI->>ModGen: generate mod.rs declarations
ModGen->>FileSystem: write pub mod statements
end
CLI->>Cargo: cargo run (pointing to cache)
Cargo-->>Developer: compiled & running app
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 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 |
Review Summary by QodoImplement TypeScript-style import transpilation and enhance CLI with start/dev commands
WalkthroughsDescription• Implemented TypeScript-style import transpiler that converts TypeScript imports to Rust use statements with automatic path transformation and snake_case conversion • Added new nestforge start and nestforge dev CLI commands with automatic transpilation support before running applications • Enhanced configuration system with improved error handling via try_for_root_with_options method and Deref implementation for ConfigService access • Removed problematic derive_config proc macro that was generating compile errors • Refactored all example applications to use simplified configuration loading patterns with load_config() function • Added new common module with re-exports of core framework types and macros for convenient access to DI, module system, and validation primitives • Updated comprehensive documentation for TypeScript-style imports, CLI commands, and transpilation workflow • Changed project license from Apache 2.0 to MIT • Updated dependencies: validator from 0.16 to 0.19, added regex and tempfile for transpiler functionality Diagramflowchart LR
A["TypeScript-style<br/>Imports"] -->|"Transpiler"| B["Rust use<br/>Statements"]
C["nestforge start<br/>nestforge dev"] -->|"Transpile"| B
B -->|"Compile & Run"| D["Application"]
E["Enhanced Config<br/>System"] -->|"try_for_root_with_options"| F["Type-safe<br/>Configuration"]
F -->|"Dependency<br/>Injection"| D
File Changes1. crates/nestforge-cli/src/main.rs
|
Code Review by Qodo
1. start/dev ignores cache build
|
![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: 12e8828aeb
ℹ️ 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
+499
to
+503
| transpile_project(&source_dir, &cache_dir)?; | ||
| println!("Transpilation complete. Running application..."); | ||
|
|
||
| let mut cmd = Command::new("cargo"); | ||
| cmd.arg("run"); |
There was a problem hiding this comment.
run_start_command writes transpiled files to .nestforge/cache but then invokes cargo run in app_root, which still compiles src/main.rs and the original src/ modules. Any project that uses the documented TypeScript-style import ... from ... syntax will still fail to build because the transpiled output is never consumed; dev has the same issue.
Useful? React with 👍 / 👎.
Comment on lines
+127
to
+129
| let import_pattern = format!( | ||
| r#"import\s*\{{}}\s*from\s*["']{}["']\s*;"#, | ||
| regex::escape(source) |
There was a problem hiding this comment.
The generated regex here becomes import\s*\{}..., which only matches an empty {} clause, so real statements like import { Controller } from "nestforge/common"; are left in the output. That leaves invalid TypeScript syntax in transpiled Rust files and breaks compilation once the cache is used.
Useful? React with 👍 / 👎.
Comment on lines
+2797
to
+2801
| r#"use nestforge::{ConfigModule, ConfigOptions, ConfigService}; | ||
|
|
||
| pub fn load_config() -> ConfigService { | ||
| ConfigModule::for_root_with_options(ConfigOptions::new().env_file(".env")) | ||
| } |
There was a problem hiding this comment.
This template now only emits load_config() and no AppConfig type, but other generated templates still import/re-export AppConfig (for example app_module and lib.rs). As a result, freshly scaffolded apps will have unresolved AppConfig symbols and fail to compile immediately.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (12)
crates/nestforge-cli/src/transpiler.rs (2)
282-289: Test coverage missing for multi-level parent imports.This test only verifies single
../traversal. Given the bug intransform_pathfor multi-level paths like../../config, add a test case to ensure proper handling:🧪 Suggested additional test
#[test] fn test_transform_path_multi_parent_import() { let temp_dir = TempDir::new().unwrap(); let transpiler = Transpiler::new(temp_dir.path(), temp_dir.path()).unwrap(); let result = transpiler.transform_path("../../shared/utils", ""); assert_eq!(result, "super::super::shared::utils"); let result = transpiler.transform_path("../../../core", ""); assert_eq!(result, "super::super::super::core"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/nestforge-cli/src/transpiler.rs` around lines 282 - 289, Add tests covering multi-level parent imports for Transpiler::transform_path: create a new test (e.g., test_transform_path_multi_parent_import) that constructs a TempDir and Transpiler like the existing tests, then call transpiler.transform_path("../../shared/utils", "") and assert it equals "super::super::shared::utils", and call transpiler.transform_path("../../../core", "") and assert it equals "super::super::super::core"; this ensures transform_path correctly converts multiple "../" segments into repeated "super::" prefixes.
127-134: Regex compiled inside loop on every capture - performance concern.
Regex::new()is called for each matched import inside the loop. For files with many imports, this is inefficient. Consider collecting all matches first, then performing a single replacement pass, or pre-computing replacement patterns.♻️ Suggested approach: collect patterns first, then replace
+ // Collect all patterns to remove first + let mut patterns_to_remove: Vec<String> = Vec::new(); + for cap in self.import_regex.captures_iter(&content) { let symbols = cap.get(1).map(|m| m.as_str()).unwrap_or(""); let source = cap.get(2).map(|m| m.as_str()).unwrap_or(""); // ... process symbols ... - let import_pattern = format!( - r#"import\s*\{{}}\s*from\s*["']{}["']\s*;"#, - regex::escape(source) - ); - result = Regex::new(&import_pattern)? - .replace_all(&result, "") - .to_string(); + patterns_to_remove.push(cap.get(0).map(|m| m.as_str()).unwrap_or("").to_string()); } + + // Remove all matched imports + for pattern in &patterns_to_remove { + result = result.replace(pattern, ""); + }This avoids regex compilation in the loop by using the already-matched full capture directly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/nestforge-cli/src/transpiler.rs` around lines 127 - 134, The loop currently recompiles a regex for each import (the import_pattern/Regex::new call) which is slow; instead, first collect the matched import strings (or the distinct `source` values) into a Vec, then perform a single replacement pass using either a precompiled Regex built from all sources or iterate that collected list to call replace_all (not compiling inside the loop) on `result`; update the logic around `import_pattern`, `Regex::new`, and `result` so regex construction happens once outside the loop (or use the already-captured full-match strings for direct removal in one pass)..nestforge.toml (1)
1-20: Configuration file is documentation-only; transpiler doesn't read it.This
.nestforge.tomldocuments the transpiler's regex patterns and mapping rules, but theTranspilerincrates/nestforge-cli/src/transpiler.rshas these patterns hardcoded and doesn't actually load this configuration file. Consider either:
- Implementing config loading from this file in the transpiler, or
- Moving this documentation to a dedicated docs file or comments in the transpiler code to avoid user confusion about expected customization capability.
Additionally, the file is missing a trailing newline.
📝 Add trailing newline
# Custom patterns can be added, but currently hardcoded +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.nestforge.toml around lines 1 - 20, The .nestforge.toml is only documentation but the Transpiler (struct Transpiler in crates/nestforge-cli/src/transpiler.rs) still uses hardcoded patterns/mappings (e.g., default_import_pattern / default_module_pattern and any internal mapping logic), so either implement config loading or move the docs: Option A — add a Transpiler::from_config / load_config function that reads .nestforge.toml, deserializes into a TranspilerConfig (mappings and regex fields), merges with existing defaults, and replace hardcoded patterns/mappings with the loaded values; Option B — remove the misleading TOML and move its contents into comments/docs near the Transpiler implementation so behavior and customization are clear. Also add a trailing newline to .nestforge.toml (if keeping the file). Ensure you reference and update the code paths where default_import_pattern/default_module_pattern and mapping resolution currently live.examples/hello-nestforge-microservices/src/app_config.rs (1)
16-17: Use the same error-handling model forConfigServicehere.
load_app_config()now surfaces config-load failures, butload_config()still does not. Returninganyhow::Result<ConfigService>from this helper too would keep the two providers in sync and avoid future drift in startup behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/hello-nestforge-microservices/src/app_config.rs` around lines 16 - 17, Change load_config to return anyhow::Result<ConfigService> and propagate any errors from ConfigModule::for_root_with_options(ConfigOptions::new().env_file(".env")) instead of returning ConfigService directly; update the function signature (load_config) and replace the direct return with the call wrapped to return a Result so failures surface like load_app_config().docs/core-concepts.md (1)
207-240: Call out the plaincargolimitation in this section too.This introduces
importsyntax as a supported feature, but it never states that files using it only work through the NestForge CLI transpilation flow. A one-line warning here would save readers from discovering that via a compile error later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/core-concepts.md` around lines 207 - 240, Add a one-line warning under the "TypeScript-Style Imports" section noting that TypeScript-style imports only work when the code is run through the NestForge CLI transpilation flow (e.g., `nestforge start` or `nestforge dev`) and will not be recognized by plain `cargo` builds; reference the section title "TypeScript-Style Imports" and the commands `nestforge start`/`nestforge dev` so reviewers can find where to insert the sentence.examples/hello-nestforge-websockets/src/app_config.rs (1)
16-17: Align the raw config helper with the fallible loader.Right now
load_app_config()andload_config()treat the same.envload differently: one returns an error, the other does not. Keeping both on a fallibleResultpath would make config initialization behavior consistent and easier to reason about.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/hello-nestforge-websockets/src/app_config.rs` around lines 16 - 17, load_config() currently returns ConfigService directly while load_app_config() is fallible; change load_config() to return a Result (e.g., Result<ConfigService, E>) and propagate any error from the ConfigModule::for_root_with_options(ConfigOptions::new().env_file(".env")) call instead of unwrapping/returning a plain ConfigService so both loaders use the same fallible initialization path; update the signature of load_config() and call sites to handle the Result accordingly.examples/hello-nestforge-graphql/src/app_config.rs (1)
16-17: Alignload_config()with the new fallible loader.
load_app_config()now propagates config-file errors, butload_config()still uses the infallible constructor. That leaves the typedAppConfigand rawConfigServicepaths with different failure semantics for the same.envsource. Prefer returninganyhow::Result<ConfigService>here as well and wiring the provider asload_config()?.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/hello-nestforge-graphql/src/app_config.rs` around lines 16 - 17, Change load_config to return anyhow::Result<ConfigService> (instead of plain ConfigService) and call the fallible loader so errors are propagated (use the same fallible path as load_app_config). Replace the current infallible ConfigModule::for_root_with_options(ConfigOptions::new().env_file(".env")) usage with the fallible constructor/return value and propagate errors with ? so callers can use load_config()?; update any provider wiring to call load_config()? to propagate errors.examples/hello-nestforge-grpc/src/app_config.rs (1)
16-17: Keepload_config()on the fallible path too.
load_app_config()now propagates config-file errors, butload_config()still goes through the infallible constructor. That leaves the typedAppConfigand rawConfigServicepaths with different failure semantics for the same.envsource. Prefer returninganyhow::Result<ConfigService>here as well and wiring the provider asload_config()?.🤖 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 16 - 17, Change load_config to return anyhow::Result<ConfigService> (instead of infallible ConfigService) and propagate errors from the ConfigModule constructor; replace the direct call to ConfigModule::for_root_with_options(ConfigOptions::new().env_file(".env")) with a fallible return (use the Result returned by that constructor and the ? operator or map_err to convert to anyhow) so callers can call load_config()?; update any provider wiring to use load_config()? instead of the infallible variant.crates/nestforge-config/src/lib.rs (3)
292-292: Move import to top of file with other imports.The
use std::sync::Arc;statement is placed in the middle of the file (line 292) rather than with the other imports at lines 1-4.use std::collections::HashMap; use std::env; use std::path::Path; +use std::sync::Arc; use thiserror::Error;Then remove line 292.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/nestforge-config/src/lib.rs` at line 292, The `use std::sync::Arc;` import is misplaced mid-file (referencing the Arc type); move that single line into the existing import block at the top with the other `use` statements so all imports are grouped, and then remove the duplicate/misplaced `use std::sync::Arc;` from line 292; ensure any code referencing Arc continues to compile after the consolidated import.
306-361: Redundant delegation methods -Derefalready provides access.
Config<T>implementsDeref<Target = ConfigService>(lines 364-370), which means allConfigServicemethods are already accessible through deref coercion. The explicit delegation methods (get,get_string,get_i32, etc.) at lines 306-361 are redundant.Consider removing these delegating methods to reduce code duplication, or if you want to keep them for explicit API documentation, add a comment explaining they exist for discoverability.
♻️ Remove redundant delegations
impl<T> Config<T> { pub fn new(service: ConfigService) -> Self { Self { _phantom: std::marker::PhantomData, service: Arc::new(service), } } - - pub fn get(&self, key: &str) -> Option<&str> { - self.service.get(key) - } - - pub fn get_string(&self, key: &str) -> String { - self.service.get_string(key) - } - - // ... remove all other delegating methods ... - - pub fn has(&self, key: &str) -> bool { - self.service.has(key) - } }🤖 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 306 - 361, The public forwarding methods (get, get_string, get_string_or, get_i32, get_i32_or, get_u16, get_u16_or, get_u32, get_u32_or, get_bool, get_bool_or, get_usize, get_usize_or, has) on Config<T> are redundant because Config<T> already implements Deref<Target = ConfigService>, so callers get direct access via deref coercion; remove these delegating methods from Config<T> to eliminate duplication, or if you intentionally want them for discoverability, keep them but replace their bodies with a one-line doc comment explaining they only exist for API discoverability and forward to ConfigService methods (referencing Config<T>, Deref, and ConfigService to locate the implementation).
23-24:ParseErrorvariant has unclear semantics.The
ParseErrorvariant storeskeyandvalue, but the error message"Failed to parse config key '{key}': {value}"uses the raw value as the error detail, which may be confusing. The message reads as ifvalueis the error description rather than the unparseable value.Consider rewording for clarity:
- #[error("Failed to parse config key `{key}`: {value}")] + #[error("Failed to parse config key `{key}` with value `{value}`")] ParseError { key: String, value: String },🤖 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 23 - 24, The ParseError variant stores the config key and the raw unparseable value but its error message ("Failed to parse config key `{key}`: {value}") makes {value} look like an error description; update the #[error(...)] string on the ParseError variant in lib.rs (the ParseError { key: String, value: String } variant) to clearly indicate that {value} is the offending value (for example: "Failed to parse config key `{key}` from value `{value}`" or "Failed to parse config key `{key}`: value=`{value}`"), or alternatively rename the field to raw_value/unparsed to make intent explicit—apply the change to the ParseError variant definition and its error annotation.docs/cli-guide.md (1)
54-62: Clarify howdevmode differs fromstart.The documentation states
nestforge devis "optimized for development workflow" but doesn't explain what that optimization entails. Consider documenting the specific differences (e.g., watch mode, incremental builds, debug symbols, hot reload).### nestforge dev Runs your application in development mode: ```bash nestforge dev-Same as
startbut optimized for development workflow.
+Same asstartbut optimized for development workflow:
+- [Document specific differences here, e.g., watch mode, debug builds, etc.]<details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/cli-guide.mdaround lines 54 - 62, Update the docs for the CLI commands
by expanding the "nestforge dev" description to explicitly list how it differs
from "start": mention that "nestforge dev" enables watch mode/hot-reload,
incremental or faster builds, debug/dev build flags (e.g., source maps or debug
symbols), different environment variables or config (dev vs prod), and any extra
CLI flags or behaviors used (e.g., polling, polling-interval, auto-restart).
Reference the command names "nestforge dev" and "start" in the prose, add
example invocation(s) and any relevant flags/options to illustrate the
differences, and keep the explanation concise and bullet-style for quick
scanning.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@Cargo.toml:
- Line 48: Update validator attribute syntax to be compatible with validator
0.19: in the test files crates/nestforge/tests/platform_validation_and_query.rs
and crates/nestforge/tests/openapi_module_docs.rs replace all
#[validate(min_length = N, max_length = M)] with #[validate(length(min = N, max
= M))] and replace numeric field validators #[validate(min = N, max = M)] with
#[validate(range(min = N, max = M))]; also ensure Cargo.toml already pins
validator = "0.19" with derive feature and verify the project MSRV is bumped to
Rust 1.81 or later to satisfy validator 0.19 requirements.In
@crates/nestforge-cli/src/transpiler.rs:
- Around line 36-39: The code currently unconditionally calls
fs::remove_dir_all(&self.cache_dir) which can delete arbitrary paths; before
removing, canonicalize self.cache_dir (std::fs::canonicalize) and validate it is
safe: reject removal if the canonical path is root ("/"), the user home, "." or
"..", or otherwise not a descendant of an expected base/project directory (e.g.,
compare with a known project_dir using Path::starts_with), and return an error
instead of deleting; only call fs::remove_dir_all(&self.cache_dir) and then
fs::create_dir_all(&self.cache_dir) after the safety checks succeed. Ensure you
reference self.cache_dir, fs::remove_dir_all and fs::create_dir_all in the
changes.In
@crates/nestforge-config/src/lib.rs:
- Around line 14-20: The ParseEnvFile error variant currently carries a
misleading line field always set to 0; either remove the line field and change
the #[error(...)] message to omit "at line {line}" and update places
constructing ParseEnvFile (where dotenvy::from_path_iter is used) to only supply
path and source, or implement manual line tracking by iterating the iterator
with .enumerate() around dotenvy::from_path_iter so you can capture the exact
line index and pass that into ParseEnvFile (and keep the existing
#[error(...)]); update the ParseEnvFile definition, its #[error(...)] template,
and all sites that construct ParseEnvFile accordingly (look for ParseEnvFile and
uses of dotenvy::from_path_iter).In
@crates/nestforge-core/src/config.rs:
- Around line 17-29: The test currently unconditionally removes the process env
var "APP_NAME", which can break or race with other tests; update the test around
register_config / ConfigService::get to save the previous value of
std::env::var("APP_NAME") (or detect it was absent), set the test value, and
restore the original state in a drop/guard (RAII) or finally block so the
original value is re-established after the test; ensure you reference the
existing usages of std::env::set_var/std::env::remove_var, Container::new,
ConfigOptions::new, register_config, and ConfigService::get when adding the
guard and avoid leaving the env mutated to prevent CI flakes.In
@docs/example-app.md:
- Around line 36-46: The Rust example output is missing the Module symbol from
the imported symbols list; update the Rust snippet that shows the transpiled
import (the use statement referencing nestforge::common::{...}) to include
Module so it reads use nestforge::common::{Module, Controller, Get}; and ensure
the following line still references self::users_service::UsersService for
consistency with the TypeScript import of Module, Controller, Get and
UsersService.In
@examples/hello-nestforge/src/app_config.rs:
- Around line 25-26: The helper load_config currently panics by calling unwrap
on ConfigService::load_with_options; change load_config to be fallible (e.g.,
pub fn load_config() -> Result<ConfigService, _> or
anyhow::Result) and remove the unwrap—propagate the error from
ConfigService::load_with_options(&ConfigOptions::new().env_file(".env")) using
?; then update the module provider registration in app_module to call
load_config()? so the error is propagated through the existing load_app_config
error handling. Ensure types/imports for the chosen Result/Error type are added
where needed.In
@README.md:
- Around line 538-543: The Documentation section in README.md contains a
duplicated "Wiki" bullet; remove one of the duplicate lines (the repeated "-
Wiki: [https://github.com/vernonthedev/nestforge/wiki]" entry) so the list only
includes a single Wiki link, leaving "Main Documentation" and one "Wiki" entry
intact and preserving the Markdown list formatting.- Around line 23-26: Update the Stable Release notice in README.md to reflect
the bumped crate version: change the displayed version string "1.6.0" to "1.7.0"
in the "[!IMPORTANT] Stable Release" section so the release note matches the
PR's dependency bumps.- Around line 99-101: The README example imports the macros with PascalCase
(Controller,Get) but the crate exports lowercase macro names; update the
example to importcontrollerandgetinstead ofControllerandGet
(replace references to Controller and Get in the snippet with controller and
get) so the documented code matches the actual exported symbols.
Nitpick comments:
In @.nestforge.toml:
- Around line 1-20: The .nestforge.toml is only documentation but the Transpiler
(struct Transpiler in crates/nestforge-cli/src/transpiler.rs) still uses
hardcoded patterns/mappings (e.g., default_import_pattern /
default_module_pattern and any internal mapping logic), so either implement
config loading or move the docs: Option A — add a Transpiler::from_config /
load_config function that reads .nestforge.toml, deserializes into a
TranspilerConfig (mappings and regex fields), merges with existing defaults, and
replace hardcoded patterns/mappings with the loaded values; Option B — remove
the misleading TOML and move its contents into comments/docs near the Transpiler
implementation so behavior and customization are clear. Also add a trailing
newline to .nestforge.toml (if keeping the file). Ensure you reference and
update the code paths where default_import_pattern/default_module_pattern and
mapping resolution currently live.In
@crates/nestforge-cli/src/transpiler.rs:
- Around line 282-289: Add tests covering multi-level parent imports for
Transpiler::transform_path: create a new test (e.g.,
test_transform_path_multi_parent_import) that constructs a TempDir and
Transpiler like the existing tests, then call
transpiler.transform_path("../../shared/utils", "") and assert it equals
"super::super::shared::utils", and call
transpiler.transform_path("../../../core", "") and assert it equals
"super::super::super::core"; this ensures transform_path correctly converts
multiple "../" segments into repeated "super::" prefixes.- Around line 127-134: The loop currently recompiles a regex for each import
(the import_pattern/Regex::new call) which is slow; instead, first collect the
matched import strings (or the distinctsourcevalues) into a Vec, then
perform a single replacement pass using either a precompiled Regex built from
all sources or iterate that collected list to call replace_all (not compiling
inside the loop) onresult; update the logic aroundimport_pattern,
Regex::new, andresultso regex construction happens once outside the loop
(or use the already-captured full-match strings for direct removal in one pass).In
@crates/nestforge-config/src/lib.rs:
- Line 292: The
use std::sync::Arc;import is misplaced mid-file (referencing
the Arc type); move that single line into the existing import block at the top
with the otherusestatements so all imports are grouped, and then remove the
duplicate/misplaceduse std::sync::Arc;from line 292; ensure any code
referencing Arc continues to compile after the consolidated import.- Around line 306-361: The public forwarding methods (get, get_string,
get_string_or, get_i32, get_i32_or, get_u16, get_u16_or, get_u32, get_u32_or,
get_bool, get_bool_or, get_usize, get_usize_or, has) on Config are redundant
because Config already implements Deref<Target = ConfigService>, so callers
get direct access via deref coercion; remove these delegating methods from
Config to eliminate duplication, or if you intentionally want them for
discoverability, keep them but replace their bodies with a one-line doc comment
explaining they only exist for API discoverability and forward to ConfigService
methods (referencing Config, Deref, and ConfigService to locate the
implementation).- Around line 23-24: The ParseError variant stores the config key and the raw
unparseable value but its error message ("Failed to parse config key{key}:
{value}") makes {value} look like an error description; update the #[error(...)]
string on the ParseError variant in lib.rs (the ParseError { key: String, value:
String } variant) to clearly indicate that {value} is the offending value (for
example: "Failed to parse config key{key}from value{value}" or "Failed to
parse config key{key}: value={value}"), or alternatively rename the field
to raw_value/unparsed to make intent explicit—apply the change to the ParseError
variant definition and its error annotation.In
@docs/cli-guide.md:
- Around line 54-62: Update the docs for the CLI commands by expanding the
"nestforge dev" description to explicitly list how it differs from "start":
mention that "nestforge dev" enables watch mode/hot-reload, incremental or
faster builds, debug/dev build flags (e.g., source maps or debug symbols),
different environment variables or config (dev vs prod), and any extra CLI flags
or behaviors used (e.g., polling, polling-interval, auto-restart). Reference the
command names "nestforge dev" and "start" in the prose, add example
invocation(s) and any relevant flags/options to illustrate the differences, and
keep the explanation concise and bullet-style for quick scanning.In
@docs/core-concepts.md:
- Around line 207-240: Add a one-line warning under the "TypeScript-Style
Imports" section noting that TypeScript-style imports only work when the code is
run through the NestForge CLI transpilation flow (e.g.,nestforge startor
nestforge dev) and will not be recognized by plaincargobuilds; reference
the section title "TypeScript-Style Imports" and the commandsnestforge start/nestforge devso reviewers can find where to insert the sentence.In
@examples/hello-nestforge-graphql/src/app_config.rs:
- Around line 16-17: Change load_config to return anyhow::Result
(instead of plain ConfigService) and call the fallible loader so errors are
propagated (use the same fallible path as load_app_config). Replace the current
infallible
ConfigModule::for_root_with_options(ConfigOptions::new().env_file(".env")) usage
with the fallible constructor/return value and propagate errors with ? so
callers can use load_config()?; update any provider wiring to call
load_config()? to propagate errors.In
@examples/hello-nestforge-grpc/src/app_config.rs:
- Around line 16-17: Change load_config to return anyhow::Result
(instead of infallible ConfigService) and propagate errors from the ConfigModule
constructor; replace the direct call to
ConfigModule::for_root_with_options(ConfigOptions::new().env_file(".env")) with
a fallible return (use the Result returned by that constructor and the ?
operator or map_err to convert to anyhow) so callers can call load_config()?;
update any provider wiring to use load_config()? instead of the infallible
variant.In
@examples/hello-nestforge-microservices/src/app_config.rs:
- Around line 16-17: Change load_config to return anyhow::Result
and propagate any errors from
ConfigModule::for_root_with_options(ConfigOptions::new().env_file(".env"))
instead of returning ConfigService directly; update the function signature
(load_config) and replace the direct return with the call wrapped to return a
Result so failures surface like load_app_config().In
@examples/hello-nestforge-websockets/src/app_config.rs:
- Around line 16-17: load_config() currently returns ConfigService directly
while load_app_config() is fallible; change load_config() to return a Result
(e.g., Result<ConfigService, E>) and propagate any error from the
ConfigModule::for_root_with_options(ConfigOptions::new().env_file(".env")) call
instead of unwrapping/returning a plain ConfigService so both loaders use the
same fallible initialization path; update the signature of load_config() and
call sites to handle the Result accordingly.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `b07c2787-f207-45b0-ba2f-e49e216e2a4d` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 50cdb2171ad5b38685af45ba6b466233e0e22979 and 12e8828aeb5d9549185dea0261f526410caf4bdb. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `Cargo.lock` is excluded by `!**/*.lock` </details> <details> <summary>📒 Files selected for processing (24)</summary> * `.gitignore` * `.nestforge.toml` * `Cargo.toml` * `LICENSE` * `README.md` * `crates/nestforge-cli/Cargo.toml` * `crates/nestforge-cli/src/cli.rs` * `crates/nestforge-cli/src/main.rs` * `crates/nestforge-cli/src/transpiler.rs` * `crates/nestforge-config/src/lib.rs` * `crates/nestforge-core/src/config.rs` * `crates/nestforge-macros/src/lib.rs` * `crates/nestforge/src/common.rs` * `crates/nestforge/src/lib.rs` * `docs/cli-guide.md` * `docs/core-concepts.md` * `docs/example-app.md` * `docs/quick-start.md` * `examples/hello-nestforge-graphql/src/app_config.rs` * `examples/hello-nestforge-grpc/src/app_config.rs` * `examples/hello-nestforge-microservices/src/app_config.rs` * `examples/hello-nestforge-websockets/src/app_config.rs` * `examples/hello-nestforge/src/app_config.rs` * `examples/hello-nestforge/src/app_module.rs` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| anyhow = "1" | ||
| thiserror = "2" | ||
| validator = { version = "0.16", features = ["derive"] } | ||
| validator = { version = "0.19", features = ["derive"] } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if validator 0.19 exists and review any usage patterns in the codebase
echo "=== Checking validator usage in codebase ==="
rg -n '#\[validate' --type rust -C2
echo ""
echo "=== Checking for derive(Validate) usage ==="
rg -n 'derive.*Validate' --type rustRepository: vernonthedev/nestforge
Length of output: 4085
🌐 Web query:
What are the breaking changes between validator crate version 0.16 and 0.19 in Rust?
💡 Result:
The validator crate underwent several breaking changes between versions 0.16 and 0.19: - 0.17.0: The derive macro was entirely rewritten. Validation is now performed through traits that users can implement themselves. This fundamentally changes how the derive macro generates validation code. - 0.18.0: Removed the require_nested validator. Nested validation now uses required and nested validators instead. The nested validator is now always required on fields for nested validation to occur. Some features from 0.16 (e.g., custom functions) were restored after regressions in 0.17. New syntax required for custom function validation. Changed behavior for Option fields: validation now executes on the contained type if Some. - 0.19.0: Switched to proc-macro-error-2 from proc-macro-error for Syn, which may affect derive macro error reporting. Bumped MSRV to Rust 1.81 due to error naming changes. Added more ValidateRegex impls (likely non-breaking). These changes primarily affect users of the derive macro for struct validation, requiring updates to attributes like #[validate(nested)] instead of plain #[validate] for nested structs, adjustments to custom validators, and potential Rust version upgrades. Check the official CHANGELOG for migration details: https://github.com/Keats/validator/blob/master/CHANGELOG.md
Citations:
- 1: https://github.com/Keats/validator/blob/master/CHANGELOG.md
- 2: Bump validator from 0.16.1 to 0.17.0 qdrant/qdrant#3761
- 3: Update Validator 0.18 qdrant/qdrant#4894
🏁 Script executed:
#!/bin/bash
# Check for nested validation patterns that might be affected by 0.18/0.19 changes
echo "=== Checking for nested validation usage ==="
rg -n 'nested' --type rust -C2 | head -100
echo ""
echo "=== Checking for require_nested (removed in 0.18) ==="
rg 'require_nested' --type rust
echo ""
echo "=== Checking for rust-toolchain or MSRV in Cargo.toml ==="
if [ -f rust-toolchain.toml ]; then
cat rust-toolchain.toml
fi
# Check Cargo.toml for rust-version field
echo "=== Checking Cargo.toml for rust-version ==="
rg 'rust-version' --type toml
echo ""
echo "=== Checking what Rust version is specified in main Cargo.toml ==="
head -20 Cargo.tomlRepository: vernonthedev/nestforge
Length of output: 4308
🌐 Web query:
validator crate 0.19 validate attribute syntax - what validators are still supported and what is the current API in 0.19?
💡 Result:
In validator crate version 0.19.0, the validate attribute syntax uses the #[validate(validator_name(args))] format on fields of structs derived with #[derive(Validate)]. All validators from prior versions (like 0.18) remain supported, with no removals noted in the 0.19 changelog (just proc-macro updates and MSRV bump). Supported validators include: - email: #[validate(email)] - HTML5 email regex. - url: #[validate(url)] - Valid URL. - length(min=.., max=.., equal=..): #[validate(length(min = 5))] - For String/Vec lengths. - range(min=.., max=.., exclusive_min=.., exclusive_max=..): #[validate(range(min = 1))] - Numeric ranges. - contains(pattern = ".."): #[validate(contains(pattern = "foo"))] - String contains substring. - does_not_contain(pattern = ".."): #[validate(does_not_contain(pattern = "foo"))] - regex(path = "regex::Regex"): #[validate(regex(path = "RE"))] - custom(function = "path::to::fn"): #[validate(custom(function = "validate_fn"))] - credit_card: #[validate(credit_card)] - Requires "card" feature. - non_control_character: #[validate(non_control_character)] - Requires "unic" feature. - required: #[validate(required)] - For Option, ensures Some. - nested: #[validate(nested)] - For nested Validate structs/Vecs. - must_match: Field-level cross-validation via validate_must_match function. Struct-level: #[validate(custom(function = "schema_fn"))] or schema. The API uses the Validate trait: impl Validate for YourStruct, call .validate -> Result<, ValidationErrors>. Enable "derive" feature for macros. MSRV is Rust 1.70+.
Citations:
- 1: https://crates.io/crates/validator/0.19.0
- 2: https://docs.rs/validator
- 3: https://docs.rs/validator/0.18.1/validator/
- 4: https://github.com/Keats/validator/blob/master/CHANGELOG.md
🏁 Script executed:
#!/bin/bash
# Check the official validator crate documentation for attribute syntax
echo "=== Checking validator 0.19 validator attributes ==="
# Try to find if there's any documentation or usage example in the repo for the new syntax
rg 'validate\(length' --type rust
rg 'validate\(range' --type rust
echo ""
echo "=== Current validators being used ==="
rg 'validate\((min_length|max_length|min|max)' --type rust -oRepository: vernonthedev/nestforge
Length of output: 368
🌐 Web query:
validator crate 0.19 changelog breaking changes min_length max_length min max attribute syntax
💡 Result:
In the Rust validator crate version 0.19.0, there are no breaking changes specifically related to min_length, max_length, min, or max attribute syntax in the changelog. The release (2024-11-03) primarily involves swapping to proc-macro-error-2, bumping MSRV to 1.81, and adding ValidateRegex impls. The attribute syntax uses two main validators: - length (for strings/Vecs): #[validate(length(min = N, max = M))] or combinations like min only, max only, or equal = N. At least one of min/max/equal required; min+max allowed; equal excludes min/max. - range (for numbers): #[validate(range(min = N, max = M))] or exclusive_min/exclusive_max. 1-2 arguments. No separate min_length/max_length attributes exist; use length(min=..., max=...). Examples from docs: #[validate(length(min = 1, max = 10))] #[validate(length(min = 1))] #[validate(range(min = 18, max = 20))] #[validate(range(exclusive_min = 0.0, max = 100.0))] This syntax is consistent pre/post 0.17 derive rewrite (major changes in 0.17/0.18). No evidence of breaking syntax changes in 0.19.
Citations:
- 1: https://github.com/Keats/validator/blob/master/CHANGELOG.md
- 2: https://crates.io/crates/validator/0.19.0
- 3: https://docs.rs/validator/0.18.1/validator/
- 4: https://crates.io/crates/validator
- 5: https://lib.rs/crates/validator_derive
Update validator attribute syntax for 0.19 compatibility.
The validator crate 0.19 requires updated attribute syntax. The min_length and max_length validators no longer exist separately and must be migrated to length(min=..., max=...). Similarly, numeric range validation must use the range(...) syntax instead of bare min and max attributes.
Required changes:
#[validate(min_length = N, max_length = M)]→#[validate(length(min = N, max = M))]#[validate(min = N, max = M)](on numeric fields) →#[validate(range(min = N, max = M))]
This affects at least:
crates/nestforge/tests/platform_validation_and_query.rscrates/nestforge/tests/openapi_module_docs.rs
Additionally, validator 0.19 requires Rust 1.81 or later (MSRV bump). Verify this is compatible with your project's Rust version requirements.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Cargo.toml` at line 48, Update validator attribute syntax to be compatible
with validator 0.19: in the test files
crates/nestforge/tests/platform_validation_and_query.rs and
crates/nestforge/tests/openapi_module_docs.rs replace all #[validate(min_length
= N, max_length = M)] with #[validate(length(min = N, max = M))] and replace
numeric field validators #[validate(min = N, max = M)] with #[validate(range(min
= N, max = M))]; also ensure Cargo.toml already pins validator = "0.19" with
derive feature and verify the project MSRV is bumped to Rust 1.81 or later to
satisfy validator 0.19 requirements.
Comment on lines
+36
to
+39
| if self.cache_dir.exists() { | ||
| fs::remove_dir_all(&self.cache_dir)?; | ||
| } | ||
| fs::create_dir_all(&self.cache_dir)?; |
There was a problem hiding this comment.
Cache directory deletion lacks safeguards against misconfiguration.
The code unconditionally deletes cache_dir if it exists. If a user accidentally configures cache_dir to point to an important directory (e.g., passing "." or /), this could cause data loss.
🛡️ Proposed safeguard
if self.cache_dir.exists() {
+ // Sanity check: only delete if it looks like a nestforge cache directory
+ let cache_name = self.cache_dir.file_name().and_then(|n| n.to_str());
+ if cache_name != Some("cache") && !self.cache_dir.join(".nestforge-cache").exists() {
+ bail!(
+ "Refusing to delete cache directory {:?} - doesn't appear to be a NestForge cache",
+ self.cache_dir
+ );
+ }
fs::remove_dir_all(&self.cache_dir)?;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/nestforge-cli/src/transpiler.rs` around lines 36 - 39, The code
currently unconditionally calls fs::remove_dir_all(&self.cache_dir) which can
delete arbitrary paths; before removing, canonicalize self.cache_dir
(std::fs::canonicalize) and validate it is safe: reject removal if the canonical
path is root ("/"), the user home, "." or "..", or otherwise not a descendant of
an expected base/project directory (e.g., compare with a known project_dir using
Path::starts_with), and return an error instead of deleting; only call
fs::remove_dir_all(&self.cache_dir) and then fs::create_dir_all(&self.cache_dir)
after the safety checks succeed. Ensure you reference self.cache_dir,
fs::remove_dir_all and fs::create_dir_all in the changes.
Comment on lines
+14
to
+20
| #[error("Failed to parse env file `{path}` at line {line}: {source}")] | ||
| ParseEnvFile { | ||
| path: String, | ||
| line: usize, | ||
| #[source] | ||
| source: dotenvy::Error, | ||
| }, |
There was a problem hiding this comment.
line field is always hardcoded to 0, making the error message misleading.
The ParseEnvFile error variant includes a line field, and the error message promises to show "at line {line}". However, at line 116, line: 0 is hardcoded because dotenvy::from_path_iter doesn't easily expose line numbers.
This produces misleading error messages like "Failed to parse env file '.env' at line 0: ...". Consider either:
- Removing the
linefield if line numbers aren't available - Tracking line numbers manually during iteration
Option 1: Remove the line field
- #[error("Failed to parse env file `{path}` at line {line}: {source}")]
+ #[error("Failed to parse env file `{path}`: {source}")]
ParseEnvFile {
path: String,
- line: usize,
#[source]
source: dotenvy::Error,
},And update the usage:
- let (key, value) = result.map_err(|source| ConfigError::ParseEnvFile {
- path: path_ref.display().to_string(),
- line: 0,
- source,
- })?;
+ let (key, value) = result.map_err(|source| ConfigError::ParseEnvFile {
+ path: path_ref.display().to_string(),
+ source,
+ })?;Option 2: Track line numbers manually
- for result in iter {
+ for (line_num, result) in iter.enumerate() {
let (key, value) = result.map_err(|source| ConfigError::ParseEnvFile {
path: path_ref.display().to_string(),
- line: 0,
+ line: line_num + 1,
source,
})?;Also applies to: 113-120
🤖 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 - 20, The ParseEnvFile
error variant currently carries a misleading line field always set to 0; either
remove the line field and change the #[error(...)] message to omit "at line
{line}" and update places constructing ParseEnvFile (where
dotenvy::from_path_iter is used) to only supply path and source, or implement
manual line tracking by iterating the iterator with .enumerate() around
dotenvy::from_path_iter so you can capture the exact line index and pass that
into ParseEnvFile (and keep the existing #[error(...)]); update the ParseEnvFile
definition, its #[error(...)] template, and all sites that construct
ParseEnvFile accordingly (look for ParseEnvFile and uses of
dotenvy::from_path_iter).
Comment on lines
+17
to
+29
| 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.
Restore the previous APP_NAME value instead of always removing it.
This test mutates a process-wide env var, so it can race with other tests and it also destroys any pre-existing APP_NAME from the caller's environment. Capture and restore the old value in a guard, and serialize env-mutating tests to avoid CI flakes.
🤖 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 17 - 29, The test currently
unconditionally removes the process env var "APP_NAME", which can break or race
with other tests; update the test around register_config / ConfigService::get to
save the previous value of std::env::var("APP_NAME") (or detect it was absent),
set the test value, and restore the original state in a drop/guard (RAII) or
finally block so the original value is re-established after the test; ensure you
reference the existing usages of std::env::set_var/std::env::remove_var,
Container::new, ConfigOptions::new, register_config, and ConfigService::get when
adding the guard and avoid leaving the env mutated to prevent CI flakes.
Comment on lines
+36
to
+46
| ```typescript | ||
| import { Module, Controller, Get } from "nestforge/common"; | ||
| import { UsersService } from "./users.service"; | ||
| ``` | ||
|
|
||
| Which get transpiled to Rust: | ||
|
|
||
| ```rust | ||
| use nestforge::common::{Controller, Get}; | ||
| use self::users_service::UsersService; | ||
| ``` |
There was a problem hiding this comment.
TypeScript import example is inconsistent with Rust output.
Line 37 imports { Module, Controller, Get } but line 44's Rust output only shows { Controller, Get }, missing Module.
📝 Fix the Rust output to match
```rust
-use nestforge::common::{Controller, Get};
+use nestforge::common::{Module, Controller, Get};
use self::users_service::UsersService;</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/example-app.md` around lines 36 - 46, The Rust example output is missing
the Module symbol from the imported symbols list; update the Rust snippet that
shows the transpiled import (the use statement referencing
nestforge::common::{...}) to include Module so it reads use
nestforge::common::{Module, Controller, Get}; and ensure the following line
still references self::users_service::UsersService for consistency with the
TypeScript import of Module, Controller, Get and UsersService.
Comment on lines
+25
to
+26
| pub fn load_config() -> ConfigService { | ||
| ConfigService::load_with_options(&ConfigOptions::new().env_file(".env")).unwrap() |
There was a problem hiding this comment.
Don't unwrap() config loading during startup.
load_app_config() already returns a normal initialization error, but load_config() hard-panics on the same .env failure path. That gives AppConfig and ConfigService different startup behavior and can abort the process before the framework can report the error cleanly. Make this helper fallible too, then register it as load_config()? in examples/hello-nestforge/src/app_module.rs.
🛠️ Suggested change
-pub fn load_config() -> ConfigService {
- ConfigService::load_with_options(&ConfigOptions::new().env_file(".env")).unwrap()
+pub fn load_config() -> anyhow::Result<ConfigService> {
+ ConfigService::load_with_options(&ConfigOptions::new().env_file(".env")).map_err(Into::into)
}Then update the module provider entry to load_config()?.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pub fn load_config() -> ConfigService { | |
| ConfigService::load_with_options(&ConfigOptions::new().env_file(".env")).unwrap() | |
| pub fn load_config() -> anyhow::Result<ConfigService> { | |
| ConfigService::load_with_options(&ConfigOptions::new().env_file(".env")).map_err(Into::into) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/hello-nestforge/src/app_config.rs` around lines 25 - 26, The helper
load_config currently panics by calling unwrap on
ConfigService::load_with_options; change load_config to be fallible (e.g., pub
fn load_config() -> Result<ConfigService, _> or anyhow::Result<ConfigService>)
and remove the unwrap—propagate the error from
ConfigService::load_with_options(&ConfigOptions::new().env_file(".env")) using
?; then update the module provider registration in app_module to call
load_config()? so the error is propagated through the existing load_app_config
error handling. Ensure types/imports for the chosen Result/Error type are added
where needed.
Comment on lines
+23
to
+26
| > [!IMPORTANT] | ||
| > **Stable Release** | ||
| > NestForge **1.6.0** is now published on crates.io. | ||
|
|
There was a problem hiding this comment.
Version number may be outdated.
The stable release notice mentions version 1.6.0, but the PR objective states all internal crate dependencies are being bumped from 1.6.0 to 1.7.0. Consider updating this to reflect the new version being released.
> [!IMPORTANT]
> **Stable Release**
-> NestForge **1.6.0** is now published on crates.io.
+> NestForge **1.7.0** is now published on crates.io.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| > [!IMPORTANT] | |
| > **Stable Release** | |
| > NestForge **1.6.0** is now published on crates.io. | |
| > [!IMPORTANT] | |
| > **Stable Release** | |
| > NestForge **1.7.0** is now published on crates.io. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 23 - 26, Update the Stable Release notice in
README.md to reflect the bumped crate version: change the displayed version
string "1.6.0" to "1.7.0" in the "[!IMPORTANT] **Stable Release**" section so
the release note matches the PR's dependency bumps.
Comment on lines
+99
to
+101
| ```rust | ||
| use nestforge::common::{Controller, Get}; | ||
| use self::users_service::UsersService; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check what's actually exported from common.rs
cat crates/nestforge/src/common.rs
# Search for any PascalCase re-exports of these macros
rg -n "pub use.*Controller|pub use.*Get" crates/nestforge/Repository: vernonthedev/nestforge
Length of output: 937
Update the README.md example to use lowercase macro names.
The documented imports use Controller and Get (PascalCase), but nestforge::common actually exports the macros as controller and get (lowercase). The example should be:
Corrected import
use nestforge::common::{controller, get};
use self::users_service::UsersService;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 99 - 101, The README example imports the macros with
PascalCase (`Controller`, `Get`) but the crate exports lowercase macro names;
update the example to import `controller` and `get` instead of `Controller` and
`Get` (replace references to Controller and Get in the snippet with controller
and get) so the documented code matches the actual exported symbols.
Comment on lines
+538
to
+543
| ## Documentation | ||
|
|
||
| - Main Documentation: [https://nestforge.suredoc.net](https://nestforge.suredoc.net) | ||
| - Wiki: [https://github.com/vernonthedev/nestforge/wiki](https://github.com/vernonthedev/nestforge/wiki) | ||
|
|
||
| - Wiki: [https://github.com/vernonthedev/nestforge/wiki](https://github.com/vernonthedev/nestforge/wiki) |
There was a problem hiding this comment.
Remove duplicate Wiki entry.
The Wiki link appears twice in the Documentation section.
## Documentation
- Main Documentation: [https://nestforge.suredoc.net](https://nestforge.suredoc.net)
- Wiki: [https://github.com/vernonthedev/nestforge/wiki](https://github.com/vernonthedev/nestforge/wiki)
-
-- Wiki: [https://github.com/vernonthedev/nestforge/wiki](https://github.com/vernonthedev/nestforge/wiki)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ## Documentation | |
| - Main Documentation: [https://nestforge.suredoc.net](https://nestforge.suredoc.net) | |
| - Wiki: [https://github.com/vernonthedev/nestforge/wiki](https://github.com/vernonthedev/nestforge/wiki) | |
| - Wiki: [https://github.com/vernonthedev/nestforge/wiki](https://github.com/vernonthedev/nestforge/wiki) | |
| ## Documentation | |
| - Main Documentation: [https://nestforge.suredoc.net](https://nestforge.suredoc.net) | |
| - Wiki: [https://github.com/vernonthedev/nestforge/wiki](https://github.com/vernonthedev/nestforge/wiki) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 538 - 543, The Documentation section in README.md
contains a duplicated "Wiki" bullet; remove one of the duplicate lines (the
repeated "- Wiki: [https://github.com/vernonthedev/nestforge/wiki]" entry) so
the list only includes a single Wiki link, leaving "Main Documentation" and one
"Wiki" entry intact and preserving the Markdown list formatting.
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+495
to
+514
| let source_dir = app_root.join("src"); | ||
| let cache_dir = app_root.join(".nestforge").join("cache"); | ||
|
|
||
| println!("Transpiling TypeScript-style imports..."); | ||
| transpile_project(&source_dir, &cache_dir)?; | ||
| println!("Transpilation complete. Running application..."); | ||
|
|
||
| let mut cmd = Command::new("cargo"); | ||
| cmd.arg("run"); | ||
| if !args.args.is_empty() { | ||
| cmd.arg("--").args(&args.args); | ||
| } | ||
|
|
||
| let status = cmd.current_dir(&app_root).status() | ||
| .with_context(|| format!("Failed to run application in {}", app_root.display()))?; | ||
|
|
||
| if !status.success() { | ||
| bail!("Application failed to start"); | ||
| } | ||
|
|
There was a problem hiding this comment.
1. start/dev ignores cache build 📎 Requirement gap ✓ Correctness
nestforge start/dev transpiles into .nestforge/cache but then runs cargo run against the original project sources, so files containing import are still compiled unmodified. This breaks the required shadow build flow and will still produce Rust parse errors for TypeScript-style imports.
Agent Prompt
## Issue description
`nestforge start/dev` transpiles sources into `.nestforge/cache` but still runs `cargo run` against the original project, so the transpiled output is not actually compiled.
## Issue Context
Compliance requires a shadow build that compiles the transpiled entry point from `.nestforge/cache`, not the original `src/` tree.
## Fix Focus Areas
- crates/nestforge-cli/src/main.rs[495-548]
- crates/nestforge-cli/src/transpiler.rs[250-254]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+114
to
+176
| let transformed_source = self.transform_path(source, prefix); | ||
| let module_path = transformed_source.replace("::", "/"); | ||
|
|
||
| for symbol in symbols.split(',') { | ||
| let symbol = symbol.trim(); | ||
| if !symbol.is_empty() { | ||
| imports_map | ||
| .entry(module_path.clone()) | ||
| .or_default() | ||
| .push((symbol.to_string(), symbol.to_string())); | ||
| } | ||
| } | ||
|
|
||
| let import_pattern = format!( | ||
| r#"import\s*\{{}}\s*from\s*["']{}["']\s*;"#, | ||
| regex::escape(source) | ||
| ); | ||
| result = Regex::new(&import_pattern)? | ||
| .replace_all(&result, "") | ||
| .to_string(); | ||
| } | ||
|
|
||
| for cap in self.module_import_regex.captures_iter(&content) { | ||
| let symbol = cap.get(1).map(|m| m.as_str()).unwrap_or(""); | ||
| let source = cap.get(2).map(|m| m.as_str()).unwrap_or(""); | ||
|
|
||
| let transformed_source = self.transform_path(source, prefix); | ||
|
|
||
| imports_map | ||
| .entry(transformed_source) | ||
| .or_default() | ||
| .push((symbol.to_string(), symbol.to_string())); | ||
|
|
||
| let import_pattern = format!( | ||
| r#"import\s+{}\s+from\s*["']{}["']\s*;"#, | ||
| symbol, | ||
| regex::escape(source) | ||
| ); | ||
| result = Regex::new(&import_pattern)? | ||
| .replace_all(&result, "") | ||
| .to_string(); | ||
| } | ||
|
|
||
| let mut use_statements: Vec<String> = Vec::new(); | ||
| for (module, symbols) in imports_map { | ||
| if symbols.is_empty() { | ||
| continue; | ||
| } | ||
|
|
||
| let symbol_list: Vec<String> = symbols.iter().map(|(orig, _)| orig.clone()).collect(); | ||
|
|
||
| let use_stmt = if symbol_list.len() == 1 { | ||
| format!("use {}::{};", module, symbol_list[0]) | ||
| } else { | ||
| format!("use {}::{{{}}};", module, symbol_list.join(", ")) | ||
| }; | ||
| use_statements.push(use_stmt); | ||
| } | ||
|
|
||
| if !use_statements.is_empty() { | ||
| let use_block = use_statements.join("\n"); | ||
| result = format!("{}\n\n{}", use_block, result); | ||
| } |
There was a problem hiding this comment.
2. Named import rewrite invalid 📎 Requirement gap ✓ Correctness
The transpiler generates invalid Rust use paths for named imports by converting :: into /, and
it attempts to remove import { ... } from ...; lines with a pattern that doesn't match non-empty
imports. This violates the requirement that transpiled output contain valid Rust use statements
and no remaining import keywords.
Agent Prompt
## Issue description
Named-import transpilation produces invalid Rust and may leave `import` statements in the output.
## Issue Context
The transpiler must output valid Rust `use` statements (with `::` module separators) and remove the original `import ...;` lines entirely.
## Fix Focus Areas
- crates/nestforge-cli/src/transpiler.rs[110-176]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+181
to
+217
| fn transform_path(&self, source: &str, _prefix: &str) -> String { | ||
| if source.starts_with("nestforge/") { | ||
| let module = source.strip_prefix("nestforge/").unwrap_or(source); | ||
| format!("nestforge::{}", module.replace('/', "::")) | ||
| } else if source.starts_with('@') { | ||
| source.replace('@', "nestforge::") | ||
| } else if source.starts_with("./") || source.starts_with("../") { | ||
| let clean_path = source.trim_start_matches("./").trim_start_matches("../"); | ||
| let path_parts: Vec<&str> = clean_path.split('/').collect(); | ||
|
|
||
| let rust_path: String = path_parts | ||
| .iter() | ||
| .map(|p| self.to_snake_case(p)) | ||
| .collect::<Vec<_>>() | ||
| .join("::"); | ||
|
|
||
| if source.starts_with("./") { | ||
| format!("self::{}", rust_path) | ||
| } else { | ||
| let parent_count = source.matches("..").count(); | ||
| let super_prefix = (0..parent_count) | ||
| .map(|_| "super") | ||
| .collect::<Vec<_>>() | ||
| .join("::"); | ||
|
|
||
| if rust_path.is_empty() { | ||
| super_prefix | ||
| } else if super_prefix.is_empty() { | ||
| rust_path | ||
| } else { | ||
| format!("{}::{}", super_prefix, rust_path) | ||
| } | ||
| } | ||
| } else { | ||
| source.replace('-', "_").replace('/', "::") | ||
| } | ||
| } |
There was a problem hiding this comment.
3. transform_path() breaks mappings 📎 Requirement gap ✓ Correctness
The path mapping logic does not correctly implement the documented transformations for @* and multi-level parent imports (e.g., ../../...). This can produce invalid Rust paths and unresolved modules in transpiled output.
Agent Prompt
## Issue description
`transform_path()` does not reliably implement the specified transformation rules for `@*`, `./*`, and repeated `../*` parent traversal.
## Issue Context
Incorrect path mapping will generate invalid `use` paths and cause unresolved imports after transpilation.
## Fix Focus Areas
- crates/nestforge-cli/src/transpiler.rs[181-217]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+79
to
+95
| let stem = path.file_stem().and_then(|n| n.to_str()).unwrap_or(""); | ||
|
|
||
| let output_path = cache_dir.join(format!("{}.rs", stem)); | ||
| fs::write(&output_path, &transpiled)?; | ||
|
|
||
| module_items.push(stem.to_string()); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if !module_items.is_empty() { | ||
| let mod_content = self.generate_mod_file(&module_items); | ||
| let mod_path = cache_dir.join("mod.rs"); | ||
| fs::write(&mod_path, mod_content)?; | ||
| } |
There was a problem hiding this comment.
4. Cache modules not snake_case 📎 Requirement gap ✓ Correctness
The transpiler writes cache filenames and pub mod declarations using the raw file stem (not snake_case), which will generate invalid module identifiers for dot-notated or kebab-case filenames (e.g., users.service.rs). This violates the required filename-to-snake_case conversion for Rust module paths.
Agent Prompt
## Issue description
Cache output paths and generated `pub mod` declarations use raw file stems, which can include dots or kebab-case and will not compile as Rust module identifiers.
## Issue Context
The transpiler is expected to support NestJS-style filenames while emitting Rust snake_case module identifiers.
## Fix Focus Areas
- crates/nestforge-cli/src/transpiler.rs[63-95]
- crates/nestforge-cli/src/transpiler.rs[239-247]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+1
to
+16
| pub use crate::{ | ||
| ConfigError, ConfigModule, ConfigOptions, ConfigService, Container, ContainerError, | ||
| ControllerDefinition, Cookies, Decorated, DynamicModuleBuilder, ExceptionFilter, Guard, | ||
| Headers, HttpException, Identifiable, InMemoryStore, InitializedModule, Inject, Injectable, | ||
| Interceptor, LifecycleHook, List, ModuleDefinition, ModuleGraphEntry, ModuleGraphReport, | ||
| ModuleRef, NestForgeFactory, NextFn, NextFuture, OpenApiSchema, OpenApiSchemaComponent, Param, | ||
| Pipe, PipedBody, PipedParam, PipedQuery, Provider, Query, RequestContext, RequestDecorator, | ||
| RequestId, ResourceError, ResourceService, ResponseEnvelope, ResponseSerializer, RouteBuilder, | ||
| RouteDocumentation, Serialized, Validate, ValidatedBody, | ||
| }; | ||
|
|
||
| pub use nestforge_macros::{controller, delete, get, injectable, module, post, put}; | ||
|
|
||
| pub mod prelude { | ||
| pub use super::*; | ||
| } |
There was a problem hiding this comment.
5. common missing pascalcase aliases 📎 Requirement gap ✓ Correctness
nestforge::common re-exports macros as lowercase names (e.g., controller, get) but does not
provide PascalCase aliases like Controller/Get that the transpiler documentation expects. This
can cause transpiled imports like use nestforge::common::{Controller, Get}; to fail to compile.
Agent Prompt
## Issue description
`nestforge::common` does not export PascalCase aliases (e.g., `Controller`, `Get`) required by the transpiled `use nestforge::common::{...}` imports.
## Issue Context
The transpiler documentation/examples imply PascalCase items under `nestforge::common`.
## Fix Focus Areas
- crates/nestforge/src/common.rs[1-16]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+75
to
+95
| if let Some(ext) = path.extension() { | ||
| if ext == "rs" { | ||
| let transpiled = self.transpile_file(&path, prefix)?; | ||
| if !transpiled.is_empty() { | ||
| let stem = path.file_stem().and_then(|n| n.to_str()).unwrap_or(""); | ||
|
|
||
| let output_path = cache_dir.join(format!("{}.rs", stem)); | ||
| fs::write(&output_path, &transpiled)?; | ||
|
|
||
| module_items.push(stem.to_string()); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if !module_items.is_empty() { | ||
| let mod_content = self.generate_mod_file(&module_items); | ||
| let mod_path = cache_dir.join("mod.rs"); | ||
| fs::write(&mod_path, mod_content)?; | ||
| } |
There was a problem hiding this comment.
6. Generates pub mod mod 🐞 Bug ✓ Correctness
The transpiler processes mod.rs as a normal source file, then also generates a new mod.rs that includes pub mod mod;, which can create self-referential/invalid module declarations and break compilation in directories that contain mod.rs.
Agent Prompt
### Issue description
The transpiler includes `mod.rs` in the module list, producing `pub mod mod;` and overwriting the transpiled `mod.rs`.
### Issue Context
Rust module directories commonly use `mod.rs`.
### Fix Focus Areas
- crates/nestforge-cli/src/transpiler.rs[46-98]
- Skip `mod.rs` (and likely `lib.rs`/`main.rs`) when collecting module items.
- Ensure generated `mod.rs` content is deterministic and de-duplicated.
- Add a test directory fixture with a `mod.rs` to validate output does not contain `pub mod mod;`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+2796
to
+2802
| fn template_app_config_rs(_transport: AppTransport) -> String { | ||
| r#"use nestforge::{ConfigModule, ConfigOptions, ConfigService}; | ||
|
|
||
| pub fn load_config() -> ConfigService { | ||
| ConfigModule::for_root_with_options(ConfigOptions::new().env_file(".env")) | ||
| } | ||
| "# |
There was a problem hiding this comment.
7. Scaffolded apps won't compile 🐞 Bug ✓ Correctness
The CLI app template no longer generates an AppConfig type/struct, but other generated template files still import/resolve AppConfig, so newly scaffolded GraphQL/gRPC/microservices apps will fail to compile.
Agent Prompt
### Issue description
Generated apps reference `AppConfig` but `src/app_config.rs` no longer defines it.
### Issue Context
Templates across transports assume `AppConfig` exists (and is injectable/resolvable).
### Fix Focus Areas
- crates/nestforge-cli/src/main.rs[2796-2804]
- Either reintroduce an `AppConfig` struct + injectable factory, or update *all* templates to use `ConfigService` consistently.
- crates/nestforge-cli/src/main.rs[2463-2472]
- crates/nestforge-cli/src/main.rs[2917-2954]
- crates/nestforge-cli/src/main.rs[2976-2996]
- Ensure GraphQL/gRPC/microservices templates compile with the chosen config approach.
- Add a CLI integration test that scaffolds each transport and runs `cargo check`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
vernonthedev
changed the title
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 fixes the release publishing issue caused by missing version specifications in internal crate dependencies and updates the configuration system to use the new typed
FromEnvtrait.Changes Made
Release Publishing Fixes
version = "1.7.0"tonestforge-configdependency innestforge-core/Cargo.toml1.6.0to1.7.0across the workspaceConfiguration API Updates
app_config.rsto use the newFromEnvtrait andEnvStorefor typed configuration loadingConfigServiceimports that were no longer neededTranspiler Configuration
Technical Details
FromEnvtrait instead of manual gettersnestforge devandnestforge startTesting
Closes #24
Summary by CodeRabbit
Release Notes
New Features
nestforge startandnestforge devCLI commands.Documentation
Chores