fix: メモリリークを修正#70
Conversation
- diffCacheRef に LRU エビクション(上限50エントリ)を追加し、無制限なキャッシュ増大を防止 - ページネーショントークン履歴に上限(50件)を追加し、無制限な配列増大を防止 - 詳細画面からの戻りで重いステート(diffTexts, differences, comments等)をクリア - バックグラウンドの approval/reaction 取得に stale-load ガードを追加し、 ナビゲーション後の不要なステート更新を防止 https://claude.ai/code/session_01AqB9UbJgm39kENE15PkGVq
There was a problem hiding this comment.
Pull request overview
PR詳細画面まわりで発生していたメモリリーク/不要なステート更新を抑止するため、キャッシュ・履歴・画面遷移時のステート管理を改善するPRです。
Changes:
- diff表示キャッシュ(diffCacheRef)に上限50のLRUエビクションを追加
- ページネーショントークン履歴を上限50に制限
- 詳細画面から戻る際に重い詳細ステートをクリアし、バックグラウンドのapproval/reaction取得にstale-loadガードを追加
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/components/PullRequestDetail.tsx | diff表示用キャッシュにLRU(最大50件)を導入して無制限増大を防止 |
| src/app.tsx | pagination履歴の上限設定、詳細画面離脱時の重いステートクリア、approval/reactionのstale-loadガード追加 |
| src/app.test.tsx | evaluateApprovalRules失敗時でもapproveフローが成立することの回帰テストを追加 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| setApprovalEvaluation(null); | ||
| setReactionsByComment(new Map()); | ||
| setCommits([]); | ||
| setCommitDifferences([]); | ||
| setCommitDiffTexts(new Map()); | ||
| setScreen("prs"); |
There was a problem hiding this comment.
Related to clearing commit/diff state on back: handleLoadCommitDiff() isn’t guarded/cancellable, so if the user presses back while a commit diff load is in-flight, the async completion can repopulate commits/commitDifferences/commitDiffTexts after these are cleared here. Please add a stale-load guard (e.g., reuse/increment diffLoadRef or a dedicated commitLoadRef) and bail out before setting these states when the detail screen has been exited.
| function handleBack() { | ||
| if (screen === "detail") { | ||
| // Clear heavyweight detail state to free memory | ||
| setPrDetail(null); | ||
| setPrDifferences([]); | ||
| setCommentThreads([]); | ||
| setDiffTexts(new Map()); |
There was a problem hiding this comment.
handleBack() clears the heavy detail state, but it doesn’t invalidate in-flight/background loads that are guarded by diffLoadRef. Since diffLoadRef.current remains equal to the last loadId, loadDiffTextsInBackground() (and the guarded approvals/reactions) can still update state after navigating back, which can repopulate diffTexts/diffTextStatus etc. Consider incrementing diffLoadRef.current (or setting it to a new token) at the start of the screen === "detail" back path to cancel any ongoing detail loads before clearing state.
- handleBack で diffLoadRef をインクリメントし、進行中のバックグラウンドロードを無効化 - handleLoadCommitDiff に stale-load ガードを追加し、ナビゲーション後の不要なステート更新を防止 https://claude.ai/code/session_01AqB9UbJgm39kENE15PkGVq
ナビゲーション後の不要なステート更新を防止
https://claude.ai/code/session_01AqB9UbJgm39kENE15PkGVq