Skip to content

refactor: typed path parameters in plugin config parsers #448

@BartWaardenburg

Description

@BartWaardenburg

Several plugin config parsers use `&str` and `String` for path-shaped values, with no contract on separator consistency or absolute-vs-relative semantics. On non-Linux platforms this leaks: the string flows through joins, lookups, and serializers without a single point of normalization. The current code mostly works because tests assert on Linux paths and most users are on Linux + macOS, but the design is structurally fragile.

Concrete cases

  1. `crates/core/src/plugins/nuxt.rs:603` - `prefix_with_src_dir(src_dir: &str, path: &str) -> String` concatenates paths as strings without normalization.

  2. `crates/core/src/plugins/webpack.rs:214-240` - `normalize_context_entry()` and `normalize_project_relative_join()` construct paths from strings, then manually parse components. No type-level guarantee about separator consistency between `base` and `child`.

  3. `crates/core/src/plugins/config_parser.rs` (~15 functions) - Functions like `extract_config_path_string()`, `extract_config_aliases()` return `String` for paths. Callers in `webpack.rs:42-57` must remember to normalize downstream; the contract is convention only.

  4. `crates/types/src/serde_path.rs:15,27` - Compensates via `.to_string_lossy().replace('\\', "/")` at serialization time. In-memory collections use `PathBuf` (good), but anywhere a `String` slips out is a future bug on Windows.

Scope

Phase 1: Replace return types in `config_parser.rs` so path-shaped functions return `PathBuf` instead of `String`. Update the ~15 call sites; the compiler will surface every consumer.

Phase 2: Update `webpack.rs` and `nuxt.rs` plugin config parsers to accept `&Path` parameters, not `&str`. `Path::join` becomes the single concatenation point.

Phase 3 (optional, larger): Introduce a `RepoRelativePath` newtype that wraps `PathBuf` and documents the invariant "relative to repository root, forward slashes only." Use it for the boundary between git-derived paths (`changed_files.rs`) and downstream consumers. Currently the invariant lives in a comment block in `changed_files.rs:161` and is implicit at all other call sites.

Why this matters

  • Windows path bugs surface as silent string concatenation on a backslash + forward-slash split. A type-level boundary prevents the class of error.
  • `changed_files.rs` already documents the repo-root-relative contract in a comment; promoting it to a newtype enforces the contract at every consumer.
  • Plugin config parsers are the most exposed surface for user-supplied paths (tsconfig paths, webpack aliases, nuxt srcDir). A typed signature is the cheapest place to enforce normalization.

Verification

  • `cargo build --workspace` passes after each phase.
  • Tests under `crates/graph/tests/cross_platform.rs` continue to pass.
  • A new test asserts that `prefix_with_src_dir` (or its successor) produces consistent separators regardless of input form.

Out of scope

  • General sweep of `.to_string_lossy()` usages outside the config-parser surface. That is a diffuse audit better done ad-hoc when a specific Windows bug surfaces.
  • Renaming or reorganizing existing PathBuf-typed code; this issue is about the str-typed boundary.

Refs PR #55 (Windows path normalization, KamilDev), project rule `.claude/rules/code-quality.md`.

Metadata

Metadata

Labels

area:configConfiguration loading, validation, and schemasarea:pluginsFramework/external plugin systemenhancementNew feature or requestpriority:lowDeferred cleanup, blocked dependency work, or nice-to-haverustPull requests that update rust codetype:refactorInternal cleanup without intended user-facing behavior change
No fields configured for Maintenance.

Projects

Status
Done

Relationships

None yet

Development

No branches or pull requests

Issue actions