feat: add support for github prefix and named registries#11324
Conversation
|
💖 Thanks for opening this pull request! 💖 |
zkochan
left a comment
There was a problem hiding this comment.
Unfortunately the github: prefix is already preserved for github-hosted repositories. So, this would a be a breaking change and a deviation from the specs used by npm CLI.
|
@zkochan I've changed to ghpkg, please rereview |
|
Use |
|
there is another problem - name collision for github namespace packages. fixing it now |
|
@zkochan named registries somewhat expanded the scope of PR, but here it is now. Please rereview :) |
There was a problem hiding this comment.
Pull request overview
Adds support for installing dependencies from non-default npm registries using vlt-style <alias>: specifiers, including a built-in gh: alias for GitHub Packages, plus publish-time rewriting so these pnpm-specific prefixes don’t leak into published manifests.
Changes:
- Introduces
namedRegistriesconfig and a newgh:/named-registry specifier parser, and wires named-registry resolution into the default resolver pipeline. - Adds named-registry-aware publish manifest rewriting (strip named-registry prefixes, emit
npm:aliases when needed) and updates pack/publish plumbing to pass config through. - Extends deps-resolver fast paths and metadata classification to recognize
named-registryresolutions; adds test coverage and fixtures.
Reviewed changes
Copilot reviewed 30 out of 31 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| store/connection-manager/src/createNewStoreController.ts | Passes namedRegistries through to the client/resolver. |
| resolving/npm-resolver/tsconfig.json | Adds project reference to new gh/named-registry parser package. |
| resolving/npm-resolver/src/index.ts | Merges/validates named registries, adds resolveFromNamedRegistry() and result type. |
| resolving/npm-resolver/src/parseBareSpecifier.ts | Adds helper to parse named-registry specifiers into registry package specs. |
| resolving/npm-resolver/test/resolveNamedRegistry.test.ts | New resolver tests covering gh alias, user aliases, auth lookup, and validation errors. |
| resolving/npm-resolver/test/fixtures/gh-acme-private.json | Fixture metadata for GitHub Packages-style scoped package. |
| resolving/npm-resolver/package.json | Adds dependency on @pnpm/resolving.gh-specifier-parser. |
| resolving/gh-specifier-parser/src/index.ts | New parser for gh: and generic named-registry specifiers + reserved-alias logic. |
| resolving/gh-specifier-parser/test/parse.test.ts | Unit tests for gh/named-registry parsing behavior and error cases. |
| resolving/gh-specifier-parser/{README.md,package.json,tsconfig*.json} | New package scaffolding/docs/build config. |
| resolving/default-resolver/src/index.ts | Plumbs resolveFromNamedRegistry into the resolution chain. |
| core/types/src/package.ts | Adds namedRegistries?: Record<string, string> to PnpmSettings. |
| config/reader/src/Config.ts | Exposes namedRegistries on the config type. |
| config/reader/src/getOptionsFromRootManifest.ts | Adds env-var replacement support for namedRegistries values. |
| releasing/exportable-manifest/src/index.ts | Strips named-registry prefixes on publish, similar to existing jsr: handling. |
| releasing/exportable-manifest/test/index.test.ts | Tests for stripping gh: and user-defined named-registry prefixes. |
| releasing/exportable-manifest/{package.json,tsconfig.json} | Adds dependency/reference on gh-specifier parser package. |
| releasing/commands/src/publish/{publish.ts,recursivePublish.ts,pack.ts} | Threads namedRegistries through publish/pack codepaths. |
| installing/deps-resolver/src/replaceVersionInBareSpecifier.ts | Supports gh: in the “paste locked version” fast path. |
| installing/deps-resolver/test/replaceVersionInPref.test.ts | Tests for gh: replacement and non-registry prefixes. |
| installing/deps-resolver/src/resolveDependencies.ts | Treats named-registry as non-exotic for blockExoticSubdeps. |
| cspell.json | Adds “ghes” to dictionary. |
| .changeset/gh-packages-prefix.md | Changeset describing new feature and config. |
| pnpm-lock.yaml | Lockfile updates including new workspace package + various catalog version shifts. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
compile failed |
|
@zkochan edit: wait, it is still broken, fixing |
|
@zkochan ok this time should be good, can you please rerun? |
|
No, it failed. Why do you push with --no-verify? |
|
@zkochan let me try fix local setup, it was failing with cryptic error locally while trying to execute webhooks |
563fac3 to
a64ad44
Compare
…resolvers' schemes Move resolveFromNamedRegistry to the end of the resolver chain so built-in schemes (npm:, jsr:, git:/github:/gitlab:/…, file:, link:, tarball URLs, runtime resolvers) are always claimed by their dedicated resolver before a user-configured alias gets a chance to shadow them. With the ordering guarantee in place, npm-resolver no longer needs to know about other resolvers' prefixes — drop RESERVED_REGISTRY_ALIASES, isReservedRegistryAlias, and the reserved-alias check from mergeNamedRegistries. URL validation stays since it is npm-resolver's local concern.
…esolver gh: hardcoding The @pnpm/resolving.named-registry-specifier-parser package was only imported by npm-resolver, so its types/parser/built-in alias map are now collocated in npm-resolver/src/parseBareSpecifier.ts and the standalone package is deleted. replaceVersionInBareSpecifier no longer hardcodes 'gh:' alongside 'npm:' - deps-resolver does not know which prefixes are configured as named registries, so a single hardcoded entry is the wrong design. Named- registry specifiers (built-in or user-defined) skip this fast path on purpose; the cost is one extra metadata fetch on re-resolution rather than a correctness bug.
replaceVersionInBareSpecifier now accepts a runtime-built list of named- registry prefixes instead of hardcoding 'gh:'. The deps-resolver merges the built-in defaults (BUILTIN_NAMED_REGISTRIES) with user-configured namedRegistries from pnpm-workspace.yaml and threads the resulting prefix list through ResolutionContext to the call site. Re-installs of locked gh:/work:/etc. specifiers skip the metadata fetch as they did with the hardcoded list, and deps-resolver is no longer pinned to a specific named-registry alias.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…egistries namedRegistries is read from pnpm-workspace.yaml. Reading settings from the root package.json's pnpm field is deprecated in v11.
The previous version restructured the function around a `body` slice and rewrote both the lastIndexOf condition and the no-range branch. The same behavior can be achieved with two minimal changes to the original: swap the single 'npm:' check for a prefix-list check and add one branch for the new `<alias>:<version_selector>` form. The original versionDelimiter logic stays untouched.
Repeating the same description on each PnpmSettings/Config/ResolverFactoryOptions/ ResolutionContext field doesn't match the repo's convention. Also collapse DEFAULT_GH_REGISTRY/BUILTIN_GH_ALIAS into the BUILTIN_NAMED_REGISTRIES literal.
…ge name Unifies the npm: parser with the named-registry shape (gh:^1.0.0): when the body of an npm: specifier is a bare semver range or version, treat the outer dependency alias as the package name. Previously npm:^1.0.0 was parsed as "package literally named ^1.0.0" and 404'd. Restricted to semver.validRange to preserve npm's package-aliasing meaning for tag-like bodies (npm:is-positive, npm:latest).
* Relax named-registry parser to accept unscoped names (work:lodash@^4) and
fall back to any (scoped or unscoped) outer alias for <prefix>:<range>.
Drops the validateScopedPackageName helper; scoped-name validation stays
inline for the @-prefixed branch.
* Optimize replaceVersionInBareSpecifier hot path: check 'npm:' first then
for-loop over namedRegistryPrefixes, avoiding a per-call array allocation.
* Guard replaceEnvInStringValues against arrays so a YAML list under
registries:/namedRegistries: surfaces as a config error instead of being
silently coerced to {0: ..., 1: ...}.
|
Re: Copilot's Other Copilot feedback addressed in 0cba981:
|
…gistry Both resolvers had near-identical bodies: parse spec → look up registry URL → getAuthHeaderValueByURI → pickPackage (with NoMatchingVersionError) → build id/resolution. Extract a pickFromSimpleRegistry helper that returns the common result fragment, and unify calcJsrSpecifier/calcNamedRegistrySpecifier into a single calcPrefixedSpecifier. Net -89/+54 lines. Side effect: jsr now also forwards publishedByExclude and optional through to pickPackage, matching the named-registry resolver and the type signature it already had.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Re Copilot's two new comments on if (!options.update && currentPkg.version && currentPkg.pkgId?.endsWith(\`@\${currentPkg.version}\`) && !calcSpecifier) {
wantedDependency.bareSpecifier = replaceVersionInBareSpecifier(...)
}A If we ever want to harden further, the right fix is at the gate (e.g. require pkgId to start with the package name), not by re-introducing cross-resolver knowledge in npm-resolver/deps-resolver. Test gap (env-var substitution inside |
|
@zkochan seems like there is an unrelated flakiness that blocked the merge?.. |
|
Congrats on merging your first pull request! 🎉🎉🎉 |
|
@zkochan I think an old issue abour JSR support got linked in the release notes for 11.1.0 instead of this one, I assume by accident?.. |
|
fixed. |
This is consistent with #9358, but implements support for the GitHub Packages npm registry and, more broadly, for vlt-style https://docs.vlt.sh/cli/registries for any registry.
This PR adds a built-in gh: specifier that resolves against the GitHub Packages npm registry, plus a namedRegistries config key so a project can map its own aliases to arbitrary registries. A project can mix public npm packages and private GitHub Packages (or self-hosted) ones without applying a scope-wide registry override to every @scope/* package.
The prefix is gh: rather than github: because github: is reserved by npm-package-arg / hosted-git-info as a git host shorthand (e.g. github:owner/repo) - reusing it would be a deviation from the specs used by the npm CLI. gh: is shorter, matches vlt's convention, and cannot collide with any existing npm scheme.
Unlike jsr:, gh: (and any other named-registry alias) does not rewrite the package name - gh:@acme/foo resolves @acme/foo from the GitHub Packages registry as-is. This also means npm/yarn consumers see the original name after the prefix is stripped on publish.