Fix npm alias packages (e.g. "npm:eslint@^9.x") failing every operation#5039
Open
Scott (7ranceaddic7) wants to merge 2 commits into
Open
Fix npm alias packages (e.g. "npm:eslint@^9.x") failing every operation#5039Scott (7ranceaddic7) wants to merge 2 commits into
Scott (7ranceaddic7) wants to merge 2 commits into
Conversation
npm outdated --json / npm list --json report npm-aliased dependencies (package.json entries like "eslint-v9": "npm:eslint@^9.39.4") with a package id shaped like "eslint-v9:eslint@^9.39.4" -- the local alias name, a literal colon, then the raw alias target specifier with the "npm:" prefix stripped. NpmPkgOperationHelper was using that id verbatim as the npm package specifier, producing commands like "npm install 'eslint-v9:eslint@^9.39.4@10.6.0'", which npm rejects outright with EINVALIDPACKAGENAME (colons aren't valid in npm package names) -- so any aliased package could never be installed, updated, or uninstalled through UniGetUI. Real npm package names can never contain a colon, so its presence in package.Id unambiguously identifies an alias. Added alias-aware resolution in NpmPkgOperationHelper: reconstructs the proper "localName@npm:targetName@version" syntax for install/update, and uses just the local name for uninstall (npm doesn't need or accept the target spec to remove a package). Verified against a real project with this exact scenario: the generated update command changed from the invalid "eslint-v9:eslint@^9.39.4@10.6.0" to the valid "eslint-v9@npm:eslint@10.6.0", which npm accepted and executed successfully (confirmed via a temporary, uncommitted test calling the production GetParameters API directly, then running the exact returned command for real -- not committed to this diff). Added 4 regression tests to NpmManagerTests.cs covering: alias resolution on update, a scoped alias target (target names can contain their own '@', e.g. "npm:@babel/core@^7.x"), local-name-only uninstall, and that ordinary non-aliased package ids are left untouched.
Author
|
Raw npm debug log from the original crash, for reference (npm's own log, not reconstructed): npm 11.13.0 / Node v24.16.0 / Windows 10.0.28120. Confirms the rejection happens at |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
npm supports aliasing a dependency to install a different underlying package under a local name, e.g.:
npm outdated --json/npm list --jsonreport these with a package id shaped likeeslint-v9:eslint@^9.39.4-- the local alias name, a literal colon, then the raw alias target specifier with thenpm:prefix stripped.NpmPkgOperationHelperwas using that id verbatim as the npm package specifier, producing commands like:npm rejects this outright:
So any npm-aliased package could never be installed, updated, or uninstalled through UniGetUI -- every operation fails with this error.
Fix: real npm package names can never contain a colon, so its presence in
package.Idunambiguously identifies an alias (this is confirmed directly against realnpm outdated --jsonoutput, not inferred). Added alias-aware resolution inNpmPkgOperationHelper: reconstructs the properlocalName@npm:targetName@versionsyntax for install/update, and uses just the local name for uninstall (npm doesn't need or accept the target spec to remove a package). Handles scoped alias targets too (npm:@babel/core@^7.x), since the target name can contain its own@.Real-world verification: ran this against a real project with this exact scenario. The generated update command changed from the invalid
eslint-v9:eslint@^9.39.4@10.6.0to the valideslint-v9@npm:eslint@10.6.0, which npm accepted and executed successfully (exit code 0). Done via a temporary, uncommitted test calling the productionGetParametersAPI directly to get the real command, then running that exact command for real -- not part of this diff.One caveat worth flagging for reviewers, discovered during that real-world run: this fix makes the operation succeed, but "succeeds" means npm does exactly what an update always does -- rewrites the alias's version specifier in
package.jsonto the new resolved version. If someone is using an alias specifically to pin a dependency below its target's latest major (as in the motivating case: keeping a secondeslint-v9on the 9.x line to satisfy a plugin's stale peer range while a top-leveleslintstays on 10.x), clicking "Update" on that alias will happily bump it past the pin and defeat its purpose -- because from npm's perspective it genuinely is just an update. That's expected/correct npm behavior, not a bug this PR introduces, and not something this fix should try to guess around. Users in that situation should use UniGetUI's existing "ignore future updates" option on that specific package.Verified with
dotnet buildonUniGetUI.PackageEngine.Managers.Npm.csproj(0 errors) anddotnet testonUniGetUI.PackageEngine.Tests(256/256 passing, 4 new regression tests added, 0 regressions) against SDK 10.0.301.N/A. No existing issue tracks this; not creating one per the PR template guidance.