Skip to content

refactor(pacquet/config): replace Config::current's default callback with a mut self builder method #11731

@KSXGitHub

Description

@KSXGitHub

Context

Config::current (pacquet/crates/config/src/lib.rs) currently has this signature:

pub fn current<Sys, Default>(
    start_dir: &Path,
    default: Default,
) -> Result<Self, LoadWorkspaceYamlError>
where
    Sys: EnvVar + EnvVarOs + GetHomeDir,
    Default: FnOnce() -> Config,
{
    let mut config = default();
    // ... layer .npmrc and pnpm-workspace.yaml onto `config` ...
    Ok(config)
}

The default closure is invoked unconditionally on the first line — the laziness it advertises is never exercised. Of the 19 call sites:

  • The single production caller (crates/cli/src/cli_args.rs) passes Default::default.
  • 17 of 18 test callers pass Config::new, which is just Self::default().
  • 1 test (test_current_folder_fallback_to_default) passes || Config { symlink: false, ..Config::new() } to assert that fields the loader doesn't touch survive on top of a caller-supplied starting Config.

So the closure parameter exists to support one test, the laziness is never used, and every production path passes the same thing.

Proposed shape

Keep the method name Config::current but reshape it into a builder that takes mut self. The caller builds the starting Config and the method layers .npmrc + pnpm-workspace.yaml on top of it:

impl Config {
    fn current<Sys>(mut self, start_dir: &Path) -> Result<Self, LoadWorkspaceYamlError>
    where
        Sys: EnvVar + EnvVarOs + GetHomeDir,
    {
        // ... layer .npmrc and pnpm-workspace.yaml onto `self` ...
        Ok(self)
    }
}

Production becomes:

Config::default().current::<Host>(&dir)

The one outlier test becomes:

Config { symlink: false, ..Config::new() }
    .current::<HostWithHome>(current_dir.path())

Notes

  • Out of scope for this issue: the inner layering logic (.npmrc reads, yaml walk, GVS derivation). Only the entry shape changes — the method name stays current.
  • Follow-up from refactor(pacquet): replace callback-DI with capability traits #11730. That PR converted Config::current's current_dir and home_dir callback-DI parameters to direct path / trait-DI, but left default alone because it isn't a side-effect seam and converting it wasn't on the task.

Written by an agent (Claude Code, claude-opus-4-7).

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions