-
Notifications
You must be signed in to change notification settings - Fork 1
Add SSR app router registry and build-time route scanner #40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| [package] | ||
| name = "rustapi-ssr" | ||
| version = { workspace = true } | ||
| edition = { workspace = true } | ||
| license = { workspace = true } | ||
| repository = { workspace = true } | ||
| homepage = { workspace = true } | ||
| documentation = { workspace = true } | ||
| rust-version = { workspace = true } | ||
|
|
||
| [lib] | ||
| path = "src/lib.rs" | ||
|
|
||
| build = "build.rs" | ||
|
|
||
| [dependencies] | ||
| inventory = { workspace = true } | ||
| matchit = { workspace = true } | ||
| thiserror = { workspace = true } | ||
|
|
||
| [features] | ||
| default = [] | ||
|
|
||
| [package.metadata] | ||
| rustapi = true | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,184 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use std::env; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use std::fs; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use std::path::{Path, PathBuf}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[derive(Debug, Clone)] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| struct RouteEntry { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pattern: String, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| normalized: String, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| source: String, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn main() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let manifest_dir = PathBuf::from(env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let app_dir = resolve_app_dir(&manifest_dir); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let out_dir = PathBuf::from(env::var("OUT_DIR").expect("OUT_DIR")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let dest_path = out_dir.join("app_router_routes.rs"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if !app_dir.exists() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| write_routes(&dest_path, &[]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| emit_rerun_if_changed(&app_dir); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+21
to
+24
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| emit_rerun_if_changed(&app_dir); | |
| return; | |
| } | |
| return; | |
| } |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The RUSTAPI_APP_DIR environment variable is read during build but there's no documentation or validation of its expected format. If a user sets this to an invalid path or uses a relative path, the behavior is undefined. Consider documenting this environment variable and validating that it points to an absolute path or properly resolving relative paths.
| if let Ok(env_dir) = env::var("RUSTAPI_APP_DIR") { | |
| return PathBuf::from(env_dir); | |
| // If set, RUSTAPI_APP_DIR points to the directory containing the app routes. | |
| // It may be an absolute path or a path relative to CARGO_MANIFEST_DIR. | |
| if let Ok(env_dir) = env::var("RUSTAPI_APP_DIR") { | |
| let trimmed = env_dir.trim(); | |
| if trimmed.is_empty() { | |
| panic!( | |
| "RUSTAPI_APP_DIR is set but empty; expected an absolute or relative path to the app directory" | |
| ); | |
| } | |
| let env_path = PathBuf::from(trimmed); | |
| let resolved = if env_path.is_absolute() { | |
| env_path | |
| } else { | |
| // Resolve relative paths against the manifest directory for deterministic behavior. | |
| manifest_dir.join(env_path) | |
| }; | |
| println!( | |
| "cargo:warning=Using RUSTAPI_APP_DIR={} resolved to {}", | |
| trimmed, | |
| resolved.display() | |
| ); | |
| return resolved; |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The collect_routes function doesn't validate or sanitize directory names before using them as route segments. Malicious or unusual directory names containing special characters (beyond the handled brackets and parentheses) could potentially create unexpected route patterns. Consider adding validation to ensure directory names contain only safe characters for URL paths.
| if !segment | |
| .chars() | |
| .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_' || c == '~') | |
| { | |
| panic!( | |
| "Invalid static segment '{}' in {}", | |
| segment, | |
| path.display() | |
| ); | |
| } |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message incorrectly shows 'route.pattern' for the existing route when it should show 'existing.pattern'. This makes the error message confusing as it displays the same pattern twice instead of showing both conflicting patterns.
| "Route conflict: '{}' from {} conflicts with {}", | |
| route.pattern, existing.source, route.source | |
| "Route conflict: '{}' from {} conflicts with '{}' from {}", | |
| existing.pattern, existing.source, route.pattern, route.source |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The route scanning and normalization logic in the build script lacks test coverage. This includes critical behaviors like dynamic segment parsing ([param] -> {param}), route group filtering ((group)), conflict detection, and pattern normalization. Consider adding unit tests for the helper functions (collect_routes, validate_routes, resolve_app_dir) to ensure edge cases are handled correctly.
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The emit_rerun_if_changed function performs a full directory traversal and emits a cargo directive for every file and directory. For large app directories with many files, this could generate an excessive number of rerun directives and slow down incremental builds. Consider emitting only the directory path or using a more targeted approach that watches only relevant files (e.g., page.rs files).
| println!("cargo:rerun-if-changed={}", app_dir.display()); | |
| let mut stack = vec![app_dir.to_path_buf()]; | |
| while let Some(dir) = stack.pop() { | |
| let entries = match fs::read_dir(&dir) { | |
| Ok(entries) => entries, | |
| Err(_) => continue, | |
| }; | |
| for entry in entries.flatten() { | |
| let path = entry.path(); | |
| if path.is_dir() { | |
| stack.push(path); | |
| } else { | |
| println!("cargo:rerun-if-changed={}", path.display()); | |
| } | |
| } | |
| } | |
| // Instruct Cargo to rerun this build script if anything under `app_dir` changes. | |
| // Cargo treats directory paths recursively, so we don't need to walk the tree ourselves. | |
| println!("cargo:rerun-if-changed={}", app_dir.display()); |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,63 @@ | ||||||
| use std::sync::OnceLock; | ||||||
|
|
||||||
| use matchit::Router; | ||||||
| use thiserror::Error; | ||||||
|
|
||||||
| #[derive(Debug, Clone, Copy)] | ||||||
| pub struct PageRoute { | ||||||
| pub pattern: &'static str, | ||||||
| pub source: &'static str, | ||||||
| } | ||||||
|
|
||||||
| inventory::collect!(PageRoute); | ||||||
|
|
||||||
| include!(concat!(env!("OUT_DIR"), "/app_router_routes.rs")); | ||||||
|
|
||||||
| #[derive(Debug, Error)] | ||||||
| pub enum RegistryError { | ||||||
| #[error("route registry build failed: {0}")] | ||||||
| Router(#[from] matchit::InsertError), | ||||||
| } | ||||||
|
|
||||||
| #[derive(Debug)] | ||||||
| pub struct MatchedRoute<'a> { | ||||||
| pub route: &'a PageRoute, | ||||||
| pub params: Vec<(&'a str, String)>, | ||||||
| } | ||||||
|
|
||||||
| #[derive(Debug)] | ||||||
| pub struct RouteRegistry { | ||||||
| router: Router<&'static PageRoute>, | ||||||
| } | ||||||
|
|
||||||
| impl RouteRegistry { | ||||||
| pub fn new() -> Result<Self, RegistryError> { | ||||||
| let mut router = Router::new(); | ||||||
| for route in inventory::iter::<PageRoute> { | ||||||
| router.insert(route.pattern, route)?; | ||||||
| } | ||||||
| Ok(Self { router }) | ||||||
| } | ||||||
|
Comment on lines
+33
to
+40
|
||||||
|
|
||||||
| pub fn match_route(&self, path: &str) -> Option<MatchedRoute<'_>> { | ||||||
| let matched = self.router.at(path).ok()?; | ||||||
| let params = matched | ||||||
| .params | ||||||
| .iter() | ||||||
| .map(|(key, value)| (key, value.to_string())) | ||||||
| .collect(); | ||||||
| Some(MatchedRoute { | ||||||
| route: matched.value, | ||||||
| params, | ||||||
| }) | ||||||
| } | ||||||
|
Comment on lines
+42
to
+53
|
||||||
| } | ||||||
|
|
||||||
| pub fn registry() -> Result<&'static RouteRegistry, RegistryError> { | ||||||
| static REGISTRY: OnceLock<RouteRegistry> = OnceLock::new(); | ||||||
| REGISTRY.get_or_try_init(RouteRegistry::new) | ||||||
| } | ||||||
|
Comment on lines
+33
to
+59
|
||||||
|
|
||||||
| pub fn routes() -> impl Iterator<Item = &'static PageRoute> { | ||||||
|
||||||
| pub fn routes() -> impl Iterator<Item = &'static PageRoute> { | |
| pub fn iter_page_routes() -> impl Iterator<Item = &'static PageRoute> { |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The public API lacks documentation. All public types (PageRoute, MatchedRoute, RouteRegistry, RegistryError) and functions (registry, routes, match_route) should have doc comments explaining their purpose, usage, and any important behavior. This is especially important for a library crate that will be consumed by other code.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| pub mod app_router; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The build script is specified outside the [lib] section, but according to Cargo.toml specification, the 'build' field should be at the package level, not inside the [lib] section. This may cause the build script to be ignored or cause a configuration error.