From 29fca8309af5bf98477afa8f3ae575c9d1ab87f3 Mon Sep 17 00:00:00 2001 From: Mohamed Mansour Date: Thu, 25 Jun 2026 12:33:21 -0700 Subject: [PATCH] fix: discover npm components declared in webui-press config External components declared as npm packages in a webui-press `config.json` `components` array (e.g. `"@mai-ui"`) failed to build with either: Component path not found: /@mai-ui or, once the name reached discovery intact: Component discovery error: Could not find node_modules/ directory (searched upward from /var/folders/.../webui-press-404-...) Two coupled causes: 1. webui-press prefixed every configured component source with the working directory (`cwd.join(source)`), turning the npm specifier `@mai-ui` into an absolute path. `webui-discovery` then classified it as a local filesystem path and failed to find it. 2. npm discovery anchors its `node_modules` walk-up at the build app directory. webui-press renders each docs page from a synthesized scratch directory in the system temp folder, which has no `node_modules` ancestor, so even a correctly-bare `@scope` specifier could not be resolved. Fixes: - webui-discovery: add `find_node_modules_with_fallback`, used by `resolve`, which walks up from the app directory first and then from the process working directory (the project root the command was run in). Normal CLI builds are unaffected because the app directory is already inside the project. - webui-discovery: expose `is_local_source` as the single source of truth for path-vs-npm classification (`classify_source` now delegates to it). - webui-press: resolve configured component sources through `is_local_source` so npm names/scopes stay bare while local paths are still made absolute against the project root. Adds unit tests for the fallback resolver, the shared classifier, and the webui-press config source resolution. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Cargo.lock | 1 + crates/webui-discovery/src/lib.rs | 39 ++++++++++++++++++--- crates/webui-discovery/src/npm.rs | 58 +++++++++++++++++++++++++++++-- crates/webui-press/Cargo.toml | 1 + crates/webui-press/src/build.rs | 51 ++++++++++++++++++++++++--- 5 files changed, 138 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6a5ab4ab..36e07df6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1943,6 +1943,7 @@ dependencies = [ "html-escape", "microsoft-webui", "microsoft-webui-dev-server", + "microsoft-webui-discovery", "microsoft-webui-handler", "microsoft-webui-tokens", "rayon", diff --git a/crates/webui-discovery/src/lib.rs b/crates/webui-discovery/src/lib.rs index 4d87192f..c1b91928 100644 --- a/crates/webui-discovery/src/lib.rs +++ b/crates/webui-discovery/src/lib.rs @@ -52,17 +52,33 @@ enum ComponentSource { /// - Starts with `.`, `/`, `\`, or contains a drive letter (Windows) → path /// - Everything else → npm package fn classify_source(source: &str) -> ComponentSource { - if source.starts_with('.') - || source.starts_with('/') - || source.starts_with('\\') - || (cfg!(windows) && source.len() >= 2 && source.as_bytes()[1] == b':') - { + if is_local_source(source) { ComponentSource::Path(PathBuf::from(source)) } else { ComponentSource::NpmPackage(source.to_string()) } } +/// Returns `true` when a `--components` source string denotes a local +/// filesystem path rather than an npm package name or scope. +/// +/// A source is a local path when it starts with `.`, `/`, `\`, or a Windows +/// drive letter (e.g. `C:\...`). Everything else — bare names like +/// `my-widget` and scopes like `@scope` / `@scope/pkg` — is an npm package. +/// +/// This is the single source of truth for the classification; callers that +/// pre-resolve sources before handing them to [`discover_source`] (such as +/// `webui-press`, which must resolve local paths against its own working +/// directory while leaving npm names bare) should use it instead of +/// re-implementing the check. +#[must_use] +pub fn is_local_source(source: &str) -> bool { + source.starts_with('.') + || source.starts_with('/') + || source.starts_with('\\') + || (cfg!(windows) && source.len() >= 2 && source.as_bytes()[1] == b':') +} + /// Discover components from a single source and register them into a component registry. /// /// Returns a [`DiscoveryResult`] with the discovered components. @@ -202,4 +218,17 @@ mod tests { ComponentSource::Path(_) )); } + + #[test] + fn test_is_local_source() { + // Local paths + assert!(is_local_source("./libs/shared")); + assert!(is_local_source("../components")); + assert!(is_local_source("/absolute/path")); + assert!(is_local_source("\\unc\\path")); + // npm packages / scopes + assert!(!is_local_source("my-widget")); + assert!(!is_local_source("@reactive-ui")); + assert!(!is_local_source("@scope/button")); + } } diff --git a/crates/webui-discovery/src/npm.rs b/crates/webui-discovery/src/npm.rs index 4ebdeeba..36a10123 100644 --- a/crates/webui-discovery/src/npm.rs +++ b/crates/webui-discovery/src/npm.rs @@ -37,6 +37,24 @@ fn find_node_modules(start: &Path) -> Result { ); } +/// Find `node_modules/` by walking up from `primary`, falling back to a +/// walk up from `fallback` when the primary search comes up empty. +/// +/// The fallback rescues callers whose primary search root lives outside +/// any project tree. For example, `webui-press` builds each docs page in a +/// synthesized scratch directory under the system temp folder, which has no +/// `node_modules` ancestor; the project's `node_modules` is instead reached +/// from the process working directory the command was invoked in. The error +/// from the primary search is preserved when both roots fail. +fn find_node_modules_with_fallback(primary: &Path, fallback: &Path) -> Result { + find_node_modules(primary).or_else(|primary_err| { + if fallback == primary { + return Err(primary_err); + } + find_node_modules(fallback).map_err(|_| primary_err) + }) +} + /// Check if a package name is a bare scope (e.g., `@reactive-ui` without a sub-package). fn is_bare_scope(name: &str) -> bool { name.starts_with('@') && !name.contains('/') @@ -79,10 +97,16 @@ fn read_to_string_limited(path: &Path, max_size: u64) -> Result { /// Resolve an npm package or scope to discovered components. pub fn resolve( name: &str, - cwd: &Path, + search_dir: &Path, cache: &mut DiscoveryCache, ) -> Result> { - let node_modules = find_node_modules(cwd)?; + // Walk up from the build's app directory first, then fall back to the + // process working directory. The fallback covers callers whose app + // directory lives outside the project (e.g. a system-temp scratch dir), + // where the project's `node_modules` is only reachable from the cwd the + // command was invoked in. + let fallback = std::env::current_dir().unwrap_or_else(|_| search_dir.to_path_buf()); + let node_modules = find_node_modules_with_fallback(search_dir, &fallback)?; if is_bare_scope(name) { resolve_scoped(name, &node_modules, cache) @@ -378,6 +402,36 @@ mod tests { assert!(result.is_err()); } + #[test] + fn test_find_node_modules_fallback_used_when_primary_empty() { + let primary = TempDir::new().unwrap(); + let project = TempDir::new().unwrap(); + let nm = project.path().join("node_modules"); + fs::create_dir_all(&nm).unwrap(); + + let found = find_node_modules_with_fallback(primary.path(), project.path()).unwrap(); + assert_eq!(found, nm); + } + + #[test] + fn test_find_node_modules_fallback_prefers_primary() { + let primary = TempDir::new().unwrap(); + let nm_primary = primary.path().join("node_modules"); + fs::create_dir_all(&nm_primary).unwrap(); + let project = TempDir::new().unwrap(); + fs::create_dir_all(project.path().join("node_modules")).unwrap(); + + let found = find_node_modules_with_fallback(primary.path(), project.path()).unwrap(); + assert_eq!(found, nm_primary); + } + + #[test] + fn test_find_node_modules_fallback_errors_when_neither_has_it() { + let primary = TempDir::new().unwrap(); + let fallback = TempDir::new().unwrap(); + assert!(find_node_modules_with_fallback(primary.path(), fallback.path()).is_err()); + } + #[test] fn test_is_bare_scope() { assert!(is_bare_scope("@reactive-ui")); diff --git a/crates/webui-press/Cargo.toml b/crates/webui-press/Cargo.toml index 38676689..a3f8053e 100644 --- a/crates/webui-press/Cargo.toml +++ b/crates/webui-press/Cargo.toml @@ -20,6 +20,7 @@ path = "src/main.rs" [dependencies] microsoft-webui = { path = "../webui", version = "0.0.17", features = ["cli"] } +microsoft-webui-discovery = { path = "../webui-discovery", version = "0.0.17" } microsoft-webui-handler = { path = "../webui-handler", version = "0.0.17" } microsoft-webui-tokens = { path = "../webui-tokens", version = "0.0.17" } diff --git a/crates/webui-press/src/build.rs b/crates/webui-press/src/build.rs index f6d75a99..9d965461 100644 --- a/crates/webui-press/src/build.rs +++ b/crates/webui-press/src/build.rs @@ -88,6 +88,20 @@ fn inject_theme_tokens( Ok(()) } +/// Resolve a configured component source for the per-page builds. +/// +/// Local paths are made absolute against `cwd` (the project root) because +/// `webui-discovery` resolves relative paths against the synthesized per-page +/// app directory, not the project. npm package names and scopes (e.g. +/// `@mai-ui`) are left bare so discovery resolves them from `node_modules`. +fn resolve_config_component_source(source: &str, cwd: &Path) -> String { + if webui_discovery::is_local_source(source) { + cwd.join(source).to_string_lossy().to_string() + } else { + source.to_string() + } +} + // ── Output helpers ────────────────────────────────────────────── // // Mirrors the styling vocabulary in `crates/webui-cli/src/utils/output.rs` @@ -266,6 +280,7 @@ pub fn build_docs_with_cache( // Step 2: Resolve component sources for the per-page builds let mut component_sources: Vec = Vec::new(); + let cwd = std::env::current_dir().unwrap_or_default(); // Built-in component library (e.g. crates/webui-press/components/) let builtin_components = template_dir.parent().map(|p| p.join("components")); if let Some(ref bc) = builtin_components { @@ -273,11 +288,12 @@ pub fn build_docs_with_cache( component_sources.push(bc.to_string_lossy().to_string()); } } - // User component dirs from config - if let Some(ref user_dirs) = config.components { - for d in user_dirs { - let abs = std::env::current_dir().unwrap_or_default().join(d); - component_sources.push(abs.to_string_lossy().to_string()); + // User component sources from config. Local paths are resolved against + // the current project root; npm package names/scopes must stay bare so + // webui-discovery resolves them from node_modules. + if let Some(ref user_sources) = config.components { + for source in user_sources { + component_sources.push(resolve_config_component_source(source, &cwd)); } } // Template-local components (e.g. docs-search, docs-theme-toggle living @@ -1232,6 +1248,31 @@ mod tests { assert_eq!(npx_command(), "npx"); } + #[test] + fn config_component_source_preserves_npm_packages() { + let cwd = Path::new("project"); + + assert_eq!(resolve_config_component_source("@mai-ui", cwd), "@mai-ui"); + assert_eq!( + resolve_config_component_source("@mai-ui/button", cwd), + "@mai-ui/button" + ); + assert_eq!( + resolve_config_component_source("plain-widget", cwd), + "plain-widget" + ); + } + + #[test] + fn config_component_source_resolves_local_paths() { + let cwd = Path::new("project"); + + assert_eq!( + std::path::PathBuf::from(resolve_config_component_source("./components", cwd)), + cwd.join("./components") + ); + } + // --- truncate_utf8 --------------------------------------------------- #[test]