Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughApp.tsx에서 인라인 Kakao 지도 초기화/오버레이/이벤트/ RN WebView 후킹 및 수동 설정 버튼을 제거하고 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App as App.tsx
participant MC as MapContainer
participant HK as useKakaoMap
participant EH as useMapEventHandlers
participant MO as useMapOverlays
participant U as mapOverlays
User->>App: 애플리케이션 로드
App->>MC: <MapContainer mapId> 렌더
MC->>HK: init(mapId, toast)
HK-->>MC: map 인스턴스 반환
MC->>EH: 이벤트/메시지 핸들러 등록
MC->>MO: overlays 마운트(map, busStops, buses)
MO->>U: createBusStopOverlays(map, busStops)
MO->>U: createBusOverlays(map, buses)
U-->>MO: OverlayHandle[]
MO-->>MC: overlays 적용
Note over HK,MC: RN WebView 브리지 + MAP_READY postMessage
sequenceDiagram
autonumber
actor User
participant BSP as BusStopsPanel
participant BS as BusStops
participant Ctx as MapContainer Context
participant Map as Kakao Map
User->>BS: 정류장 클릭
BS-->>BSP: onSelect(stop)
BSP->>Ctx: moveToLocation(lat, lng)
Ctx->>Map: setCenter(lat, lng)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
src/hooks/useMapEventHandlers.ts (1)
15-22: 메시지 핸들러가 수신한 메시지를 처리하지 않습니다.현재
messageHandler함수가 origin 검증만 수행하고 실제 메시지 처리 로직이 없습니다. 이벤트 데이터를 처리하거나 특정 동작을 수행해야 한다면 구현이 필요합니다.TODO 주석이나 향후 구현 예정인 기능이 있다면 이슈로 등록하시겠습니까?
src/components/MapContainer.tsx (1)
18-18: 불필요한 배열 복사를 제거하세요.
busStops와buses는 이미 배열이므로 spread 연산자를 사용한 복사가 불필요합니다.- useMapOverlays(map, [...busStops], [...buses]); + useMapOverlays(map, busStops, buses);src/hooks/useKakaoMap.ts (2)
50-50: 전역 변수 사용에 대한 타입 안전성을 개선하세요.이전 PR 리뷰에서 학습한 내용에 따르면, TypeScript에서
any타입 사용을 피하고 더 타입 안전한 코드를 선호하신다고 알고 있습니다.window.map과 같은 전역 변수 사용 시 타입 정의를 추가하는 것이 좋겠습니다.global.d.ts 파일을 생성하여 window 객체의 커스텀 속성들에 대한 타입을 정의하세요:
// global.d.ts interface Window { map?: any; // Kakao Map 인스턴스 타입이 있다면 더 구체적으로 정의 kakao?: { maps: { LatLng: new (lat: number, lng: number) => any; Map: new (container: HTMLElement, options: any) => any; CustomOverlay: new (options: any) => any; load: (callback: () => void) => void; }; }; __moveFromRN?: (lat: number, lng: number) => void; __pendingMove?: { lat: number; lng: number } | null; __onMapReady?: () => void; __panAnimationId?: number; ReactNativeWebView?: { postMessage: (message: string) => void; }; }
123-144: 스크립트 이벤트 핸들러 중복 할당 가능성이 있습니다.이미 존재하는 스크립트 엘리먼트에 대해
onload와onerror핸들러를 재할당하는 것은 예상치 못한 동작을 일으킬 수 있습니다. 스크립트가 이미 로드된 경우를 더 명확하게 처리하는 것이 좋겠습니다.if (script) { - if (!window.kakao?.maps?.load) { - script.onload = () => { - window.kakao.maps.load(initMap); - }; - script.onerror = () => { - try { - toast({ - title: "지도 로드 실패", - description: - "Kakao Maps API 스크립트를 불러오는 데 실패했습니다.", - variant: "destructive", - }); - } catch { - // fallback to console - // eslint-disable-next-line no-console - console.error( - "Kakao Maps API 스크립트를 로드하는데 실패했습니다." - ); - } - }; - } + // 스크립트가 이미 존재하면 로드 상태를 확인 + if (script.readyState === 'complete' || script.readyState === 'loaded') { + if (window.kakao?.maps?.load) { + window.kakao.maps.load(initMap); + } + } else if (!window.kakao?.maps?.load) { + // 아직 로드 중인 경우에만 이벤트 핸들러 설정 + const originalOnload = script.onload; + script.onload = () => { + if (originalOnload && typeof originalOnload === 'function') { + (originalOnload as () => void)(); + } + window.kakao.maps.load(initMap); + }; + // onerror 핸들러도 동일하게 처리... + } return; }src/utils/mapOverlays.ts (4)
29-30: innerHTML 사용 시 보안 고려사항정적 SVG 경로를 사용하고 있어 현재는 XSS 위험이 없지만, DOM API를 사용하여 더 안전하게 구현할 수 있습니다.
- busIconDiv.innerHTML = - '<img src="/ic_busstop.svg" alt="Bus Icon" width="48" height="48" />'; + const img = document.createElement("img"); + img.src = "/ic_busstop.svg"; + img.alt = "Bus Icon"; + img.width = 48; + img.height = 48; + busIconDiv.appendChild(img);
38-38: 타입 캐스팅을 피하고 타입 가드나 assertion을 사용하세요.여러 번의 타입 캐스팅보다 타입 가드를 사용하거나 Kakao Maps 타입 정의를 개선하는 것이 좋습니다.
- (overlay as unknown as { setMap: (m: unknown) => void }).setMap(map); + // 타입 가드를 사용한 안전한 접근 + if ('setMap' in overlay && typeof overlay.setMap === 'function') { + overlay.setMap(map); + }Also applies to: 73-73
16-42: 함수 반환 타입을 명시적으로 선언하세요.TypeScript의 타입 추론을 돕고 API 계약을 명확하게 하기 위해 반환 타입을 명시하는 것이 좋습니다.
export const createBusStopOverlays = ( map: unknown, busStops: BusStop[] -): OverlayHandle[] => { +): ReadonlyArray<OverlayHandle> => {
12-14: OverlayHandle.setMap의 map 파라미터를 구체 타입으로 변경 (kakao.maps.Map | null)src/utils/mapOverlays.ts (12–14): 현재 setMap: (map: unknown) => void; 를 setMap: (map: kakao.maps.Map | null) => void; 로 변경하세요. 프로젝트에 src/types/kakao.d.ts가 존재하고 코드에서 window.kakao.maps.*를 사용하므로 전역 kakao 타입을 사용하면 됩니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/App.tsx(4 hunks)src/components/BusStopsPanel.tsx(1 hunks)src/components/MapContainer.tsx(1 hunks)src/components/SettingsButton.tsx(1 hunks)src/hooks/useKakaoMap.ts(1 hunks)src/hooks/useMapEventHandlers.ts(1 hunks)src/hooks/useMapOverlays.ts(1 hunks)src/utils/mapOverlays.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: KwonDeaGeun
PR: KwonDeaGeun/WhatTheBus-Web#21
File: src/App.tsx:16-23
Timestamp: 2025-09-22T04:45:54.433Z
Learning: KwonDeaGeun은 TypeScript에서 any 타입 사용을 피하고 더 타입 안전한 코드를 선호한다.
🧬 Code graph analysis (5)
src/components/MapContainer.tsx (6)
src/components/ui/use-toast.tsx (1)
useToast(83-89)src/hooks/useKakaoMap.ts (1)
useKakaoMap(12-178)src/hooks/useMapOverlays.ts (1)
useMapOverlays(19-42)src/data/busStops.ts (1)
busStops(7-14)src/data/bus.ts (1)
buses(8-39)src/hooks/useMapEventHandlers.ts (1)
useMapEventHandlers(3-53)
src/hooks/useMapOverlays.ts (3)
src/data/busStops.ts (2)
BusStop(1-5)busStops(7-14)src/data/bus.ts (2)
Bus(1-6)buses(8-39)src/utils/mapOverlays.ts (2)
createBusStopOverlays(16-42)createBusOverlays(44-77)
src/components/BusStopsPanel.tsx (3)
src/data/busStops.ts (2)
BusStop(1-5)busStops(7-14)src/components/BusStops.tsx (1)
BusStops(11-246)src/hooks/useMapMovement.ts (1)
moveToLocation(1-89)
src/App.tsx (4)
src/components/SettingsButton.tsx (1)
SettingsButton(8-38)src/components/MapContainer.tsx (1)
MapContainer(14-26)src/components/Bubble.tsx (1)
Bubble(18-221)src/components/BusStopsPanel.tsx (1)
BusStopsPanel(11-34)
src/utils/mapOverlays.ts (2)
src/data/busStops.ts (2)
BusStop(1-5)busStops(7-14)src/data/bus.ts (2)
Bus(1-6)buses(8-39)
🪛 ast-grep (0.39.5)
src/utils/mapOverlays.ts
[warning] 28-29: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: busIconDiv.innerHTML =
'
'
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 28-29: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: busIconDiv.innerHTML =
''
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
🔇 Additional comments (4)
src/components/SettingsButton.tsx (1)
1-38: LGTM! 설정 버튼 컴포넌트가 잘 분리되었습니다.컴포넌트 분리와 접근성 속성(aria-label, aria-haspopup, aria-expanded) 구현이 잘 되어 있습니다.
src/components/BusStopsPanel.tsx (1)
1-34: LGTM! BusStopsPanel 컴포넌트가 잘 구성되었습니다.컴포넌트가 명확한 책임을 가지고 있으며, props 전달이 적절하게 구현되었습니다.
src/App.tsx (2)
76-104: 컴포넌트 분리가 잘 수행되었습니다!App.tsx에서 지도 관련 로직과 UI 컴포넌트들을 성공적으로 분리했습니다. 이로 인해 코드의 가독성과 유지보수성이 크게 향상되었습니다.
91-96:MapContainer의 children으로Bubble컴포넌트를 전달하는 구조가 적절합니다.지도 컨테이너와 버블 UI의 관계를 명확히 표현하고 있습니다.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/utils/mapOverlays.ts (4)
25-33: 불필요한 이중 캐스팅 제거 및 가독성 개선
setMap호출 때마다 임시 캐스팅하지 말고, 생성 시 한 번만OverlayHandle로 단언하세요.- const overlay = new window.kakao.maps.CustomOverlay({ + const overlay = new window.kakao.maps.CustomOverlay({ position: markerPosition, content: busIconDiv, yAnchor: 1, - }); - (overlay as unknown as { setMap: (m: unknown) => void }).setMap(map); - - return overlay as OverlayHandle; + }) as unknown as OverlayHandle; + overlay.setMap(map); + return overlay;버스 오버레이도 동일하게:
- const busOverlay = new window.kakao.maps.CustomOverlay({ + const busOverlay = new window.kakao.maps.CustomOverlay({ position: busPosition, content: busDiv, yAnchor: 1, - }); - (busOverlay as unknown as { setMap: (m: unknown) => void }).setMap(map); - - return busOverlay as OverlayHandle; + }) as unknown as OverlayHandle; + busOverlay.setMap(map); + return busOverlay;Also applies to: 60-68
49-49: UX: 클릭 동작이 없다면 포인터 커서 제거커서를 포인터로 두면 클릭 가능하다고 오해할 수 있습니다.
- busDiv.style.cursor = "pointer"; + // 클릭 동작을 추가할 때 커서를 설정하세요.
53-53: 대체 텍스트 구체화
shuttleId는 필수 필드이므로 fallback 불필요하며, 더 의미 있는 alt로 개선하세요.- img.alt = bus.shuttleId || "bus"; + img.alt = `셔틀 ${bus.shuttleId}`;
4-6: 타입 개선 제안: Kakao 타입 선언 추가로 unknown/단언 줄이기팀 선호(any 회피)에 맞춰 최소 d.ts를 추가하면 캐스팅을 크게 줄일 수 있습니다.
예시: 새 파일
types/kakao.maps.d.ts추가// types/kakao.maps.d.ts declare namespace kakao.maps { class Map {} class LatLng { constructor(lat: number, lng: number); } class CustomOverlay { constructor(opts: { position: LatLng; content: HTMLElement | string; yAnchor?: number }); setMap(map: Map | null): void; } }그 후 이 파일에서의 시그니처를 다음처럼 조정할 수 있습니다:
-export interface OverlayHandle { - setMap: (map: unknown) => void; -} +export interface OverlayHandle { + setMap: (map: kakao.maps.Map | null) => void; +} -export const createBusStopOverlays = (map: unknown, busStops: BusStop[]): OverlayHandle[] => { ... } +export const createBusStopOverlays = (map: kakao.maps.Map | null, busStops: BusStop[]): OverlayHandle[] => { ... }원하시면 전체 적용 패치를 만들어드리겠습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/hooks/useMapOverlays.ts(1 hunks)src/utils/mapOverlays.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/hooks/useMapOverlays.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: KwonDeaGeun
PR: KwonDeaGeun/WhatTheBus-Web#21
File: src/App.tsx:16-23
Timestamp: 2025-09-22T04:45:54.433Z
Learning: KwonDeaGeun은 TypeScript에서 any 타입 사용을 피하고 더 타입 안전한 코드를 선호한다.
🧬 Code graph analysis (1)
src/utils/mapOverlays.ts (2)
src/data/busStops.ts (2)
busStops(7-14)BusStop(1-5)src/data/bus.ts (2)
buses(8-39)Bus(1-6)
🪛 ast-grep (0.39.5)
src/utils/mapOverlays.ts
[warning] 20-21: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: busIconDiv.innerHTML =
''
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 20-21: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: busIconDiv.innerHTML =
''
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
🔇 Additional comments (1)
src/utils/mapOverlays.ts (1)
1-2: 중복 타입 제거 반영 확인 (굿)
Bus/BusStop를 데이터 모듈에서 import하여 로컬 인터페이스 중복을 해소했습니다.
1 participant
Summary by CodeRabbit
New Features
Refactor
Bug Fixes