feat: タイムラインとマップに路線ID(line_id)フィルターを追加#15
Conversation
- LocationUpdate型にline_idフィールドを追加 - LocationStateにlineIds配列を追加し、受信時に自動収集 - タイムライン: 路線IDによる絞り込みフィルター追加(テキスト検索にも対応) - マップ: 路線IDで軌跡データをフィルタリング可能に - LocationCardにline_idの表示を追加 https://claude.ai/code/session_01Ro6oDyNq7sLWArLvCZ3YEk
📝 WalkthroughWalkthroughルート(路線ID)ベースのフィルタリングを地図とタイムラインに追加。stateと型にline_id/lineIdsを導入し、ライン名取得フックを実装、UI(フィルター、バッジ、コールアウト、LocationCard)と軌跡生成を選択された路線で絞り込むように変更。 Changes
シーケンス図sequenceDiagram
participant User as ユーザー
participant UI as フィルターUI\n(Map/Timeline)
participant State as ロケーション\n状態管理
participant Hook as useLineNames\n(GraphQL cache)
participant Memo as filteredUpdates\n(memoized)
participant Render as コンポーネント\n再描画
User->>UI: 路線を選択
UI->>State: handleRouteSelectでselectedRoutes更新
State->>Memo: selectedRoutes依存で再評価
Memo->>Hook: 必要なlineIdsを要求(表示/コールアウト用名解決)
Hook->>Memo: line名キャッシュを返却/更新通知
Memo->>Render: フィルタ済み軌跡・コールアウトデータを供給
Render->>User: 地図/タイムラインを再表示
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
ポエム
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
フィルターボタン・LocationCard・マップ吹き出しで路線名を表示する。 モジュールレベルキャッシュによりアプリ全体でAPIリクエストを共有。 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/__tests__/location-store.test.ts (1)
28-70:⚠️ Potential issue | 🟡 Minorテスト用reducerに
lineIdsの処理が欠落しています。AI summaryによると、実際の
lib/location-store.tsxではADD_UPDATE時にlineIdsを更新し、CLEAR_UPDATES時にリセットしていますが、このテスト用reducerにはその処理が含まれていません。これにより、lineIdsに関連するバグを検出できません。🐛 reducerにlineIds処理を追加する提案
case "ADD_UPDATE": { const update = action.payload; const newUpdates = enforcePerDeviceLimit( [update, ...state.updates], MAX_UPDATES_PER_DEVICE ); const deviceIds = Array.from( new Set([update.device, ...state.deviceIds]) ); + const lineIds = update.line_id + ? Array.from(new Set([update.line_id, ...state.lineIds])) + : state.lineIds; return { ...state, updates: newUpdates, messageCount: state.messageCount + 1, deviceIds, + lineIds, }; }case "CLEAR_UPDATES": - return { ...state, updates: [], logs: [], messageCount: 0, deviceIds: [] }; + return { ...state, updates: [], logs: [], messageCount: 0, deviceIds: [], lineIds: [] };
🧹 Nitpick comments (6)
lib/__tests__/use-line-names.test.ts (2)
101-108: 非同期待機にsetTimeoutを使用するのは不安定です。
vi.waitForを使用する代わりに固定時間のsetTimeoutを使用すると、CI環境やマシンの負荷によってテストが不安定になる可能性があります。♻️ vi.waitForを使用した改善案
it("fetchエラー時はキャッシュが変更されない", async () => { fetchMock.mockRejectedValueOnce(new Error("Network error")); fetchLineNames(["99999"]); - // エラー処理を待つ - await new Promise((r) => setTimeout(r, 10)); + // エラー処理を待つ(fetchedIdsに追加されることを確認) + await vi.waitFor(() => { + // fetchは失敗するがキャッシュは空のまま + expect(_getCache()).toEqual({}); + }); - expect(_getCache()).toEqual({}); });
133-146: リスナー通知のテストが不完全です。テスト名は「subscribeでリスナーが通知される」ですが、
listener変数を作成した後、実際にsubscribeしておらず、呼び出しを検証していません。このテストは現在、キャッシュ更新のみを確認しています。♻️ リスナー通知を実際にテストする改善案
it("subscribeでリスナーが通知される", async () => { - const { _resetCache: _, ...mod } = await import("../../hooks/use-line-names"); + // モジュールから subscribe をインポート + const { subscribe } = await import("../../hooks/use-line-names"); _resetCache(); const listener = vi.fn(); + const unsubscribe = subscribe(listener); - // subscribe/getSnapshotを直接テストするためにモジュールのsubscribeを利用 - // fetchLineNamesの内部でnotifyが呼ばれることを間接的にテスト mockFetchResponse([{ id: 11302, nameShort: "山手線" }]); fetchLineNames(["11302"]); await vi.waitFor(() => expect(_getCache()).toHaveProperty("11302")); - // キャッシュが更新されたことで正しい値が入っている - expect(_getCache()["11302"]).toBe("山手線"); + + // リスナーが通知されたことを確認 + expect(listener).toHaveBeenCalled(); + unsubscribe(); });hooks/use-line-names.ts (2)
57-57: エラーを完全に無視するとデバッグが困難になります。
.catch(() => {})はすべてのエラーを握りつぶしています。ネットワークエラーや不正なレスポンスが発生した場合、問題の特定が難しくなります。♻️ 最低限のエラーログを追加する提案
- .catch(() => {}); + .catch((err) => { + if (__DEV__) { + console.warn("[useLineNames] Failed to fetch line names:", err); + } + });
63-65:useEffectの依存配列に配列を渡すと、参照が変わるたびに再実行されます。
lineIdsが呼び出し元で毎レンダリング新しい配列参照として渡される場合(例:[...state.lineIds])、無限ループが発生する可能性があります。現在の使用箇所(state.lineIds)では問題ないかもしれませんが、将来的なリスクがあります。♻️ 配列の内容をJSON化して安定させる案
+import { useEffect, useSyncExternalStore, useMemo } from "react"; export function useLineNames(lineIds: string[]): Record<string, string> { const lineNames = useSyncExternalStore(subscribe, getSnapshot, getSnapshot); + + // 配列参照の変更ではなく、内容の変更でのみ再実行 + const lineIdsKey = useMemo(() => lineIds.join(","), [lineIds]); useEffect(() => { fetchLineNames(lineIds); - }, [lineIds]); + }, [lineIdsKey]); return lineNames; }または、呼び出し元で
useMemoを使用してlineIdsの参照を安定させることもできます。app/(tabs)/timeline.tsx (1)
476-476:formatLineNameヘルパー関数の使用を検討してください。
hooks/use-line-names.tsにはformatLineName関数がエクスポートされています。一貫性のために、インラインのロジックの代わりにこの関数を使用することを検討してください。♻️ formatLineNameを使用する提案
+import { useLineNames, formatLineName } from "@/hooks/use-line-names"; -import { useLineNames } from "@/hooks/use-line-names"; // Line 476 -{lineNames[lineId] ? <><Text style={{ fontWeight: "bold" }}>{lineNames[lineId]}</Text>({lineId})</> : lineId} +{formatLineName(lineId, lineNames)}ただし、現在の実装では路線名部分に
fontWeight: "bold"を適用しているため、formatLineNameの出力形式と完全には一致しません。必要に応じてformatLineNameを拡張するか、現在のインライン実装を維持してください。app/(tabs)/map.tsx (1)
418-418:timeline.tsxと同様にformatLineNameの使用を検討してください。こちらも同じインラインパターンを使用しています。コードの一貫性を保つために、両方のファイルで同じアプローチを採用することをお勧めします。
https://claude.ai/code/session_01Ro6oDyNq7sLWArLvCZ3YEk
Summary by CodeRabbit
新機能
改善
テスト