Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
158 changes: 158 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
"jsdom": "^28.1.0",
"server-only": "^0.0.1",
"tailwindcss": "^4",
"ts-node": "^10.9.2",
"typescript": "^5",
Comment on lines 47 to 49

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The ts-node dependency appears to be unused in this project. Next.js and Vitest both natively support TypeScript compilation and execution without requiring ts-node. Removing this dependency will keep the package.json clean and reduce the installation footprint.

Suggested change
"tailwindcss": "^4",
"ts-node": "^10.9.2",
"typescript": "^5",
"tailwindcss": "^4",
"typescript": "^5",

"vitest": "^4.1.5"
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 不要な依存関係の追加

ts-node の追加がこの PR の目的(キャッシュ設定の変更)と無関係に見えます。PR の説明には ts-node を追加する理由が一切記載されておらず、変更されたソースファイル(cardDataFetcher.ts)でも使用されていません。Jules が自動生成したものである可能性が高く、意図しない追加の可能性があります。この依存関係が本当に必要かどうか確認をお願いします。

Prompt To Fix With AI
This is a comment left during a code review.
Path: package.json
Line: 51

Comment:
**不要な依存関係の追加**

`ts-node` の追加がこの PR の目的(キャッシュ設定の変更)と無関係に見えます。PR の説明には `ts-node` を追加する理由が一切記載されておらず、変更されたソースファイル(`cardDataFetcher.ts`)でも使用されていません。Jules が自動生成したものである可能性が高く、意図しない追加の可能性があります。この依存関係が本当に必要かどうか確認をお願いします。

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Expand Down
2 changes: 1 addition & 1 deletion src/lib/cardDataFetcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ async function getJson<T>(url: string): Promise<{ status: number; data: T | null
try {
response = await fetch(url, {
headers: getHeaders(),
cache: "no-store",
next: { revalidate: 3600 },

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Since the API route calling this function (src/app/api/card/[username]/route.ts) is configured to use the Edge Runtime (export const runtime = "edge"), the Next.js Data Cache (next: { revalidate }) will not be persisted across requests on platforms like Vercel. As a result, this caching optimization will not have any effect in production.

To resolve this and enable the Next.js Data Cache, you should change the API route's runtime to Node.js by removing export const runtime = "edge" (or setting it to "nodejs"), or implement a custom caching layer (e.g., using @upstash/redis which is already a dependency in this project).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 ヒートマップデータの陳腐化

revalidate: 3600 により GitHub リポジトリのデータ(pushed_at を含む)が最大 1 時間キャッシュされます。一方、buildHeatmapFromRepoPushes 内の new Date() は呼び出しごとに「今日」の日付を計算します。その結果、最大 1 時間前の pushed_at データを元にヒートマップが描画されるため、直近 1 時間以内のプッシュが表示に反映されない可能性があります。パフォーマンス向上とのトレードオフとして許容範囲内ですが、ユーザーへの影響を考慮した上で revalidate の値が適切かどうか検討してください。

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/cardDataFetcher.ts
Line: 87

Comment:
**ヒートマップデータの陳腐化**

`revalidate: 3600` により GitHub リポジトリのデータ(`pushed_at` を含む)が最大 1 時間キャッシュされます。一方、`buildHeatmapFromRepoPushes` 内の `new Date()` は呼び出しごとに「今日」の日付を計算します。その結果、最大 1 時間前の `pushed_at` データを元にヒートマップが描画されるため、直近 1 時間以内のプッシュが表示に反映されない可能性があります。パフォーマンス向上とのトレードオフとして許容範囲内ですが、ユーザーへの影響を考慮した上で `revalidate` の値が適切かどうか検討してください。

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

signal: controller.signal,
});
Comment on lines 85 to 89

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 signal オプションとキャッシュの相互作用

signalAbortController)と next: { revalidate } を同時に指定した場合、タイムアウトによってリクエストがアボートされると、その応答はキャッシュに保存されません。通常の成功レスポンスはキャッシュされるため、大きな問題ではありませんが、高負荷時に連続してタイムアウトが発生すると、1 時間のキャッシュ効果が得られず、常に GitHub API へのリクエストが発生し続ける可能性があります。現在の動作で問題ない場合は許容範囲ですが、意図した動作であることを確認してください。

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/cardDataFetcher.ts
Line: 85-89

Comment:
**`signal` オプションとキャッシュの相互作用**

`signal``AbortController`)と `next: { revalidate }` を同時に指定した場合、タイムアウトによってリクエストがアボートされると、その応答はキャッシュに保存されません。通常の成功レスポンスはキャッシュされるため、大きな問題ではありませんが、高負荷時に連続してタイムアウトが発生すると、1 時間のキャッシュ効果が得られず、常に GitHub API へのリクエストが発生し続ける可能性があります。現在の動作で問題ない場合は許容範囲ですが、意図した動作であることを確認してください。

How can I resolve this? If you propose a fix, please make it concise.

} catch (error) {
Expand Down
Loading