feat!: pr/issue tracker #69
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes introduce a new "Tracker" feature for repositories, including navigation updates, server-side redirects, data fetching, and several new Svelte components for displaying repository issues and pull requests. Enhancements to GitHub caching logic support organization members, all issues, and pull requests. The GHBadge component gains new display modes and styling flexibility. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Layout as Tracker Layout
participant Server as Server Load
participant GitHubCache
participant RepoPanel as RepoSidePanel
participant Page as Tracker Page
User->>Layout: Navigate to /tracker, /tracker/org, or /tracker/org/repo
Layout->>Server: load({ url or params })
alt /tracker
Server->>uniqueRepos: Get first repo
alt No repo
Server-->>User: Redirect to /
else
Server-->>User: Redirect to /tracker/{org}/{repo}
end
else /tracker/org
Server->>uniqueRepos: Find first repo for org
alt No repo
Server-->>User: Redirect to /
else
Server-->>User: Redirect to /tracker/{org}/{repo}
end
else /tracker/org/repo
Server->>GitHubCache: getOrganizationMembers(org)
Server->>GitHubCache: getAllPRs(org, repo)
Server->>GitHubCache: getAllIssues(org, repo)
Server->>Server: Filter and sort PRs and issues
Server-->>Layout: Return data
end
Layout->>RepoPanel: Render repo navigation
Layout->>Page: Render issues and PRs lists
Page->>GHBadge: Render badge for each item
Page->>User: Display tracker UI
sequenceDiagram
participant GHBadge
participant Consumer
Consumer->>GHBadge: Render with mode="regular" or mode="minimal"
GHBadge->>GHBadge: Compute textColor and bgColor based on type/status
alt mode = "regular"
GHBadge-->>Consumer: Render badge with icon, label, bgColor, white text
else mode = "minimal"
GHBadge-->>Consumer: Render icon only with textColor, no label/bg
end
sequenceDiagram
participant GitHubCache
participant Redis
participant GitHubAPI
GitHubCache->>Redis: Check cache for members/issues/prs
alt Cache hit
Redis-->>GitHubCache: Return cached data
else Cache miss
GitHubCache->>GitHubAPI: Fetch data from GitHub
GitHubAPI-->>GitHubCache: Return data
GitHubCache->>Redis: Store data with TTL
Redis-->>GitHubCache: Ack
end
GitHubCache-->>Caller: Return data
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/lib/server/github-cache.tsOops! Something went wrong! :( ESLint: 9.26.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@sveltejs/adapter-vercel' imported from /svelte.config.js src/routes/tracker/[org]/[repo]/+page.server.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 (
|
# Conflicts: # src/lib/server/github-cache.ts
WarningImHack3r
changed the title
WarningImHack3r
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (11)
src/lib/server/github-cache.ts (2)
651-666:getOrganizationMembersstops at the first 100 usersThe GitHub REST call is limited to one page. Larger orgs (e.g.
vercel,microsoft) exceed 100 public members, so the tracker will under-filter PRs.Consider auto-paging:
- const { data: members } = await this.#octokit.rest.orgs.listPublicMembers({ org: owner, per_page }); - return members; + const members: Member[] = await this.#octokit.paginate( + this.#octokit.rest.orgs.listPublicMembers, + { org: owner, per_page } + ); + return members;(Identical concern for the two methods below.)
704-720: Pagination missing ingetAllPRs(andgetAllIssues)Only the first 100 open PRs are cached; older-but-still-relevant PRs get ignored, breaking “most recent” ordering once the repo surpasses 100 open PRs.
Use
octokit.paginateor manualpagelooping to fetch all pages, or at least document the limitation.src/routes/+layout.svelte (1)
128-143: Extract navigation items into a constant for clarity & reuseHard-coding the array inside the template makes future edits noisy and duplicates logic (e.g., mobile nav). Defining a
const NAV_ITEMS = [...]in<script>keeps markup clean and avoids repeated allocations.src/routes/tracker/[org]/[repo]/+page.ts (1)
1-10: Type the load function withPageLoadExplicitly annotating the
loadexport prevents accidental misuse and gives IDE autocomplete.-import type { MetaTagsProps } from "svelte-meta-tags"; +import type { MetaTagsProps } from "svelte-meta-tags"; +import type { PageLoad } from "./$types"; -export function load({ data, params }) { +export const load: PageLoad = ({ data, params }) => { ... -}; +};src/routes/tracker/[org]/+page.server.ts (1)
10-10: Consider using relative URL construction.You might want to use the URL pathname like in the base tracker route rather than hardcoding "/tracker/" for better maintainability.
- redirect(307, `/tracker/${matchingRepo.owner}/${matchingRepo.name}`); + redirect(307, `${url.pathname.split("/").slice(0, 2).join("/")}/${matchingRepo.owner}/${matchingRepo.name}`);src/routes/tracker/[org]/[repo]/+page.server.ts (1)
29-43: Consider extracting the PR filtering logic to a separate function.The PR filtering logic is complex with multiple conditions. Extracting it to a named function would improve readability and maintainability.
+ function isPRFixingIssue(body: string | null): boolean { + if (!body) return false; + const lowerBody = body.toLowerCase(); + for (const keyword of closingKeywords) { + if ( + lowerBody.includes(`${keyword} #`) || + lowerBody.includes(`${keyword} https://github.com`) || + new RegExp(`${keyword} [A-z0-9]+/[A-z0-9]+#[0-9]+`).test(lowerBody) + ) { + return true; + } + } + return false; + } return { prs: unfilteredPRs .filter(({ user, body }) => { if (!membersNames.includes(user?.login ?? "")) return false; - if (!body) return true; - const lowerBody = body.toLowerCase(); - for (const keyword of closingKeywords) { - if ( - lowerBody.includes(`${keyword} #`) || - lowerBody.includes(`${keyword} https://github.com`) || - new RegExp(`${keyword} [A-z0-9]+/[A-z0-9]+#[0-9]+`).test(lowerBody) - ) { - return false; - } - } - return true; + return !isPRFixingIssue(body); })src/routes/tracker/[org]/[repo]/+layout.svelte (2)
44-44: Consider standardizing responsive margin increments.The margin values jump from
mt-12tosm:mt-16without intermediate breakpoints, which could cause layout shifts.- class="absolute right-0 mt-12 ml-auto lg:hidden sm:mt-16" + class="absolute right-0 mt-12 sm:mt-14 md:mt-16 ml-auto lg:hidden"
61-61: Non-standard Tailwind class detected.The
mt-43class is not a standard Tailwind class (which typically uses multiples of 4). This might be a typo or custom class.- <RepoSidePanel title="Repositories" class="mt-43 shrink-0 hidden h-fit w-80 lg:flex flex-col"> + <RepoSidePanel title="Repositories" class="mt-44 shrink-0 hidden h-fit w-80 lg:flex flex-col">src/routes/tracker/[org]/[repo]/+page.svelte (3)
64-128: Consider refactoring the complex status determination logic.The nested conditional logic for determining status (lines 74-85) is quite complex and makes the template harder to read.
Consider extracting this logic to a separate function for better maintainability:
+ /** + * Determines the badge status for an item. + * + * @param item GitHub issue or PR + * @returns The appropriate badge status + */ + function determineBadgeStatus(item: Item): PropsObj["status"] { + if (item.state === "closed") { + if ("merged" in item) { + return item.merged ? "merged" : "closed"; + } + return "state_reason" in item && item.state_reason === "completed" ? "solved" : "closed"; + } + return item.draft ? "draft" : "open"; + }Then simplify the template:
- status={item.state === "closed" - ? "merged" in item - ? item.merged - ? "merged" - : "closed" - : "state_reason" in item && item.state_reason === "completed" - ? "solved" - : "closed" - : item.draft - ? "draft" - : "open"} + status={determineBadgeStatus(item)}
67-70: Enhance keyboard accessibility.The link styling includes hover effects but should also include focus styling for keyboard navigation.
- class="flex items-center gap-6 rounded-xl px-4 py-3 transition-colors hover:bg-neutral-100 dark:hover:bg-neutral-800" + class="flex items-center gap-6 rounded-xl px-4 py-3 transition-colors hover:bg-neutral-100 focus:bg-neutral-100 focus:outline-none focus:ring-2 focus:ring-primary dark:hover:bg-neutral-800 dark:focus:bg-neutral-800"
155-158: Use more robust URL parsing for repository extraction.The current approach uses regex to extract repository information from URLs, which could break if GitHub changes its URL structure.
Consider using a more robust URL parsing approach:
- const ownerSlashRepo = issue.html_url - .replace("https://github.com/", "") - .replace(/\/[A-z]+\/\d+$/, ""); + // Parse the URL using the URL API for better reliability + const url = new URL(issue.html_url); + const ownerSlashRepo = url.pathname.split('/').slice(1, 3).join('/');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/lib/components/GHBadge.svelte(3 hunks)src/lib/server/github-cache.ts(6 hunks)src/routes/+layout.svelte(1 hunks)src/routes/tracker/+page.server.ts(1 hunks)src/routes/tracker/[org]/+page.server.ts(1 hunks)src/routes/tracker/[org]/[repo]/+layout.svelte(1 hunks)src/routes/tracker/[org]/[repo]/+page.server.ts(1 hunks)src/routes/tracker/[org]/[repo]/+page.svelte(1 hunks)src/routes/tracker/[org]/[repo]/+page.ts(1 hunks)src/routes/tracker/[org]/[repo]/RepoSidePanel.svelte(1 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.
🧬 Code Graph Analysis (2)
src/routes/tracker/[org]/[repo]/+page.server.ts (4)
src/routes/tracker/+page.server.ts (1)
load(4-9)src/routes/tracker/[org]/+page.server.ts (1)
load(4-11)src/routes/tracker/[org]/[repo]/+page.ts (1)
load(3-10)src/lib/server/github-cache.ts (1)
gitHubCache(755-755)
src/lib/server/github-cache.ts (1)
src/lib/types.ts (2)
Issues(60-60)Pulls(61-61)
🔇 Additional comments (22)
src/routes/tracker/+page.server.ts (1)
4-9: LGTM: Simple and effective redirect logic.The load function effectively redirects from the base
/trackerroute to a specific repository tracker page. The use of 307 (Temporary Redirect) status code is appropriate for this purpose.src/routes/tracker/[org]/+page.server.ts (1)
6-8: Nice use of localeCompare for case-insensitive comparison.The use of
localeComparewithsensitivity: "base"for case-insensitive comparison is a good practice for user-facing routes.src/routes/tracker/[org]/[repo]/+page.server.ts (5)
4-15: Good documentation of GitHub closing keywords.The comprehensive list with the source URL is very helpful for understanding the PR filtering logic that follows.
18-19: Appropriate error handling for missing organization.Good practice to check if members exist and return a 404 error with a clear message if the organization is not found or empty.
23-26: Efficient concurrent API requests.Using Promise.all for concurrent fetching of PRs and issues is a good performance optimization.
46-46: Elegant filtering of issues.Good use of a concise condition to filter out pull requests and ensure the author is an organization member.
44-44:❓ Verification inconclusive
Browser compatibility consideration for toSorted.
The
.toSorted()method is relatively new (ECMAScript 2023) and might require polyfills for older browsers. Ensure your build setup includes appropriate transpilation or polyfills if targeting older browsers.Also applies to: 47-47
🌐 Web query:
What is the browser compatibility for JavaScript Array.prototype.toSorted method?💡 Result:
The
Array.prototype.toSorted()method, introduced in ECMAScript 2023, is supported in the following browsers:
- Chrome: Version 110 and later
- Edge: Version 110 and later
- Firefox: Version 115 and later
- Safari: Version 16.0 and later
- Opera: Version 96 and later
In your case, using Chrome 110, Edge 110, Firefox 115, or Safari 16.0 or later will ensure compatibility with the
toSorted()method. (caniuse.com)For environments where
toSorted()is not supported, such as older browsers or certain JavaScript engines, you can use a polyfill to add support. One such polyfill is available through the npm packagearray.prototype.tosorted, which provides a compliant implementation that works as far back as ES3. (npmjs.com)Additionally, if you're using TypeScript and encounter issues with the
toSorted()method, ensure that yourtsconfig.jsonincludes the appropriate library definitions. Adding"ES2023.Array"to thelibarray can resolve these issues. (stackoverflow.com)By considering these compatibility details and utilizing polyfills or appropriate configurations, you can effectively use the
toSorted()method across different environments.Citations:
- 1: https://caniuse.com/mdn-javascript_builtins_array_tosorted?utm_source=openai
- 2: https://www.npmjs.com/package/array.prototype.tosorted?utm_source=openai
- 3: https://stackoverflow.com/questions/76593892/how-to-use-tosorted-method-in-typescript?utm_source=openai
Ensure compatibility for Array.prototype.toSorted across target browsers
The
toSorted()method is part of ECMAScript 2023 and is supported in:
- Chrome 110+, Edge 110+, Firefox 115+, Safari 16.0+, Opera 96+
If your application needs to run in older browsers or environments without native support, please verify that your build or bundler:
- Includes a polyfill (e.g. install and import the
array.prototype.tosortednpm package)- Transpiles appropriately via your toolchain (Babel, TypeScript, etc.)
- For TypeScript projects, adds
"ES2023.Array"to the"lib"array intsconfig.jsonFiles/locations to review:
- src/routes/tracker/[org]/[repo]/+page.server.ts (lines 44 and 47)
src/routes/tracker/[org]/[repo]/RepoSidePanel.svelte (5)
6-12: Well-structured Props type definition.The Props type is clearly defined with appropriate optional properties and good documentation.
13-13: Modern Svelte 5 props syntax.Using the new
$props()API is a nice touch for clean, declarative props handling with defaults.
15-23: Good use of conditional styling.The dynamic class binding with conditional objects is well implemented, making the component flexible for different use cases.
24-28: Clean conditional rendering.Good use of conditional rendering for the card header based on the headless prop.
29-31: Safe rendering of optional children content.The
@render children?.()pattern safely handles the case when no children are provided.src/lib/components/GHBadge.svelte (5)
2-2: The import of ClassValue is appropriate for type-safe class handling.The addition of this import enables proper type checking for the new
classprop, allowing the component to accept various class value formats.
25-29: Well-structured component props with good TypeScript typing.The addition of the
modeandclassprops enhances the component's flexibility while maintaining type safety. The optional nature of these props with sensible defaults is a good practice.
32-32: The props destructuring uses modern Svelte 5 syntax.Using
$props()for destructuring is the preferred approach in Svelte 5, with good default values set for optional props.
36-37: Good separation of text and background colors.Splitting the single color state into separate
textColorandbgColorstates improves flexibility for the different display modes.
92-103: Excellent implementation of conditional rendering based on display mode.The component handles both display modes effectively:
- "regular" mode provides a full badge with background color and label
- "minimal" mode shows only the icon with appropriate styling
The class composition is well-implemented using Svelte's array syntax to combine fixed, dynamic, and user-provided classes.
src/routes/tracker/[org]/[repo]/+layout.svelte (3)
12-31: Well-structured repository list with active state handling.The snippet effectively handles the active repository styling and creates semantic navigation with proper links and text formatting.
33-37: Good layout structure with responsive considerations.The flex container with
min-w-0on the content area prevents overflow issues, ensuring the content stays within bounds even with long repository names.
38-59: Accessible sheet implementation for mobile navigation.The sheet component implementation is well-structured with proper ARIA attributes, screen reader text, and organized content hierarchy.
src/routes/tracker/[org]/[repo]/+page.svelte (2)
12-46: Well-implemented date utility functions with clear JSDoc.The date utility functions are well-documented and handle various date formatting and comparison needs effectively:
isNewchecks if a date is within the last monthdaysAgoprovides relative time formattingareSameDayperforms precise day comparisonGood use of native JavaScript date methods and the Intl API for localization.
130-143: The repository header is well-designed with good typography and accessibility.The heading structure provides clear hierarchy with appropriate styling and text formatting. The external link has good hover animation with the arrow indicator.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Time for a much-requested feature: a PR tracker.
This PR adds a new route called
/tracker/[user]/[repo](the name looks shady, but I couldn't think of a better one lol).It's a simple page with the top PRs, discussions, and issues for the given repo.
Filtering algorithm
openingupdate dateTake the first 10 results from each categoryNote
I originally wanted to rely almost (exclusively on) reactions in the sorting mechanism, but some interesting PRs (like the latest one on Async Svelte) have no reactions at all (because it's locked and/or moved to a linked discussion); I could try to make this work by fetching the discussion if locked PR/linked discussion exists, but I don't think it will be reliable enough as well as being computation and fetching-expensive
Roadmap
Rely on reactions if any for improved sorting-> requires too much code for too few contentAdd "See more" buttons?-> not needed, too few contentProbably not because there's nowhere to put themSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
Chores