fix(npm): support catalog: protocol in overrides#33799
Conversation
The npm overrides parser handled `npm:`, `jsr:`, `$ref`, and plain version strings, but not `catalog:` references. When a package.json used `"catalog:"` or `"catalog:<name>"` in its overrides field, the parser tried to interpret it as a semver range and emitted a warning: "Failed to parse override value "catalog:": Invalid version requirement" This adds a `catalog:` branch to `parse_override_string` that resolves the catalog entry from the workspace catalogs, then parses the resolved version string. Catalogs are threaded through from `npm_overrides_from_workspace` in the resolver factory. Fixes the issue reported in #32947.
- key with version selector (e.g. "foo@^1.0.0": "catalog:") - catalog: in nested override's "." key
fibibot
left a comment
There was a problem hiding this comment.
LGTM. Tight, well-tested fix for a real gap left over from #32947's catalog feature — the catalog: protocol worked in dependencies and peerDependencies but the npm overrides parser was emitting Failed to parse override value "catalog:" for key "...": Invalid version requirement because it didn't know to dispatch on the catalog: prefix.
Substance walk
Catalogs = IndexMap<String, IndexMap<String, String>>is the right shape — ordered (deterministic resolution), nested by catalog name → package name → version string, matches the in-memory representationworkspace.catalogs()already returns.resolve_catalog_overrideatoverrides.rs:478+:- Empty-name →
"default"mapping (catalog:with no name) — matches pnpm/yarn convention. ✓ - Key-selector stripping: uses
parse_override_key(key).0so"foo@^1.0.0": "catalog:"looks up"foo"(not"foo@^1.0.0") in the catalog. Theparse_catalog_override_with_key_selectortest pins this. ✓ - Miss →
UnresolvedCatalogerror (not silent fallthrough to plain version parsing) — important, because otherwise the bug being fixed would just shift to a confusing "Invalid version requirement: catalog:" message. ✓ - Resolved version →
VersionReq::parse_from_npmwithValueParsemapping for malformed catalog values. ✓
- Empty-name →
parse_override_stringdispatch order:npm:→jsr:→catalog:→ fallback to plain version. Thenpm:/jsr:checks run first viastrip_prefixwhich handles "npm:" withnmatching first;catalog:only fires when neither matches. Correct. ✓- Recursive plumbing through
parse_override_value/parse_override_rules— catalogs flow into nested overrides too. Theparse_catalog_override_in_nested_dot_keytest confirms a"foo": { ".": "catalog:", "bar": "1.0.0" }resolves both. ✓ factory.rswiring viaworkspace.catalogs()— uses the existing workspace API; no new public surface added here.
Test coverage
5 new unit tests (parse_catalog_override, _named, _with_key_selector, _in_nested_dot_key, _unresolved) + 1 spec test (tests/specs/npm/overrides/catalog_override/) that exercises the end-to-end with @denotest/different-nested-dep actually resolving to the catalog version 2.0.0. All five units cover the matrix of (default vs named catalog) × (top-level vs key-selector vs nested-dot) plus the error path. The spec test's "output": "[WILDCARD]2\n" confirms the override actually flows through to the resolved package's exported version.
CI
Fully green at this head: 134/136 success, 2 skip, 0 fail. All test libs and spec shards pass — including specs::npm::overrides::catalog_override which is the new spec test.
Smaller observation (non-blocking)
- The dispatch chain in
parse_override_stringchecksnpm:,jsr:, thencatalog:. A pathological input like"npm:bar@catalog:react18"would hit thenpm:branch first and thecatalog:react18would be parsed as the alias's version part — likely failing with a different error. Not a real-world shape (catalog isn't a valid npm version), but worth knowing the dispatch order if anyone reports a confusing error there. Adding a stub test likeparse_npm_alias_with_catalog_in_version_partto lock in the current behavior (error or no) would be belt-and-suspenders. - The single-line PR body cross-reference to
#32947#issuecomment-...is slightly truncated/garbled in the body but readable. Just a cosmetic nit.
|
@bartlomieju I get this now maybe its expected ? |
|
`` |
) - Fixes `catalog:` overrides failing with "could not be resolved (package not found in catalog)" when the catalog is defined inside the `workspaces` object form (Bun/Yarn format: `"workspaces": { "packages": [...], "catalog": {...} }`) - The package.json parser now handles both the array form (`"workspaces": [...]`) and the object form, extracting `catalog`/`catalogs` from the latter - Top-level `catalog`/`catalogs` fields take precedence over those nested in the workspaces object Reported in #33799 (comment)
Summary
The npm overrides parser handles
npm:,jsr:,$ref, and plain versionstrings, but was missing support for
catalog:references. When apackage.jsonused"catalog:"or"catalog:<name>"in itsoverridesfield, the parser tried to interpret it as a semver range and emitted:
This adds a
catalog:branch toparse_override_stringthat resolves theentry from workspace catalogs, then parses the resolved version string.
Both
catalog:(default catalog) andcatalog:<name>(named catalog) aresupported.
Reported in #32947 (comment)
Test plan
parse_catalog_override,parse_catalog_override_named,parse_catalog_override_unresolvedspecs::npm::overrides::catalog_override— end-to-end with@denotest/different-nested-depverifying catalog-resolved override takes effect