Skip to content

fix(ui): #576 最終検証の回帰を修正 - #610

Merged
hmjn023 merged 3 commits into
developfrom
fix/issue-576-final-verification
Jul 18, 2026
Merged

fix(ui): #576 最終検証の回帰を修正#610
hmjn023 merged 3 commits into
developfrom
fix/issue-576-final-verification

Conversation

@hmjn023

@hmjn023 hmjn023 commented Jul 18, 2026

Copy link
Copy Markdown
Owner

概要

Issue #576の最終検証で見つかったUI・E2E・リアルタイム接続の回帰を修正します。

変更内容

  • controlled Source dialogのフォーカス復元を修正
  • Upload Media modalの自己追跡effectによるstack overflowを修正
  • Add Media E2Eを実際のfile chooser操作へ修正
  • source event streamをsource ID単位で共有し、画面遷移時の接続枯渇を防止
  • Vite+のworkspace rootとBun package manager設定を修正
  • 検索再訪E2Eへhydration待ちとSSE接続数回帰assertを追加

検証

  • 全workspace typecheck 成功
  • UI unit tests: 51 passed
  • full dev E2E: 53 passed / 4 skipped
  • full production E2E: 53 passed / 4 skipped

Fixes #576

Summary by CodeRabbit

  • 改善
    • 画面遷移やキャッシュ再訪時のイベント購読を共有し、重複した通信を抑制しました。
    • 現在の画面に応じてイベント購読を開始・停止するよう改善しました。
    • メディア追加時のファイル選択とアップロード状態の復元を安定化しました。
    • モーダルを閉じた際、開く前に選択していた要素へフォーカスを戻すようにしました。

@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@hmjn023, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 46 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5b9fa8d0-80c6-41f2-add7-2488ef5021dd

📥 Commits

Reviewing files that changed from the base of the PR and between 9184d79 and 63c5beb.

📒 Files selected for processing (2)
  • apps/server/src/hooks/use-media-source-events.ts
  • apps/server/src/tests/e2e/route-reload.spec.ts
📝 Walkthrough

Walkthrough

イベントトランスポートをルート単位で制御・共有し、購読解除後の遅延解放を追加しました。モーダルのフォーカス復帰と開成時初期化を調整し、関連E2E、Viteルート、パッケージマネージャー設定を更新しました。

Changes

UI・イベント安定化

Layer / File(s) Summary
共有イベントトランスポート
apps/server/src/hooks/use-media-source-events.ts
現在のpathnameとmediaSourceIdが一致する場合のみ購読し、globalThis上でトランスポートを共有します。購読者がなくなった場合はアイドルタイムアウト後に解放します。
モーダル状態管理
packages/ui/src/source-form-modal.tsx, packages/ui/src/upload-media-modal.tsx
SourceFormModalのクローズ時に保存した要素へフォーカスを戻し、UploadMediaModalの初期化を開成時のみに限定します。
E2E検証更新
apps/server/src/tests/e2e/route-reload.spec.ts, apps/server/src/tests/e2e/sources-source-media.responsive.spec.ts
ハイドレーション後の操作、イベント購読リクエスト数、fixtureファイル選択とファイル名復元を検証します。
実行環境設定
apps/server/vite.config.ts, package.json
Viteのrootを__dirnameに固定し、packageManagerをBunに変更します。

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Route as Current route
  participant Hook as use-media-source-events
  participant Transport as createServerTransport
  participant Events as /api/rpc/sources/events
  Route->>Hook: Provide mediaSourceId and pathname
  Hook->>Transport: Register listener
  Transport->>Events: Reuse or create shared subscription
  Events-->>Transport: Emit source event
  Transport-->>Hook: Dispatch to registered handlers
  Hook-->>Transport: Remove listener on unlisten
  Transport->>Transport: Release after idle timeout
Loading

Possibly related PRs

  • hmjn023/solid-imager#595: route-reload.spec.tsでハイドレーション後のイベント購読リクエストを検証する変更と関連します。
  • hmjn023/solid-imager#608: 同じモーダルコンポーネントのフォーム状態管理を変更しています。
  • hmjn023/solid-imager#609: createServerTransportへのmediaSourceId受け渡し経路に関連します。
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 主要変更であるUI/E2E/接続回帰の修正を示しており、タイトルとして十分具体的です。
Linked Issues check ✅ Passed #576のフォーカス復元、モーダル不具合、file chooser化、SSE共有、E2E回帰検証を概ね満たしています。
Out of Scope Changes check ✅ Passed フォーカス、モーダル、SSE、E2E、Vite/Bun設定はいずれも目的に沿っており、明確な脱線はありません。
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-576-final-verification

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 `@apps/server/src/hooks/use-media-source-events.ts`:
- Around line 91-95: Update the event callback in subscribeToEventStream’s
cleanup listener so each handlers listener invocation is wrapped in its own
exception handling, allowing subsequent listeners to receive the event. Log each
caught listener error with the existing logging mechanism and prevent it from
propagating as a stream failure or triggering reconnection.
- Around line 112-118: 制御されていないアイドルSSE接続の蓄積を防ぐため、sharedSourceTransports
のアイドル接続管理を更新してください。各source
IDの接続を30秒保持するだけでなく、保持可能なアイドル接続数を制限し、上限超過時は最古のアイドル接続を即時に shared.cleanup() して
sharedSourceTransports から削除してください。既存の idleTimer による遅延クリーンアップと、handlers
が存在する接続の動作は維持してください。

In `@apps/server/src/tests/e2e/route-reload.spec.ts`:
- Around line 497-503: Update the assertion for apiRequestCountPathSince in the
route reload test to also require at least one /api/rpc/sources/events request,
while preserving the existing upper-bound check of three to verify shared
subscriptions.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 801beb17-3b55-462d-944e-0b94303ed72c

📥 Commits

Reviewing files that changed from the base of the PR and between 617cae1 and 9184d79.

📒 Files selected for processing (7)
  • apps/server/src/hooks/use-media-source-events.ts
  • apps/server/src/tests/e2e/route-reload.spec.ts
  • apps/server/src/tests/e2e/sources-source-media.responsive.spec.ts
  • apps/server/vite.config.ts
  • package.json
  • packages/ui/src/source-form-modal.tsx
  • packages/ui/src/upload-media-modal.tsx

Comment thread apps/server/src/hooks/use-media-source-events.ts
Comment thread apps/server/src/hooks/use-media-source-events.ts
Comment thread apps/server/src/tests/e2e/route-reload.spec.ts
@hmjn023
hmjn023 merged commit d29f159 into develop Jul 18, 2026
1 check passed
@hmjn023
hmjn023 deleted the fix/issue-576-final-verification branch July 18, 2026 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant