Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ members = [
"crates/rustapi-toon",
"crates/rustapi-ws",
"crates/rustapi-view",
"crates/rustapi-ssr",
"crates/rustapi-testing",
"crates/rustapi-jobs",
"crates/cargo-rustapi",
Expand Down Expand Up @@ -108,6 +109,6 @@ rustapi-extras = { path = "crates/rustapi-extras", version = "0.1.14" }
rustapi-toon = { path = "crates/rustapi-toon", version = "0.1.14" }
rustapi-ws = { path = "crates/rustapi-ws", version = "0.1.14" }
rustapi-view = { path = "crates/rustapi-view", version = "0.1.14" }
rustapi-ssr = { path = "crates/rustapi-ssr", version = "0.1.14" }
rustapi-testing = { path = "crates/rustapi-testing", version = "0.1.14" }
rustapi-jobs = { path = "crates/rustapi-jobs", version = "0.1.14" }

25 changes: 25 additions & 0 deletions crates/rustapi-ssr/Cargo.toml
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"

Comment on lines +10 to +15
Copy link

Copilot AI Jan 19, 2026

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.

Suggested change
[lib]
path = "src/lib.rs"
build = "build.rs"
build = "build.rs"
[lib]
path = "src/lib.rs"

Copilot uses AI. Check for mistakes.
[dependencies]
inventory = { workspace = true }
matchit = { workspace = true }
thiserror = { workspace = true }

[features]
default = []

[package.metadata]
rustapi = true
184 changes: 184 additions & 0 deletions crates/rustapi-ssr/build.rs
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
Copy link

Copilot AI Jan 19, 2026

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 is called even when app_dir doesn't exist (line 21). If the directory doesn't exist, the function will print the rerun-if-changed directive for the non-existent directory path, then attempt to read it and fail silently in the loop. While not causing a build failure, this is redundant - the early return on line 22 should come before the emit_rerun_if_changed call on line 21.

Suggested change
emit_rerun_if_changed(&app_dir);
return;
}
return;
}

Copilot uses AI. Check for mistakes.
let mut routes = Vec::new();
collect_routes(&app_dir, &app_dir, &mut routes);

validate_routes(&routes);
write_routes(&dest_path, &routes);
emit_rerun_if_changed(&app_dir);
}

fn resolve_app_dir(manifest_dir: &Path) -> PathBuf {
if let Ok(env_dir) = env::var("RUSTAPI_APP_DIR") {
return PathBuf::from(env_dir);
Comment on lines +34 to +35
Copy link

Copilot AI Jan 19, 2026

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.

Suggested change
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 uses AI. Check for mistakes.
}

let workspace_root = manifest_dir
.parent()
.and_then(|parent| parent.parent())
.unwrap_or(manifest_dir);
let workspace_app = workspace_root.join("app");
if workspace_app.exists() {
return workspace_app;
}

manifest_dir.join("app")
}

fn collect_routes(app_dir: &Path, current: &Path, routes: &mut Vec<RouteEntry>) {
let entries = match fs::read_dir(current) {
Ok(entries) => entries,
Err(_) => return,
};

for entry in entries.flatten() {
let path = entry.path();
if path.is_dir() {
collect_routes(app_dir, &path, routes);
continue;
}

if path.file_name().and_then(|s| s.to_str()) != Some("page.rs") {
continue;
}

let relative = match path.strip_prefix(app_dir) {
Ok(path) => path,
Err(_) => continue,
};

let segments: Vec<String> = relative
.components()
.filter_map(|component| component.as_os_str().to_str().map(|s| s.to_string()))
.collect();

if segments.is_empty() {
continue;
}

let mut route_segments = Vec::new();
for segment in &segments[..segments.len() - 1] {
if let Some(stripped) = segment.strip_prefix('(').and_then(|s| s.strip_suffix(')')) {
if stripped.is_empty() {
panic!("Invalid route group segment '()' in {}", path.display());
}
continue;
}

if let Some(param) = segment.strip_prefix('[').and_then(|s| s.strip_suffix(']')) {
if param.is_empty() {
panic!("Invalid dynamic segment '[]' in {}", path.display());
}
if !param
.chars()
.all(|c| c.is_ascii_alphanumeric() || c == '_')
{
panic!("Invalid dynamic segment '[{}]' in {}", param, path.display());
}
route_segments.push(format!("{{{}}}", param));
continue;
}

if segment.is_empty() {
panic!("Empty path segment in {}", path.display());
}

Copy link

Copilot AI Jan 19, 2026

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.

Suggested change
if !segment
.chars()
.all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_' || c == '~')
{
panic!(
"Invalid static segment '{}' in {}",
segment,
path.display()
);
}

Copilot uses AI. Check for mistakes.
route_segments.push(segment.to_string());
}

let pattern = if route_segments.is_empty() {
"/".to_string()
} else {
format!("/{}", route_segments.join("/"))
};

let normalized = if route_segments.is_empty() {
"/".to_string()
} else {
let mut normalized_segments = Vec::new();
for seg in &route_segments {
if seg.starts_with('{') && seg.ends_with('}') {
normalized_segments.push("{}");
} else {
normalized_segments.push(seg);
}
}
format!("/{}", normalized_segments.join("/"))
};

let source = format!("app/{}", segments.join("/"));
routes.push(RouteEntry {
pattern,
normalized,
source,
});
}
}

fn validate_routes(routes: &[RouteEntry]) {
let mut seen = std::collections::HashMap::new();
for route in routes {
if let Some(existing) = seen.insert(route.normalized.clone(), route) {
panic!(
"Route conflict: '{}' from {} conflicts with {}",
route.pattern, existing.source, route.source
Comment on lines +145 to +146
Copy link

Copilot AI Jan 19, 2026

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.

Suggested change
"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 uses AI. Check for mistakes.
);
}
}
}
Comment on lines +50 to +150
Copy link

Copilot AI Jan 19, 2026

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 uses AI. Check for mistakes.

fn write_routes(dest_path: &Path, routes: &[RouteEntry]) {
let mut output = String::new();
output.push_str("// @generated by rustapi-ssr build script.\n");

for route in routes {
output.push_str(&format!(
"::inventory::submit! {{ ::rustapi_ssr::app_router::PageRoute {{ pattern: \"{}\", source: \"{}\" }} }}\n",
route.pattern, route.source
));
}

fs::write(dest_path, output).expect("failed to write app router routes");
}

fn emit_rerun_if_changed(app_dir: &Path) {
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());
}
}
}
Comment on lines +167 to +183
Copy link

Copilot AI Jan 19, 2026

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).

Suggested change
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());

Copilot uses AI. Check for mistakes.
}
63 changes: 63 additions & 0 deletions crates/rustapi-ssr/src/app_router/mod.rs
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
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The RouteRegistry struct doesn't implement the Default trait, but the new() method takes no arguments and returns a Result. For consistency with Rust conventions, consider either implementing TryDefault (if available) or providing a more explicit constructor name like 'from_inventory' to clarify that it's building from the global inventory.

Copilot uses AI. Check for mistakes.

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
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The match_route method returns Option which loses information about why matching failed. Consider returning Result<MatchedRoute, MatchError> instead, where MatchError could distinguish between "no route found" and other potential matching errors from the underlying matchit router. This would provide better diagnostics for callers.

Copilot uses AI. Check for mistakes.
}

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
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The RouteRegistry and route matching logic lack test coverage. Important behaviors to test include: registry initialization with multiple routes, route matching with various path patterns, parameter extraction from dynamic segments, handling of non-matching paths, and error cases during registry construction. Consider adding tests to verify these behaviors work correctly.

Copilot uses AI. Check for mistakes.

pub fn routes() -> impl Iterator<Item = &'static PageRoute> {
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function name 'routes' is ambiguous - it's unclear whether it returns route patterns, route entries, or PageRoute instances. Consider renaming to 'iter_page_routes' or 'all_routes' to make the purpose clearer and indicate that it returns an iterator over all registered PageRoute instances.

Suggested change
pub fn routes() -> impl Iterator<Item = &'static PageRoute> {
pub fn iter_page_routes() -> impl Iterator<Item = &'static PageRoute> {

Copilot uses AI. Check for mistakes.
inventory::iter::<PageRoute>
}
Comment on lines +6 to +63
Copy link

Copilot AI Jan 19, 2026

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.

Copilot uses AI. Check for mistakes.
1 change: 1 addition & 0 deletions crates/rustapi-ssr/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub mod app_router;
Loading