-
Notifications
You must be signed in to change notification settings - Fork 0
⚡ Optimize GitHub Heatmap Fetching #361
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -316,7 +316,7 @@ export async function fetchCommitActivityHeatmap(username: string, year: number, | |
| url.searchParams.set("until", to.toISOString()); | ||
| url.searchParams.set("per_page", "100"); | ||
|
|
||
| const res = await fetch(url.toString(), { headers: headers(token), cache: "no-store" }); | ||
| const res = await fetch(url.toString(), { headers: headers(token), cache: "force-cache" }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/lib/githubYearInReview.ts
Line: 319
Comment:
**現在年のデータが永久に陳腐化する問題**
`force-cache` はリバリデーション期間を指定しないと Next.js のデータキャッシュに永続的に保存されます。`year` が現在の年の場合(例:2026年中に呼び出されると)、年の途中で新しいコミットが追加されても最初に取得したレスポンスが永遠にキャッシュされ続けます。PR 説明では「過去の不変データ」と述べていますが、この関数は過去年だけでなく現在進行中の年にも呼ばれます。過去年のみに `force-cache` を適用するか、`{ next: { revalidate: 3600 } }` などの TTL を設定する形を検討してください。
How can I resolve this? If you propose a fix, please make it concise.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Next.js のデータキャッシュ( Prompt To Fix With AIThis is a comment left during a code review.
Path: src/lib/githubYearInReview.ts
Line: 319
Comment:
**認証トークンがキャッシュキーに含まれないセキュリティ問題**
Next.js のデータキャッシュ(`force-cache`)は `Authorization` などのリクエストヘッダーをキャッシュキーに含めません。キャッシュキーはリクエスト URL のみです。もし複数ユーザーが同じ `owner/repo` をトップリポジトリとして持つ場合、ユーザー A のトークンで取得したプライベートリポジトリのコミット一覧がキャッシュされ、本来アクセス権を持たないユーザー B が同じ URL をリクエストするとキャッシュされたレスポンス(ユーザー A のデータ)が返される可能性があります。`no-store` に戻すか、認証付きリクエストへのキャッシュ適用を慎重に検討してください。
How can I resolve this? If you propose a fix, please make it concise. |
||
| if (res.status === 403) { | ||
| handleRateLimit(res); | ||
| } | ||
|
|
||
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.
Using
force-cacheunconditionally will cache the commit activity heatmap indefinitely. While this is correct and highly efficient for past years (which are immutable), it introduces a correctness bug for the current year, as any new commits made by the user will not be reflected in the heatmap.We should conditionally apply
force-cacheonly if the requestedyearis in the past, and fallback tono-storefor the current year.