Conversation
# Conflicts: # src/providers/diagnostics/rules/replacement.ts # src/providers/diagnostics/rules/upgrade.ts # src/providers/diagnostics/rules/vulnerability.ts # src/utils/package.ts # tests/utils/package.test.ts
📝 WalkthroughWalkthroughThis PR adds package-name resolution and JSR-aware handling across utilities and providers. It extends version parsing with an aliasName, adds helpers (resolvePackageName, isJsrNpmPackage, jsrNpmToJsrName), and threads a resolved Possibly related PRs
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/providers/diagnostics/rules/upgrade.ts (1)
28-29:⚠️ Potential issue | 🟡 MinorCompare against the raw dist-tag, not
exactVersion.
exactVersionis already the resolved semver by this point, so this condition will not fire for normal tag-based inputs such aslatestornext. The rule still emits upgrade hints for dist-tag specs instead of lettingcheckDistTagown that case.Minimal fix
- if (Object.hasOwn(pkg.distTags, exactVersion)) + if (Object.hasOwn(pkg.distTags, parsed.version)) return
🧹 Nitpick comments (2)
src/providers/completion-item/version.ts (1)
37-40: Unreachable guard:resolvePackageNamealways returns a string.Per
src/utils/package.ts:19-21,resolvePackageNamereturnsparsed?.aliasName ?? depName, which always yields a non-empty string sincedepName(frominfo.name) is guaranteed to be truthy. Theif (!packageName)check is therefore unreachable.This is harmless defensive code, but you could simplify by removing the guard.
🔧 Optional simplification
const packageName = resolvePackageName(name, parsed) - if (!packageName) - return - const pkg = await getPackageInfo(packageName)src/utils/package.ts (1)
29-38: Consider defensive handling for malformed JSR npm names.The function correctly transforms
@jsr/scope__name→@scope/name. However, when no__separator is found (lines 35-36), it returns a bare name like'something', which is invalid for JSR (all JSR packages are scoped).Since this edge case shouldn't occur with valid JSR npm packages, this is low priority. Optionally, you could return
nullor throw to surface malformed input early.🛡️ Optional defensive handling
const bare = name.slice(JSR_NPM_SCOPE.length) const separatorIndex = bare.indexOf('__') if (separatorIndex === -1) - return bare + return bare // Note: bare names without scope are invalid for JSR.io return `@${bare.slice(0, separatorIndex)}/${bare.slice(separatorIndex + 2)}`
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c185daab-8014-47af-a6b7-0829dfc3d395
📒 Files selected for processing (15)
playground/package.jsonsrc/providers/completion-item/version.tssrc/providers/diagnostics/index.tssrc/providers/diagnostics/rules/deprecation.tssrc/providers/diagnostics/rules/dist-tag.tssrc/providers/diagnostics/rules/engine-mismatch.tssrc/providers/diagnostics/rules/replacement.tssrc/providers/diagnostics/rules/upgrade.tssrc/providers/diagnostics/rules/vulnerability.tssrc/providers/hover/npmx.tssrc/utils/package.tssrc/utils/version.tstests/diagnostics/context.tstests/utils/package.test.tstests/utils/version.test.ts
| if (protocol === 'npm') { | ||
| const lastAtIndex = version.lastIndexOf('@') | ||
| if (lastAtIndex > 0) { | ||
| aliasName = version.substring(0, lastAtIndex) | ||
| version = version.substring(lastAtIndex + 1) | ||
|
|
||
| if (isJsrNpmPackage(aliasName)) { | ||
| aliasName = jsrNpmToJsrName(aliasName) | ||
| protocol = 'jsr' | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -name "version.ts" -type f 2>/dev/null | head -20Repository: npmx-dev/vscode-npmx
Length of output: 46
🏁 Script executed:
git ls-files | grep -E "(version\.ts|npmx\.ts)" | head -20Repository: npmx-dev/vscode-npmx
Length of output: 223
🏁 Script executed:
git ls-files src/utils/ | head -20Repository: npmx-dev/vscode-npmx
Length of output: 288
🏁 Script executed:
cat -n src/utils/version.ts | head -80Repository: npmx-dev/vscode-npmx
Length of output: 2844
🏁 Script executed:
cat -n src/utils/version.ts | tail -40Repository: npmx-dev/vscode-npmx
Length of output: 1354
🏁 Script executed:
cat -n src/utils/package.ts | grep -A 10 "isJsrNpmPackage\|jsrNpmToJsrName"Repository: npmx-dev/vscode-npmx
Length of output: 670
🏁 Script executed:
cat -n src/providers/completion-item/version.ts | head -50Repository: npmx-dev/vscode-npmx
Length of output: 1881
🏁 Script executed:
cat -n src/providers/document-link/npmx.ts | head -50Repository: npmx-dev/vscode-npmx
Length of output: 2311
🏁 Script executed:
cat -n src/providers/document-link/npmx.ts | sed -n '47,120p'Repository: npmx-dev/vscode-npmx
Length of output: 2415
🏁 Script executed:
cat -n src/utils/package.ts | head -50Repository: npmx-dev/vscode-npmx
Length of output: 1856
🏁 Script executed:
grep -r "isSupportedProtocol\|formatUpgradeVersion" src/ --include="*.ts" -B 2 -A 2 | head -80Repository: npmx-dev/vscode-npmx
Length of output: 5593
🏁 Script executed:
cat -n src/utils/version.ts | sed -n '1,60p'Repository: npmx-dev/vscode-npmx
Length of output: 2209
🏁 Script executed:
find . -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "parseVersion\|jsrNpmPackage" 2>/dev/null | head -5Repository: npmx-dev/vscode-npmx
Length of output: 93
🏁 Script executed:
rg "jsr:" -A 3 -B 3 --type ts | head -60Repository: npmx-dev/vscode-npmx
Length of output: 908
🏁 Script executed:
cat -n tests/utils/version.test.tsRepository: npmx-dev/vscode-npmx
Length of output: 5515
🏁 Script executed:
cat -n src/utils/package.ts | sed -n '19,42p'Repository: npmx-dev/vscode-npmx
Length of output: 922
🏁 Script executed:
rg "npm-compatible\|jsr.*npm\|@jsr/" --type ts -i -B 2 -A 2 | head -100Repository: npmx-dev/vscode-npmx
Length of output: 46
🏁 Script executed:
rg "parseVersion\|formatUpgradeVersion" src/providers -A 3 -B 1 | grep -A 3 "isSupportedProtocol" | head -40Repository: npmx-dev/vscode-npmx
Length of output: 46
🏁 Script executed:
cat -n src/utils/resolve.tsRepository: npmx-dev/vscode-npmx
Length of output: 1968
Rewrite npm-compatible JSR aliases to preserve the npm protocol rather than converting to jsr.
When parsing npm:@jsr/luca__cases@^1.0.1, the code rewrites the protocol from npm to jsr after transforming the alias name to @luca/cases (lines 51–53). This causes the parsed result to have protocol: 'jsr', which is in the UNSUPPORTED_PROTOCOLS set, so downstream callers like completion and document-link providers that check isSupportedProtocol(parsed.protocol) skip processing these dependencies entirely.
Additionally, formatUpgradeVersion() would then output jsr:@luca/cases@version instead of the original npm:@jsr/luca__cases@version, losing the ability to round-trip the original spec.
Preserve the npm protocol and alias target during parsing, and delegate canonicalisation of the package identity (mapping @jsr/luca__cases → @luca/cases and determining the final registry) to a separate resolver step when needed for metadata or link resolution.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 70355416-7033-44e6-88d0-c710040b0d70
📒 Files selected for processing (2)
src/providers/diagnostics/rules/upgrade.tstests/utils/package.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/utils/package.test.ts
| if (Object.hasOwn(pkg.distTags, dep.version)) | ||
| return |
There was a problem hiding this comment.
Alias dist-tags are no longer filtered out.
Line 28 now checks pkg.distTags against dep.version, but DependencyInfo.version is the raw manifest spec (src/types/extractor.ts:8-13). For alias declarations like npm:react@next or npm-compatible JSR aliases, that raw value will never equal a dist-tag key such as next, so this early return is skipped and we emit a bogus upgrade hint. Please normalise the declared spec down to its tag component before calling hasOwn, or read the tag from parsed instead.
1 participant
Support alias packages like
npm:nuxt@~4.3.0This PR also tries to resolve JSR packages, see: https://jsr.io/docs/npm-compatibility