feat: support repo issue and pull count with graphql#150
Conversation
|
@bluwy is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughSwitch repository data fetching from REST to GitHub GraphQL to retrieve issue and PR counts; add a cached GraphQL helper using ohash for cache keys; extend GithubRepo type with Changes
Sequence DiagramsequenceDiagram
actor Client
participant Server as Repo Route
participant CachedFn as ghGraphql (cached)
participant GitHub as GitHub GraphQL API
Client->>Server: GET /repos/{owner}/{repo}
Server->>CachedFn: ghGraphql(RepoInfo, vars)
alt Cache hit
CachedFn-->>Server: cached data
else Cache miss
CachedFn->>GitHub: POST GraphQL {query, variables}
GitHub-->>CachedFn: data (id,name,issues,pullRequests,...)
CachedFn-->>CachedFn: store cache (key = ohash(query + vars))
CachedFn-->>Server: data
end
Server->>Server: map GraphQL result -> GithubRepo
Server-->>Client: { repo: GithubRepo }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 19: The project currently lists "ohash" under devDependencies but it's
imported at runtime in utils/github.ts (import of ohash on line 3), causing
production builds to fail when only prod deps are installed; move "ohash" from
devDependencies into dependencies in package.json so it's installed in
production (update the dependencies object to include the existing version
string for "ohash" and remove it from devDependencies) and ensure
package-lock/yarn.lock is updated by reinstalling dependencies.
In `@routes/repos/`[owner]/[repo]/index.ts:
- Line 72: repository.defaultBranchRef is nullable and currently dereferenced as
repository.defaultBranchRef.name which can throw; change the assignment to guard
the value (e.g., use repository.defaultBranchRef ?
repository.defaultBranchRef.name : null or a fallback string) and update the
GithubRepo.defaultBranch type from string to string | null to match the API
contract; ensure the code paths that consume GithubRepo.defaultBranch handle
null accordingly (look for usages of defaultBranch in this file or downstream
consumers).
In `@utils/github.ts`:
- Around line 163-168: The current error branch always throws createError with
statusCode 500 for any GraphQL errors; change it to inspect res.errors (the
GraphQL response errors array) and if any error object has type === "NOT_FOUND"
throw createError with statusCode 404 and a clear message (e.g., "Repository not
found"), otherwise keep throwing a 500-level createError; update the code in
utils/github.ts where res.errors is handled to check res.errors.some(e => e.type
=== "NOT_FOUND") and branch accordingly while preserving existing error creation
via createError.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d379eb67-ccca-457e-b1f9-143d94092822
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
package.jsonroutes/repos/[owner]/[repo]/index.tstypes/index.tsutils/github.ts
| "eslint": "^9.37.0", | ||
| "eslint-config-unjs": "^0.5.0", | ||
| "nitropack": "npm:nitropack-nightly@2.12.5-20250819-230311.89278001", | ||
| "ohash": "^2.0.11", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Runtime imports of ohash =="
rg -nP --type=ts --type=js 'from\s+"ohash"|require\("ohash"\)' || echo "No matches found"
echo ""
echo "== ohash declaration in package.json =="
jq '{dependencies: .dependencies.ohash, devDependencies: .devDependencies.ohash}' package.json
echo ""
echo "== Check if utils/github.ts exists and its content =="
if [ -f "utils/github.ts" ]; then
head -20 utils/github.ts
else
echo "utils/github.ts not found"
fi
echo ""
echo "== All files in utils directory =="
ls -la utils/ 2>/dev/null || echo "utils directory not found"Repository: unjs/ungh
Length of output: 1135
Move ohash to runtime dependencies.
ohash is imported at runtime by utils/github.ts (line 3) but currently exists only as a dev dependency. Production deployments that install only production dependencies will fail.
🔧 Proposed fix
+ "dependencies": {
+ "ohash": "^2.0.11"
+ },
"devDependencies": {
"@upstash/redis": "^1.35.4",
"eslint": "^9.37.0",
"eslint-config-unjs": "^0.5.0",
"nitropack": "npm:nitropack-nightly@2.12.5-20250819-230311.89278001",
- "ohash": "^2.0.11",
"openapi-renderer": "^0.1.1",
"prettier": "^3.6.2",
"std-env": "^3.9.0",
"typescript": "^5.9.3"
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 19, The project currently lists "ohash" under
devDependencies but it's imported at runtime in utils/github.ts (import of ohash
on line 3), causing production builds to fail when only prod deps are installed;
move "ohash" from devDependencies into dependencies in package.json so it's
installed in production (update the dependencies object to include the existing
version string for "ohash" and remove it from devDependencies) and ensure
package-lock/yarn.lock is updated by reinstalling dependencies.
| forks: repository.forkCount, | ||
| issues: repository.issues.totalCount, | ||
| pullRequests: repository.pullRequests.totalCount, | ||
| defaultBranch: repository.defaultBranchRef.name, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file and its context
cat -n routes/repos/\[owner\]/\[repo\]/index.ts | sed -n '60,85p'Repository: unjs/ungh
Length of output: 731
🏁 Script executed:
# Check the type definitions and imports
head -40 routes/repos/\[owner\]/\[repo\]/index.tsRepository: unjs/ungh
Length of output: 946
🏁 Script executed:
# Search for where repository object is defined/typed
rg -A5 "repository\." routes/repos/\[owner\]/\[repo\]/index.ts | head -50Repository: unjs/ungh
Length of output: 594
🏁 Script executed:
# Look for type definitions related to GithubRepo
rg "interface GithubRepo|type GithubRepo" -A10Repository: unjs/ungh
Length of output: 432
🌐 Web query:
GitHub GraphQL Repository defaultBranchRef nullable schema
💡 Result:
In GitHub’s GraphQL schema, Repository.defaultBranchRef is nullable because its type is Ref (not Ref!)—so clients must handle null. This is shown directly in the schema definition (defaultBranchRef: Ref). [1][2]
Practically, you can receive null when GitHub can’t provide a default-branch ref for that repository (so don’t assume defaultBranchRef { name } always exists). [1][2]
Sources
[1] https://2fd.github.io/graphdoc/github/repository.doc.html
[2] https://gist.github.com/suntong/30ca6ef2a7c32c966a045118fc57de6c
🏁 Script executed:
# Get the full GithubRepo interface definition
rg "interface GithubRepo" -A 20 types/index.tsRepository: unjs/ungh
Length of output: 421
🏁 Script executed:
# Get the full GraphQL query from the route file
sed -n '20,55p' routes/repos/\[owner\]/\[repo\]/index.tsRepository: unjs/ungh
Length of output: 842
Guard nullable defaultBranchRef before dereferencing to prevent runtime crash.
Line 72 dereferences repository.defaultBranchRef.name without null checking. GitHub's GraphQL API returns defaultBranchRef as nullable; when null is returned, this code throws "Cannot read property 'name' of null" at runtime.
Additionally, GithubRepo.defaultBranch is typed as string, but the API can return null, creating a type-safety mismatch.
🔧 Proposed fix
- defaultBranch: repository.defaultBranchRef.name,
+ defaultBranch: repository.defaultBranchRef?.name ?? "",Update GithubRepo.defaultBranch to string | null to accurately reflect the API contract, or default to an appropriate fallback value.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| defaultBranch: repository.defaultBranchRef.name, | |
| defaultBranch: repository.defaultBranchRef?.name ?? "", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@routes/repos/`[owner]/[repo]/index.ts at line 72, repository.defaultBranchRef
is nullable and currently dereferenced as repository.defaultBranchRef.name which
can throw; change the assignment to guard the value (e.g., use
repository.defaultBranchRef ? repository.defaultBranchRef.name : null or a
fallback string) and update the GithubRepo.defaultBranch type from string to
string | null to match the API contract; ensure the code paths that consume
GithubRepo.defaultBranch handle null accordingly (look for usages of
defaultBranch in this file or downstream consumers).
| if (res.errors) { | ||
| throw createError({ | ||
| // Hide errors in case they contain sensitive info | ||
| message: "GitHub GraphQL error", | ||
| statusCode: 500, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GitHub GraphQL API repository not found error response fields type extensions code
💡 Result:
For GitHub’s GraphQL API, a “repository not found” error is returned in the top-level errors array. The error object typically includes:
message(string) — e.g."Could not resolve to a Repository with the name 'owner/name'."(stackoverflow.com)type(string) — acts like the error code, usually"NOT_FOUND"for this case (stackoverflow.com)path(array) — often["repository"](stackoverflow.com)locations(array of{line, column}) — where in the query the failing field occurs (stackoverflow.com)extensions(object, optional) — GitHub sometimes includes extra metadata here (for example{"saml_failure": false}on some auth/forbidden-style errors). (github.com)
Example (repo not found) shape:
{
"data": { "repository": null },
"errors": [
{
"message": "Could not resolve to a Repository with the name 'owner/name'.",
"type": "NOT_FOUND",
"path": ["repository"],
"locations": [{ "line": 3, "column": 9 }]
}
]
}
``` ([stackoverflow.com](https://stackoverflow.com/questions/67878464/repository-type-not-found-graphql-api?utm_source=openai))
Citations:
- 1: https://stackoverflow.com/questions/67878464/repository-type-not-found-graphql-api?utm_source=openai
- 2: https://stackoverflow.com/questions/67878464/repository-type-not-found-graphql-api?utm_source=openai
- 3: https://stackoverflow.com/questions/67878464/repository-type-not-found-graphql-api?utm_source=openai
- 4: https://stackoverflow.com/questions/67878464/repository-type-not-found-graphql-api?utm_source=openai
- 5: https://github.com/orgs/community/discussions/137839?utm_source=openai
- 6: https://stackoverflow.com/questions/67878464/repository-type-not-found-graphql-api?utm_source=openai
---
🏁 Script executed:
```shell
# First, let's understand the context of the code in question
find . -name "github.ts" -type f | head -5Repository: unjs/ungh
Length of output: 70
🏁 Script executed:
# Let's examine the actual code context in utils/github.ts
wc -l ./utils/github.tsRepository: unjs/ungh
Length of output: 74
🏁 Script executed:
# Read the entire file to understand context
cat -n ./utils/github.tsRepository: unjs/ungh
Length of output: 5708
Map repository-not-found errors to HTTP 404, not 500.
Lines 163–168 currently map all GraphQL errors to HTTP 500. This misclassifies repository-not-found conditions as server errors when they should return 404, violating HTTP semantics and breaking client expectations.
GitHub's GraphQL API returns not-found errors with type: "NOT_FOUND" in the error object. Distinguish this case and respond with the appropriate status code:
Suggested approach
- if (res.errors) {
+ if (res.errors?.length) {
+ const notFound = res.errors.some(
+ (error: any) =>
+ error?.type === "NOT_FOUND" || error?.extensions?.code === "NOT_FOUND",
+ );
throw createError({
- // Hide errors in case they contain sensitive info
- message: "GitHub GraphQL error",
- statusCode: 500,
+ message: notFound ? "Repository not found" : "GitHub GraphQL error",
+ statusCode: notFound ? 404 : 502,
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (res.errors) { | |
| throw createError({ | |
| // Hide errors in case they contain sensitive info | |
| message: "GitHub GraphQL error", | |
| statusCode: 500, | |
| }); | |
| if (res.errors?.length) { | |
| const notFound = res.errors.some( | |
| (error: any) => | |
| error?.type === "NOT_FOUND" || error?.extensions?.code === "NOT_FOUND", | |
| ); | |
| throw createError({ | |
| message: notFound ? "Repository not found" : "GitHub GraphQL error", | |
| statusCode: notFound ? 404 : 502, | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@utils/github.ts` around lines 163 - 168, The current error branch always
throws createError with statusCode 500 for any GraphQL errors; change it to
inspect res.errors (the GraphQL response errors array) and if any error object
has type === "NOT_FOUND" throw createError with statusCode 404 and a clear
message (e.g., "Repository not found"), otherwise keep throwing a 500-level
createError; update the code in utils/github.ts where res.errors is handled to
check res.errors.some(e => e.type === "NOT_FOUND") and branch accordingly while
preserving existing error creation via createError.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
routes/users/[name]/repos.ts (1)
22-37: Optional: extract shared repo-list mapper to avoid drift.This mapping is effectively duplicated with
routes/orgs/[owner]/repos.ts; a shared helper would keep both endpoints in sync as fields evolve.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@routes/users/`[name]/repos.ts around lines 22 - 37, The repo mapping in routes/users/[name]/repos.ts duplicates logic used in routes/orgs/[owner]/repos.ts; extract the mapping into a shared helper (e.g., a function named mapGithubRepo or normalizeGithubRepo) that accepts a rawRepo and returns the GithubRepoWithoutIssuesPRs-shaped object (mapping id, name, full_name→repo, description, created_at→createdAt, updated_at→updatedAt, pushed_at→pushedAt, stargazers_count→stars, watchers→watchers, forks→forks, default_branch→defaultBranch), export it from a common utils module, and replace the inline rawRepos.map usage in both routes to call that helper to keep the shape consistent as fields evolve.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@routes/users/`[name]/repos.ts:
- Around line 22-37: The repo mapping in routes/users/[name]/repos.ts duplicates
logic used in routes/orgs/[owner]/repos.ts; extract the mapping into a shared
helper (e.g., a function named mapGithubRepo or normalizeGithubRepo) that
accepts a rawRepo and returns the GithubRepoWithoutIssuesPRs-shaped object
(mapping id, name, full_name→repo, description, created_at→createdAt,
updated_at→updatedAt, pushed_at→pushedAt, stargazers_count→stars,
watchers→watchers, forks→forks, default_branch→defaultBranch), export it from a
common utils module, and replace the inline rawRepos.map usage in both routes to
call that helper to keep the shape consistent as fields evolve.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 992ba5b8-5678-418a-b3d8-88c34b1a7241
📒 Files selected for processing (3)
routes/orgs/[owner]/repos.tsroutes/users/[name]/repos.tstypes/index.ts
closes #148
alternative to #149
Refactors
/repos/[owner]/[repo]to fetch with graphql and return the issue and pr count. The cost of this graphql query is 1 point, which is part of the 5000 points/hour rate limit for authenticated requests.Summary by CodeRabbit
New Features
Chores