feat: add deprecation warning#35
Conversation
|
Warning Review limit reached
Next review available in: 31 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughPackage analysis now records deprecation status for each dependent, checks statuses in batches with progress updates, and displays warning indicators in Markdown output and the results table. API fetch caching and dependent-version parsing are also updated. ChangesPackage deprecation indicators
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
|
Preview deployment ✅ Deployment complete!
|
![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)
src/components/App.tsx (1)
160-172: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider the N+1 API call pattern and silent error handling for deprecation checks.
Each item triggers a separate
getPackageIsDeprecatedHTTP request — up to 3000 for a full result set. While batching (chunks of 50) andcachedFetchmitigate repeat cost, the first analysis for a large package could be slow and risk registry rate-limiting. Additionally,.catch(() => false)on line 165 silently masks network and rate-limit errors as "not deprecated," which could produce incorrect results under load.Two things worth considering:
- The
DependentValueObjecttype added inapi.ts(line 159) includes adeprecatedfield — if the dependents view data already contains this, you could readr.value.deprecated?.deprecateddirectly duringgetSortedDependentsprocessing and avoid the N+1 calls entirely.- If separate fetches remain necessary, consider logging or surfacing a count of failed deprecation checks so users know results may be incomplete.
🤖 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 `@src/components/App.tsx` around lines 160 - 172, The deprecation analysis loop in App.tsx performs one request per item and silently treats failures as not deprecated. Reuse the existing deprecated value from DependentValueObject during getSortedDependents processing when available, removing unnecessary getPackageIsDeprecated calls; otherwise retain the fetches but track and surface failed checks instead of mapping errors silently to false.
🤖 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.
Inline comments:
In `@src/components/App.tsx`:
- Around line 534-549: Update the deprecated icon SVG in the pkg.deprecated Show
block to include role="img" and aria-label="Deprecated" alongside its existing
title, matching the accessibility attributes used by the error icon.
---
Nitpick comments:
In `@src/components/App.tsx`:
- Around line 160-172: The deprecation analysis loop in App.tsx performs one
request per item and silently treats failures as not deprecated. Reuse the
existing deprecated value from DependentValueObject during getSortedDependents
processing when available, removing unnecessary getPackageIsDeprecated calls;
otherwise retain the fetches but track and surface failed checks instead of
mapping errors silently to false.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c681d427-3648-4397-9fb4-2f9e3024f310
📒 Files selected for processing (2)
src/components/App.tsxsrc/lib/api.ts
For now, I was deciding against this idea (because I was also thinking about it), because deprecated packages are very rare, which means we would have a column that is mostly empty. And I also wonder how often such a column would actually be filled with some info, even if we add more stuff like "has a replacement", "last updated...", etc. IMHO a column should only be added if every package has associated info with it, which isn't the case here. That's why I am a little hesitant. Especially, because tables with many rows quickly become inaccessible on smaller devices as well... |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
I think it depends on the package, packages that have a replacement are more likely to have deprecated or replaceable dependents too What I think we could do is keep the icon in the ui, but when "Copying as Markdown table", then, if a package has a note, add a Notes column, instead of copying an emoji
If no package in the table has any note, we could hide the column maybe? Don't know if that's a good idea though
Yeah, it's currently not very good on mobile, I think mobile needs some more improvements, maybe on mobile make it into cards or several rows instead of simple table |
2 participants
Description
This PR adds a little warning symbol to deprecated packages near the name of the package in the list. To make it accessible, it also has a
<title>Deprecated</title>, which shows on hover and gets picked up by screen readers.Here is an example in the preview: https://pr-35-dependents-dev.gameroman.workers.dev/?package=qs
Summary by CodeRabbit
New Features
Bug Fixes