Skip to content

[rush] Resolve PNPM catalog references in implicitly preferred versions - #6139

Draft
Bharat Middha (bmiddha) wants to merge 1 commit into
bmiddha-catalog-config-fallbackfrom
bmiddha-catalog-aware-preferred-versions
Draft

Bharat Middha (bmiddha) wants to merge 1 commit into
bmiddha-catalog-config-fallbackfrom
bmiddha-catalog-aware-preferred-versions

Conversation

@bmiddha

Copy link
Copy Markdown
Member

Summary

Part of the pnpm-catalog stack. It is based on #6138 (stack: #6138 → #6136 → #6133 → #6131).

When a project references a dependency through a PNPM catalog ("typescript": "catalog:"), Rush used the literal string catalog: as that package's implicitly preferred version. As a result:

  • rush install crashed with TypeError: Invalid comparator: catalog: when the package is also listed in preferredVersions in common-versions.json. PnpmfileConfiguration calls semver.subset(explicit, implicit). In this repo that would hit typescript, eslint, and minimatch as soon as they move to catalogs.
  • The pnpmfile shim stopped narrowing transitive ranges. For a cataloged package, "catalog:" was not a parseable range, so it was silently dropped.
  • rush add and rush upgrade-interactive saw catalog: as an "existing version".

Rush now resolves catalog references to the versions in the catalog. It uses the catalogs of the subspace that the declaring project belongs to, which is the same thing rush install writes to pnpm-workspace.yaml.

Details

  • New internal helper logic/pnpm/PnpmCatalogReference.ts
    • tryParsePnpmCatalogName(versionSpecifier) maps catalog: and catalog:default to default, and catalog:<name> to <name>.
    • tryGetPnpmCatalogVersion(catalogs, catalogName, packageName) does an own-property-safe lookup.
    • tryResolvePnpmCatalogReference(packageName, versionSpecifier, catalogs) combines the two.
    • Later layers of the stack (catalog-aware rush check and rush add) are meant to reuse these.
  • DependencyAnalyzer
    • Resolves each catalog specifier via getEffectivePnpmCatalogs(project.subspace, rushConfiguration), which is the layer-2 helper that honors the common/config/rush/pnpm-config.json fallback.
    • Catalogs are cached per subspace and loaded lazily, only when a project actually uses a catalog specifier.
    • The resolved version then goes through the existing workspace: and local-project checks.
    • A catalog reference that can't be resolved is ignored. That covers a missing entry, a missing catalog, a non-pnpm package manager, and no catalogs at all. PNPM itself reports ERR_PNPM_CATALOG_ENTRY_NOT_FOUND_FOR_SPEC for these at install time, so Rush neither crashes nor invents a version here.
    • The analysis now also returns catalogNamesByPackageName: Map<string, Set<string>>, which says which catalogs each package is referenced through. It is intended for rush add catalog support.
  • PnpmfileConfiguration
    • Only calls semver.subset when both specifiers are valid SemVer ranges (semver.validRange).
    • Otherwise the explicitly preferred version wins. So npm: aliases, git URLs, file:, or any leftover non-range spec can never crash install.
  • Verified without code changes:
    • The pnpmfile shim never rewrites a workspace project's own catalog: specifiers, because parseRange rejects specs that contain :.
    • PnpmShrinkwrapFile.isWorkspaceProjectModifiedAsync compares catalog: specifiers equal to the v9 lockfile importer specifier: catalog: values. Tests are added for both directions.
  • Behavior change:
    • rush add and rush upgrade-interactive now see resolved versions (e.g. ~5.8.2) instead of catalog:. They still write literal versions; writing catalog: references is left to the rush add layer of the stack.
    • In non-workspace mode, common/package.json now gets real versions for cataloged packages.
    • The shim narrows transitive ranges for cataloged packages again, so a later rush update may produce a slightly different lockfile.
  • Performance: catalogs are only loaded for subspaces whose projects use catalog: specifiers, and at most once per subspace per analysis.
  • Compatibility: IDependencyAnalysis is internal, and there are no public API changes. Only a doc comment on RushConfiguration.getImplicitlyPreferredVersions changed, and the API report is unchanged.

How it was tested

New unit tests:

  • PnpmCatalogReference.test.ts: catalog name parsing, lookups, and resolution.
  • DependencyAnalyzer.test.ts:
    • catalog: and catalog:default are equivalent.
    • Named catalogs resolve.
    • Missing entries and missing catalogs are ignored.
    • Per-subspace resolution works with repoWithSubspacesCommonCatalogs, including the add-action path for a subspace with its own config and a subspace that falls back to common.
  • PnpmfileConfiguration.test.ts:
    • The exact crash scenario: preferredVersions has typescript: ~5.8.2 and a project uses typescript: catalog: with the default catalog typescript: ~5.8.2. Also covers a named catalog plus npm: alias and git preferred versions.
    • The shim narrows external ^5.0.0 / ^2.0.0 / ^9.0.0 ranges to the catalog versions.
    • The shim leaves a workspace project's catalog: specifiers untouched.
  • PnpmShrinkwrapFile.test.ts: a v9 lockfile with catalog importer specifiers is not modified, and changing one specifier is detected as modified.
  • New fixture logic/pnpm/test/repoWithCatalogs/ and lockfiles pnpm-lock-v9/catalogs-{not-,}modified.yaml.

I confirmed the crash: with the original PnpmfileConfiguration.ts and catalog resolution disabled, five of the new tests fail with TypeError: Invalid comparator: catalog:tools.

Commands, run from the worktree root using the locally built dev Rush:

  • node <dev-rush>/apps/rush/lib-commonjs/start-dev.js install: passed.
  • node <dev-rush>/apps/rush/lib-commonjs/start-dev.js build --to @microsoft/rush-lib: passed.
  • cd libraries/rush-lib && node_modules/.bin/heft test --clean --test-path-pattern "DependencyAnalyzer|PnpmfileConfiguration|PnpmShrinkwrapFile|PnpmCatalogReference": passed, 56/56 tests in 4 suites.
  • cd libraries/rush-lib && node_modules/.bin/heft test --clean --production:
    • 1957 tests passed and 1 failed.
    • The failure is the pre-existing TarExecutable › kills tar when its signal aborts while it runs, which also fails on the base branch.
    • There were no build, lint, or API Extractor warnings, and the API report is unchanged.

A rush change file for @microsoft/rush (patch) is included.

DependencyAnalyzer resolves `catalog:` specifiers using the catalogs of
each project's subspace, so implicitly preferred versions are real
ranges instead of "catalog:". This fixes the semver.subset crash in
rush install for cataloged packages listed in preferredVersions and
restores pnpmfile shim range narrowing. Unresolvable catalog references
are ignored. PnpmfileConfiguration skips semver.subset when either side
is not a valid range. Also records catalogNamesByPackageName for rush add.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Needs triage

Development

Successfully merging this pull request may close these issues.

1 participant