feat(ui): flag git and https dependencies#2234
feat(ui): flag git and https dependencies#2234howwohmm wants to merge 1 commit intonpmx-dev:mainfrom
Conversation
Add visual warning icon next to dependencies that use URL-based version specifiers (git:, https:, github:user/repo, etc.) instead of the npm registry. These bypass npm registry integrity checks and can be manipulated. - Add isUrlDependency() utility in shared/utils/version-source.ts - Detect all non-registry protocols: git:, git+https:, git+ssh:, http:, https:, file:, github:, gist:, bitbucket:, gitlab:, and user/repo shorthand - Show unlink icon with i18n tooltip in all dependency sections (dependencies, peer dependencies, optional dependencies) - Add unit tests with 100% coverage for the detection function Closes npmx-dev#1084 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughThis pull request adds detection and visual indication for URL-based dependencies in the package dependencies interface. A new utility function identifies dependencies using URL schemes (like Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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: 1
🧹 Nitpick comments (1)
test/unit/shared/utils/version-source.spec.ts (1)
67-74: Consider adding tests for workspace and link protocols.The test suite comprehensively covers the documented cases. However, since the protocol regex will also match
workspace:andlink:protocols (commonly used in monorepos), it would be valuable to add explicit tests documenting the expected behaviour for these cases.🧪 Suggested additional test cases
it('returns true for gitlab: prefix', () => { expect(isUrlDependency('gitlab:user/repo')).toBe(true) }) + it('returns true for workspace: protocol (monorepo local refs)', () => { + expect(isUrlDependency('workspace:*')).toBe(true) + expect(isUrlDependency('workspace:^1.0.0')).toBe(true) + }) + + it('returns true for link: protocol (local path refs)', () => { + expect(isUrlDependency('link:../packages/shared')).toBe(true) + }) + it('returns false for scoped packages in semver ranges', () => {
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fc386698-420a-459f-b8b8-ffce87247d3d
📒 Files selected for processing (5)
app/components/Package/Dependencies.vuei18n/locales/en.jsoni18n/schema.jsonshared/utils/version-source.tstest/unit/shared/utils/version-source.spec.ts
| // Protocols: git:, git+https:, git+ssh:, git+http:, https:, http:, file: | ||
| if (/^[a-z][a-z+]*:/i.test(version)) return true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if workspace: or link: protocols are used in any test fixtures or real packages
rg -n 'workspace:|link:' --type json -g 'package.json' || echo "No matches in package.json files"
# Verify the regex behaviour
echo "Testing regex against workspace and link protocols:"
node -e "
const re = /^[a-z][a-z+]*:/i;
console.log('workspace:* matches:', re.test('workspace:*'));
console.log('link:../path matches:', re.test('link:../path'));
"Repository: npmx-dev/npmx.dev
Length of output: 198
🏁 Script executed:
cat -n shared/utils/version-source.ts | head -50Repository: npmx-dev/npmx.dev
Length of output: 1099
🏁 Script executed:
rg -l 'version-source' --type ts --type tsx | head -10Repository: npmx-dev/npmx.dev
Length of output: 88
🏁 Script executed:
rg -B 2 -A 2 'from.*version-source|import.*version-source' --type ts --type tsx | head -30Repository: npmx-dev/npmx.dev
Length of output: 88
🏁 Script executed:
fd -t f 'test\|spec' -e ts | xargs rg -l 'isUrlDependency' 2>/dev/null | head -10Repository: npmx-dev/npmx.dev
Length of output: 177
🏁 Script executed:
rg -B 2 -A 2 'isUrlDependency' --type ts | head -40Repository: npmx-dev/npmx.dev
Length of output: 3367
🏁 Script executed:
cat -n test/unit/shared/utils/version-source.spec.tsRepository: npmx-dev/npmx.dev
Length of output: 3321
🏁 Script executed:
cat -n app/components/Package/Dependencies.vue | grep -A 10 -B 5 'isUrlDependency'Repository: npmx-dev/npmx.dev
Length of output: 3114
Exclude workspace and link protocols from external source detection.
The protocol regex /^[a-z][a-z+]*:/i matches workspace: and link: protocols used by pnpm and Yarn for local monorepo references, causing false positives. These are local, not external sources, and conflict with the function's documented purpose. Add explicit false returns for these protocols before the generic regex check:
Suggested fix
export function isUrlDependency(version: string): boolean {
// npm: protocol aliases are safe (resolved from the registry)
if (version.startsWith('npm:')) return false
+ // workspace: and link: are local monorepo references, not external sources
+ if (version.startsWith('workspace:') || version.startsWith('link:')) return false
+
// Protocols: git:, git+https:, git+ssh:, git+http:, https:, http:, file:
if (/^[a-z][a-z+]*:/i.test(version)) return trueAdd test cases for these protocols to prevent regression.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Protocols: git:, git+https:, git+ssh:, git+http:, https:, http:, file: | |
| if (/^[a-z][a-z+]*:/i.test(version)) return true | |
| export function isUrlDependency(version: string): boolean { | |
| // npm: protocol aliases are safe (resolved from the registry) | |
| if (version.startsWith('npm:')) return false | |
| // workspace: and link: are local monorepo references, not external sources | |
| if (version.startsWith('workspace:') || version.startsWith('link:')) return false | |
| // Protocols: git:, git+https:, git+ssh:, git+http:, https:, http:, file: | |
| if (/^[a-z][a-z+]*:/i.test(version)) return true |
|
there is already a PR tackling the original issue #2017 and this one does not seem to implement showing the warning for nested dependencies. Maybe you can help review that PR instead? |
2 participants
Summary
git:,git+https:,git+ssh:,http:,https:,file:,github:,gist:,bitbucket:,gitlab:, anduser/reposhorthand — everything exceptnpm:aliasesChanges
shared/utils/version-source.ts— newisUrlDependency()utility, pure frontend detection from the version specifier stringapp/components/Package/Dependencies.vue— warning icon with tooltip in all dependency list sectionsi18n/locales/en.json+i18n/schema.json— newurl_dependencytranslation keytest/unit/shared/utils/version-source.spec.ts— 15 test cases covering all formatsDesign decisions
user/repo)i-lucide:unlinkicon — visually distinct from existing icons (circle-alert for outdated, lightbulb for replacement, shield-check for vulnerabilities, octagon-alert for deprecated)Live example
react-native-gauge has a
github:dependency that would be flagged.Test plan
pnpm test -- --run test/unit/shared/utils/version-source.spec.tsCloses #1084
🤖 Generated with Claude Code