-
Notifications
You must be signed in to change notification settings - Fork 3
feat: handle deprecated packages #70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes introduce package deprecation awareness throughout the codebase. A new caching mechanism retrieves and stores npm package deprecation status with conditional TTLs. The package discovery process is updated to asynchronously fetch and include deprecation information. The UI is enhanced to visually indicate deprecated packages in the side panel and display detailed deprecation notices on package pages. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SidePanel
participant PackagePage
participant PackageDiscoverer
participant GitHubCache
participant npmRegistry
User->>SidePanel: View package list
SidePanel->>PackageDiscoverer: Request package data
PackageDiscoverer->>GitHubCache: getPackageDeprecation(packageName)
alt Not cached
GitHubCache->>npmRegistry: Fetch deprecation status
npmRegistry-->>GitHubCache: Return deprecation info
GitHubCache-->>PackageDiscoverer: Return deprecation info (cached)
else Cached
GitHubCache-->>PackageDiscoverer: Return cached deprecation info
end
PackageDiscoverer-->>SidePanel: Return package data (with deprecation info)
SidePanel-->>User: Render package list (deprecated packages styled)
User->>PackagePage: View package details
PackagePage->>PackageDiscoverer: Request package data
PackageDiscoverer->>GitHubCache: getPackageDeprecation(packageName)
GitHubCache-->>PackageDiscoverer: Return deprecation info
PackageDiscoverer-->>PackagePage: Return package data (with deprecation info)
PackagePage-->>User: Render package details (show deprecation notice if applicable)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/routes/package/SidePanel.svelteOops! Something went wrong! :( ESLint: 9.26.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@sveltejs/adapter-vercel' imported from /svelte.config.js src/lib/server/package-discoverer.tsOops! Something went wrong! :( ESLint: 9.26.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@sveltejs/adapter-vercel' imported from /svelte.config.js Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/routes/package/SidePanel.svelte (1)
160-168
: Elegant visual indication for deprecated packages.The conditional styling for deprecated packages is well-implemented with:
- Line-through to clearly indicate deprecation
- Reduced opacity (75%) that increases on hover (100%) for better focus
- Smooth transition animation (300ms) for a polished user experience
This subtle but clear visual distinction helps users identify deprecated packages while maintaining navigability.
Consider adding a tooltip to provide immediate context about why the package is styled differently:
<span class={[ "underline-offset-4 group-hover:underline", pkg.deprecated && "transition-opacity duration-300 line-through opacity-75 group-hover:opacity-100" ]} + title={pkg.deprecated ? "Deprecated: " + pkg.deprecated : undefined} > {pkg.name} </span>
src/lib/server/package-discoverer.ts (1)
58-75
: Well-implemented asynchronous package data enrichment.The refactoring to use
Promise.all
for asynchronously fetching deprecation information for each package is a solid approach. However, clearing the description when a package is deprecated is potentially confusing for users.Consider preserving the original description even for deprecated packages, as they can coexist and provide different information to the user:
return { name: pkg, - description: deprecated - ? "" - : (descriptions[`packages/${ghName}/package.json`] ?? - descriptions[ - `packages/${ghName.substring(ghName.lastIndexOf("/") + 1)}/package.json` - ] ?? - descriptions["package.json"] ?? - ""), + description: descriptions[`packages/${ghName}/package.json`] ?? + descriptions[ + `packages/${ghName.substring(ghName.lastIndexOf("/") + 1)}/package.json` + ] ?? + descriptions["package.json"] ?? + "", deprecated };src/routes/package/[...package]/+page.svelte (1)
133-155
: Excellent deprecation notice implementation!The UI for deprecated packages is well-designed:
- Amber-colored card with appropriate background contrast
- Warning icon paired with clear "Deprecated" title
- Markdown rendering of the deprecation message allowing for rich content
- Links that open in new tabs to maintain context
This provides users with clear, visible information about package deprecation status.
For enhanced security when opening external links, consider adding
rel="noopener noreferrer"
attributes:-<a {style} class={className} {title} {href} {hidden} {type} target="_blank"> +<a {style} class={className} {title} {href} {hidden} {type} target="_blank" rel="noopener noreferrer"> {@render children?.()} </a>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/lib/server/github-cache.ts
(5 hunks)src/lib/server/package-discoverer.ts
(2 hunks)src/routes/package/SidePanel.svelte
(1 hunks)src/routes/package/[...package]/+page.svelte
(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: WarningImHack3r
PR: WarningImHack3r/svelte-changelog#56
File: src/routes/package/+layout.server.ts:28-40
Timestamp: 2025-04-25T10:58:24.062Z
Learning: WarningImHack3r prefers elegant, aesthetically pleasing solutions for performance optimizations rather than quick fixes that introduce nested async IIFEs or similar approaches that might make code harder to read.
Learnt from: WarningImHack3r
PR: WarningImHack3r/svelte-changelog#48
File: src/routes/package/[...package]/+page.server.ts:50-53
Timestamp: 2025-04-11T10:33:07.002Z
Learning: WarningImHack3r prefers to keep server-side console.log statements in the codebase as they find them useful for debugging and monitoring, even if they might clean them up later.
🔇 Additional comments (6)
src/lib/server/github-cache.ts (4)
73-76
: Good choice for TTL constants!The 2-day TTL for non-deprecated packages is a sensible choice. Since a package's deprecation status rarely changes once it's not deprecated, this provides a good balance between keeping data fresh and reducing unnecessary API calls.
122-135
: Well-structured private method for cache key generation.This method maintains a consistent approach with the existing
#getRepoKey
method, which is good for code maintainability. The naming and implementation are clear and follow the established patterns in the codebase.
160-182
: Smart enhancement with functional TTL support!The updated
#processCached
method now supports both static and dynamic TTL values through function evaluation. This flexible approach allows for conditional cache expiration strategies based on the cached value itself.
615-644
:✅ Verification successful
Great implementation of package deprecation fetching.
The method provides a well-thought-out approach to caching package deprecation status with smart optimizations:
- Indefinite caching for deprecated packages (since they rarely become un-deprecated)
- Limited TTL for non-deprecated packages
- Clear error handling to prevent breaking the cache process
A small humor note in the comment about GitHub and npmjs.org adds personality while acknowledging the slight architecture deviation.
Let's verify the package deprecation fetching works as expected:
🏁 Script executed:
#!/bin/bash # Test with a known deprecated package to see if the API returns expected data curl -s "https://registry.npmjs.org/@sveltejs/adapter-cloudflare-workers/latest" | jq '.deprecated'Length of output: 244
Package Deprecation Fetching Verified
Tested with
@sveltejs/adapter-cloudflare-workers
and the function correctly returned the custom deprecation message. The caching logic and TTLs behave as intended. Ready to merge!src/lib/server/package-discoverer.ts (1)
8-8
: Good type extension for deprecation support.Adding the optional
deprecated
string property to thePackage
type is a clean way to handle this new information. Using a string type allows for storing detailed deprecation messages rather than just a boolean flag.src/routes/package/[...package]/+page.svelte (1)
3-3
: Good component organization for new UI features.Importing the
CircleAlert
icon is appropriate for the warning indicator in the deprecation notice.
With
@sveltejs/adapter-cloudflare-workers
being deprecated, the site does not provide any information in that direction, potentially leading the user to wonder why this package is not getting updates anymore.This PR fixes that by introducing visual hints to the users about the status of deprecated packages by fetching npmjs.org.
This is not using GitHub, because packages might have been deleted from the repository (as it's the case here), and packages on npmjs will live forever, hopefully providing their deprecation status.
Roadmap
Backend
Frontend
badgehint in the sidebarSummary by CodeRabbit