refactor: replace semver with verkit#3061
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe changes replace ChangesVerkit SemVer migration
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ 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 |
|
Hello! Thank you for opening your first PR to npmx, @sxzz! 🚀 Here’s what will happen next:
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
e18e dependency analysisNo dependency warnings found. |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/utils/versions.ts (1)
283-299: 🚀 Performance & Scalability | 🔵 TrivialConsider using the normalised range for
satisfies.If
normalizeRange(trimmed)returns a parsed or normalised range string, it may be more efficient or correct to pass the normalised result tosatisfiesrather than the rawtrimmedstring, to avoid duplicate parsing/normalisation insidesatisfies.♻️ Proposed refactor
export function filterVersions(versions: string[], range: string): Set<string> { const trimmed = range.trim() if (trimmed === '') { return new Set(versions) } - if (!normalizeRange(trimmed)) { + const normalised = normalizeRange(trimmed) + if (!normalised) { return new Set() } const matched = new Set<string>() for (const v of versions) { - if (satisfies(v, trimmed, { includePrerelease: true })) { + if (satisfies(v, normalised, { includePrerelease: true })) { matched.add(v) } } return matched }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/utils/versions.ts` around lines 283 - 299, Update filterVersions to retain the result of normalizeRange(trimmed) and pass that normalized range to satisfies instead of the raw trimmed value, while preserving the existing empty-range and invalid-range behavior.app/composables/npm/useOutdatedDependencies.ts (1)
29-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse an explicit null check for consistency.
While
!getPrerelease(v)correctly filters out prereleases (assuming it returns a truthy value for prereleases andnullfor stable versions), using an explicit=== nullcheck improves clarity and aligns with the pattern used inuseInstallSizeDiff.ts.♻️ Proposed refactor
- filteredVersions = versions.filter(v => !getPrerelease(v)) + filteredVersions = versions.filter(v => getPrerelease(v) === null)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/composables/npm/useOutdatedDependencies.ts` at line 29, Update the versions filter in the outdated-dependency flow to retain only entries where getPrerelease(v) explicitly equals null, replacing the truthiness check while preserving the existing prerelease filtering behavior and matching useInstallSizeDiff.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/composables/npm/useOutdatedDependencies.ts`:
- Line 29: Update the versions filter in the outdated-dependency flow to retain
only entries where getPrerelease(v) explicitly equals null, replacing the
truthiness check while preserving the existing prerelease filtering behavior and
matching useInstallSizeDiff.ts.
In `@app/utils/versions.ts`:
- Around line 283-299: Update filterVersions to retain the result of
normalizeRange(trimmed) and pass that normalized range to satisfies instead of
the raw trimmed value, while preserving the existing empty-range and
invalid-range behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c29d7fed-f026-4611-bcbe-ff05c98b3328
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
app/components/Package/Versions.vueapp/components/VersionSelector.vueapp/composables/npm/useOutdatedDependencies.tsapp/composables/useCommandPaletteVersionCommands.tsapp/composables/useInstallSizeDiff.tsapp/pages/package-timeline/[[org]]/[packageName].vueapp/pages/package/[[org]]/[name]/versions.vueapp/utils/npm/api.tsapp/utils/npm/outdated-dependencies.tsapp/utils/publish-security.tsapp/utils/versions.tsnuxt.config.tspackage.jsonpnpm-workspace.yamlserver/utils/compare.tsserver/utils/dependency-analysis.tsserver/utils/dependency-resolver.tsserver/utils/npm.tsserver/utils/version-downloads.ts
|
Thanks for your first contribution, @sxzz! 🙌 We'd love to welcome you to the npmx community. Come and say hi on Discord! And once you've joined, visit npmx.wamellow.com to claim the contributor role. |
🔗 Linked issue
Related to e18e/ecosystem-issues#277
🧭 Context
npmx is an ESM Nuxt application that uses semantic-version operations in both client and server code.
verkitprovides a pure ESM, zero-runtime-dependency implementation with first-party TypeScript declarations, so it fits the project while removing the CommonJSsemverpackage and its separate type package.📚 Description
semverand@types/semverwithverkit@0.1.2tryParseThere should be no user-facing behavior change to version sorting, filtering, dependency resolution, or package analysis.
✅ Validation
pnpm lint:fixpnpm test:typespnpm build