Pull Request: Frontend Architecture Refactoring & Stability Improvements#74
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbitリリースノート
Walkthrough
ChangesApp.tsx フック分離リファクタリング
Sequence Diagram(s)sequenceDiagram
rect rgba(173, 216, 230, 0.5)
Note over App,useImageManager: 画像取り込みフロー
App->>useImageManager: handleExifResults(ExifResult[])
useImageManager->>useImageManager: importedImages 差し替え / selectedIndex リセット
useImageManager->>App: imageObj (Image プリロード完了)
App->>App: renderImageToCanvas(imageManager.imageObj, imageManager.exif, ...)
end
rect rgba(255, 218, 185, 0.5)
Note over App,useBackgroundProcessor: バックグラウンド自動保存フロー
WailsEvents->>useBackgroundProcessor: process_file イベント
useBackgroundProcessor->>AppAPI: GetSettings()
AppAPI-->>useBackgroundProcessor: Settings
useBackgroundProcessor->>useBackgroundProcessor: renderImageToCanvas (オフスクリーン)
useBackgroundProcessor->>AppAPI: SaveAutoImage(token)
useBackgroundProcessor->>SaveServer: POST /api/save?token=...
end
rect rgba(144, 238, 144, 0.5)
Note over App,useUpdater: アップデートフロー
useUpdater->>AppAPI: CheckForUpdate()
AppAPI-->>useUpdater: updateState.stage 更新
App->>useUpdater: triggerUpdate()
useUpdater->>AppAPI: TriggerUpdate()
App->>useUpdater: restartApp()
useUpdater->>AppAPI: RestartApp()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/hooks/useBackgroundProcessor.ts`:
- Around line 56-140: The offscreenCanvas created at the beginning of this code
block is never explicitly released after use, which can cause memory leaks when
processing multiple files in the background. After the blob upload completes
(either successfully or with an error) in the toBlob callback, you need to
release the canvas resources by clearing its context and removing references to
it. Add canvas cleanup logic at the end of the toBlob callback, ensuring it runs
in both success and failure paths, similar to how it's handled in the
downloadAllImages function using a finally block pattern.
In `@frontend/src/hooks/useSettingsSync.ts`:
- Around line 36-41: The assignment `setExifRef.current = setExif` is being
performed directly during render, but the PR goal requires moving useRef.current
direct modifications to useLayoutEffect to resolve React Strict Mode violations.
Move the `setExifRef.current = setExif` assignment from the render body into a
new useLayoutEffect hook with `setExif` as a dependency. This ensures
consistency with the pattern used in App.tsx and aligns with the PR's approach
for handling ref updates safely.
In `@frontend/src/hooks/useUpdater.ts`:
- Around line 16-17: The comment above the getEventData helper function has
unnecessary indentation (4 spaces) that doesn't align with module-scope code
conventions. Remove the leading indentation from the comment that precedes the
getEventData function definition so that it aligns with the function declaration
itself at the module scope level.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 80c16f7d-2fd2-47a1-97cf-c01877f797c2
📒 Files selected for processing (7)
frontend/src/App.tsxfrontend/src/hooks/useBackgroundProcessor.tsfrontend/src/hooks/useExport.tsfrontend/src/hooks/useImageManager.tsfrontend/src/hooks/useSettingsSync.tsfrontend/src/hooks/useUpdater.tsfrontend/src/types.ts
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/hooks/useBackgroundProcessor.ts`:
- Around line 103-106: The code in useBackgroundProcessor.ts silently returns
when blob is null without any logging, making it difficult to debug when toBlob
fails due to canvas contamination or other issues. Add a console warning or
logger call in the condition where blob is null (the `if (!blob) return;`
statement) to log the issue before returning, so that debugging becomes easier
when this null case occurs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 519f624a-48fa-4a4a-b0c9-d69a9c401387
📒 Files selected for processing (4)
frontend/src/hooks/useBackgroundProcessor.tsfrontend/src/hooks/useExport.tsfrontend/src/hooks/useSettingsSync.tsfrontend/src/hooks/useUpdater.ts
|
@coderabbitai review |
✅ Action performedReview finished.
|
概要 (Overview)
巨大化していた
App.tsxのモノリシックな状態管理とロジックを解体し、関心事ごとに分離されたカスタムフックへ移行する大規模リファクタリングを実施しました。これに伴い、Reactのライフサイクルに関連する複数の潜在的なバグ(無限ループ、ステートの巻き戻り、メモリリーク等)を発見・修正し、アプリケーションの安定性とパフォーマンスを大幅に向上させています。
主な変更点 (Changes)
1. フックベースのアーキテクチャへの移行
App.tsxに集中していた処理を以下の5つのカスタムフックに分割しました:useImageManager: 読み込み画像のステート(配列)、プレビュー選択、画像個別のメタデータ(EXIF、フレーム色、テキスト色、縦横の向き)の管理。useSettingsSync: Wailsバックエンドとのグローバル設定(自動書き出しデフォルト等)の同期と状態管理。useExport: 単一画像・全画像のバッチエクスポート処理、およびバックエンドへのPOST通信。useBackgroundProcessor: フォルダ監視によるバックグラウンド画像処理イベントの検知と、ユーザーの操作を妨げないサイレント自動保存。useUpdater: アプリケーションの自動アップデート確認・適用ロジックの管理。2. 重大なバグ修正 (Bug Fixes)
setExif)による再レンダリング・ループをuseRefパターンを用いて遮断しました。useRef.currentを書き換えていたアンチパターンをuseLayoutEffect内に移動し、React Concurrent Mode への安全性を確保しました。3. パフォーマンスとメモリ最適化 (Optimizations)
width=0,height=0)と動的ロードしたImage要素(src="")をfinallyブロックで即座に解放し、メモリリークとスパイクを防止しました。useEffectやuseCallbackの依存配列において、オブジェクト全体(globalExifなど)を渡すのをやめ、実際に使用しているプリミティブ値のみを展開して渡すことで、コールバックの不要な再生成を防ぎました。4. ネットワークとエラーハンドリング強化 (Network & Error Handling)
fetch)に対し、AbortControllerを用いて15秒のタイムアウトを設定し、サーバー無応答時にUIが無期限にフリーズする問題に対処しました。AbortErrorを捕捉し、単なる通信エラーではなく「Timeout」として明示的にログを出力するように改善しました。影響範囲 (Impact)
resolved #71