fix: always color replaceable packages as orange#2748
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR reorders CSS class selection priority in a single dependency version highlighting function. Replacement status now takes precedence over vulnerable or deprecated status when determining the visual highlight colour. ChangesDependency version class priority
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/components/Package/Dependencies.vue (1)
83-84: ⚡ Quick winConsider simplifying the redundant conditional.
Both lines 83 and 84 return
getVersionClass(undefined), making the conditional check on line 83 functionally redundant. Since vulnerable and deprecated dependencies use the same default styling (represented by icons rather than version text colour), you could simplify this to:if (replacementDeps.value[dep]) return 'text-amber-700 dark:text-amber-500' - if (getVulnerableDepInfo(dep) || getDeprecatedDepInfo(dep)) return getVersionClass(undefined) return getVersionClass(undefined)This removes the unnecessary condition whilst maintaining identical behaviour. If you're planning to differentiate vulnerable/deprecated styling in future, keep the structure; otherwise, the simplification improves readability.
🤖 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/components/Package/Dependencies.vue` around lines 83 - 84, The conditional in Dependencies.vue is redundant: both branches return getVersionClass(undefined) when checking getVulnerableDepInfo(dep) || getDeprecatedDepInfo(dep); simplify by removing the if-check and directly return getVersionClass(undefined) (keeping references to getVulnerableDepInfo, getDeprecatedDepInfo, getVersionClass and the dep parameter in mind) unless you intentionally plan to add different styling for vulnerable/deprecated deps in which case preserve the current structure.
🤖 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/components/Package/Dependencies.vue`:
- Around line 83-84: The conditional in Dependencies.vue is redundant: both
branches return getVersionClass(undefined) when checking
getVulnerableDepInfo(dep) || getDeprecatedDepInfo(dep); simplify by removing the
if-check and directly return getVersionClass(undefined) (keeping references to
getVulnerableDepInfo, getDeprecatedDepInfo, getVersionClass and the dep
parameter in mind) unless you intentionally plan to add different styling for
vulnerable/deprecated deps in which case preserve the current structure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aa8f8aab-7a26-4a99-9ccd-305b31b81706
📒 Files selected for processing (1)
app/components/Package/Dependencies.vue
| if (replacementDeps.value[dep]) return 'text-amber-700 dark:text-amber-500' | ||
| if (getVulnerableDepInfo(dep) || getDeprecatedDepInfo(dep)) return getVersionClass(undefined) |
There was a problem hiding this comment.
could we add test coverage for this precedence?
🔗 Linked issue
🧭 Context
Before:
After:
📚 Description