[Feat] 라우팅 발생 시 웹뷰 여부에 따른 별도 핸들링 구현 #196
Conversation
|
Caution Review failedThe pull request is closed. Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
apps/mobile/app/_layout.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "expo" to extend from. Please check that the name of the config is correct. The config "expo" was referenced from the config file in "/apps/mobile/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. 개요워크스루이 풀 리퀘스트는 모바일 및 웹 애플리케이션의 광범위한 리팩토링과 구조 변경을 포함합니다. 주요 변경 사항은 새로운 라우팅 아키텍처, WebView 통합, 컴포넌트 재구성, 의존성 업데이트 등을 포함합니다. 모바일 앱의 탐색 방식을 완전히 변경하고, 웹과 모바일 간의 통합을 강화하는 데 중점을 두고 있습니다. 변경 사항
시퀀스 다이어그램sequenceDiagram
participant WebView
participant MobileApp
participant WebApp
WebView->>MobileApp: 라우팅 메시지 전송
MobileApp->>MobileApp: 메시지 파싱
MobileApp->>WebApp: 네비게이션 액션 처리
WebApp-->>MobileApp: 응답
MobileApp->>WebView: 화면 업데이트
관련 가능성이 있는 PR
제안된 리뷰어
시 (코드래빗의 관점에서)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (11)
apps/web/app/settings/_components/modals/ManageTeamModal.tsx (1)
Line range hint
1-62: 전반적인 구현이 잘 되어있습니다만, 몇 가지 개선사항을 제안드립니다.코드는 전체적으로 잘 구현되어 있으나, 다음과 같은 개선사항들을 고려해보시면 좋을 것 같습니다:
- 폼 유효성 검사 추가
- 에러 핸들링 구현
- 로딩 상태 처리
다음과 같은 개선된 구현을 제안드립니다:
const { handleSubmit, register, setValue, reset } = useCreateForm(); -const { mutate: postCreateTeamMutate } = useCreateTeam(onClose); -const { mutate: updateTeamMutate } = useUpdateTeamName(); +const { mutate: postCreateTeamMutate, isLoading: isCreating } = useCreateTeam(onClose); +const { mutate: updateTeamMutate, isLoading: isUpdating } = useUpdateTeamName(); +const isSubmitting = isCreating || isUpdating; const debouncedSubmit = debounce((teamName: string) => { + if (!teamName.trim()) { + // 에러 처리 로직 추가 + return; + } isCreate ? postCreateTeamMutate({ name: teamName }) : updateTeamMutate({ teamId: _id ?? "", newName: teamName }); reset(); }, 800);그리고 버튼에 로딩 상태를 반영:
-<SettingsModalButton type="submit">{buttonText}</SettingsModalButton> +<SettingsModalButton type="submit" disabled={isSubmitting}> + {isSubmitting ? "처리중..." : buttonText} +</SettingsModalButton>apps/mobile/constants/routes.ts (1)
3-10: 라우트 상수의 타입 안전성을 개선할 수 있습니다.상수 객체에 타입을 추가하면 타입 안전성이 향상되고 자동완성 기능을 더 잘 활용할 수 있습니다.
다음과 같은 개선을 제안드립니다:
+export type RouteKeys = 'HOME' | 'DASHBOARD' | 'MEETINGS' | 'SEATS' | 'SETTINGS' | 'NOT_FOUND'; +export type Routes = Record<RouteKeys, string>; -export const ROUTES = { +export const ROUTES: Routes = { HOME: "/index", DASHBOARD: "/dashboard", MEETINGS: "/meetings", SEATS: "/seats", SETTINGS: "/settings", NOT_FOUND: "+not-found", };apps/web/app/utils/sendRouterEvent.ts (1)
3-5: 타입 안전성 개선 필요
SendRouterEventParams인터페이스에 추가 속성을 허용하지 않도록 제한하는 것이 좋습니다.-interface SendRouterEventParams { +interface SendRouterEventParams extends Record<string, never> { path: string; }apps/web/app/(admin)/rooms/page.tsx (1)
Line range hint
4-21: 접근성 및 유지보수성 개선 필요다음과 같은 개선이 필요합니다:
- 섹션과 제목에 적절한 ARIA 레이블이 없습니다.
- 하드코딩된 문자열은 다국어 지원을 위해 상수로 분리되어야 합니다.
- CSS 클래스의 매직 넘버(mt-80, mt-40 등)는 의미 있는 변수나 유틸리티 클래스로 대체되어야 합니다.
export default function RoomsPage(): JSX.Element { return ( - <section className="md:mt-80"> + <section className="md:mt-80" aria-label="회의실 관리 섹션"> <div> - <h1>회의실 관리</h1> + <h1 aria-label="페이지 제목">회의실 관리</h1> </div> - <div className="mt-40"> + <div className="mt-10"> <CategoryList /> </div> <hr className="border-1 my-24 border-solid border-gray-100" /> - <div> + <div aria-label="카테고리 추가 섹션"> <AddCategoryButton /> </div> </section> ); }apps/web/app/_hooks/useWebView.ts (1)
7-21: 상태 관리 및 메모리 누수 방지 개선 필요현재 구현에는 다음과 같은 개선이 필요합니다:
useRef대신useState를 사용하여 더 나은 반응성을 제공할 수 있습니다.- 컴포넌트 언마운트 시 정리(cleanup) 함수가 없습니다.
-export const useWebView = (): UseWebViewResult => { - const isWebViewRef = useRef(false); +export const useWebView = (): UseWebViewResult => { + const [isWebView, setIsWebView] = useState(false); useEffect(() => { - if (isWebViewRef.current) return; + let mounted = true; if (typeof window !== "undefined" && window.ReactNativeWebView) { - isWebViewRef.current = true; + if (mounted) { + setIsWebView(true); + } } + + return () => { + mounted = false; + }; }, []); return { - isWebView: isWebViewRef.current, + isWebView, }; };apps/web/app/_hooks/useAppRouter.ts (1)
5-7: 타입 안전성을 개선해주세요.
UseAppRouterResult인터페이스에서 URL 타입을 더 구체적으로 정의하면 좋을 것 같습니다.+ type ValidUrl = `/${string}` | `https://${string}` | `http://${string}`; interface UseAppRouterResult { - push: (url: string) => void; + push: (url: ValidUrl) => void; }apps/mobile/app/(route)/dashboard.tsx (1)
11-13: 메시지 핸들링 타입 안정성 개선 필요WebView 메시지 처리 시 타입 안정성이 보장되지 않아 런타임 에러가 발생할 수 있습니다.
메시지 타입을 정의하고 타입 가드를 추가하는 것을 추천드립니다:
interface NavigationMessage { type: 'navigation'; payload: { route: string; params?: Record<string, unknown>; }; } const isNavigationMessage = (data: unknown): data is NavigationMessage => { return ( typeof data === 'object' && data !== null && 'type' in data && data.type === 'navigation' && 'payload' in data ); }; const requestOnMessage = (e: WebViewMessageEvent) => { try { const data = JSON.parse(e.nativeEvent.data); if (isNavigationMessage(data)) { handleNavigationActions(e); } } catch (error) { console.warn('Invalid message format:', error); } };apps/mobile/constants/Colors.ts (1)
Line range hint
9-25: 상수 타입 정의 개선 필요색상 상수에 대한 타입 정의가 누락되어 있어 타입 안정성이 보장되지 않습니다.
다음과 같이 타입을 추가하는 것을 추천드립니다:
type ColorScheme = 'light' | 'dark'; interface ColorSet { text: string; background: string; tint: string; icon: string; tabIconDefault: string; tabIconSelected: string; } type Colors = { [key in ColorScheme]: ColorSet; }; export const COLORS: Colors = { // ... existing implementation };apps/web/components/Gnb/GnbMenu.tsx (1)
42-74: 코드 중복 제거 및 상수화 필요네비게이션 아이템 렌더링에서 다음과 같은 개선이 필요합니다:
- 중복된 아이콘 클래스 이름 로직
- 하드코딩된 클래스 이름
다음과 같이 개선하는 것을 제안합니다:
+const getIconClassName = (Icon: typeof GearIcon, isActive: boolean) => + Icon === GearIcon + ? cn("fill-white/60", isActive ? "fill-white" : "fill-white/60") + : cn("stroke-white/60", isActive ? "stroke-white" : "stroke-white/60"); + +const MENU_STYLES = { + button: "h-full justify-start md:h-40 md:w-full md:px-0 md:py-0", + container: "rounded-10 md:w-168 flex w-48 flex-col items-center md:flex-row md:gap-10 md:px-16 md:py-8", + text: "text-12 md:text-16", +} as const; + {NAV_ITEMS.map(({ href, name, icon: Icon }, index) => { const isActive = pathname === href; - const iconClassName = - Icon === GearIcon - ? cn("fill-white/60", isActive ? "fill-white" : "fill-white/60") - : cn("stroke-white/60", isActive ? "stroke-white" : "stroke-white/60"); + const iconClassName = getIconClassName(Icon, isActive); return ( <Button key={name} as={isWebView ? "button" : Link} {...(isWebView ? { type: "button", onClick: () => handleButtonClick(href), } : { href })} - className={cn("h-full justify-start md:h-40 md:w-full md:px-0 md:py-0", index === 3 && "block md:hidden")} + className={cn(MENU_STYLES.button, index === 3 && "block md:hidden")} variant="Text" > <div className={clsx( - "rounded-10 md:w-168 flex w-48 flex-col items-center md:flex-row md:gap-10 md:px-16 md:py-8", + MENU_STYLES.container, isActive ? "md:bg-gray-300" : "md:hover:bg-gray-300", )} > <Icon className={iconClassName} /> - <div className={clsx("text-12 md:text-16", isActive ? "text-white" : "text-white/60")}>{name}</div> + <div className={clsx(MENU_STYLES.text, isActive ? "text-white" : "text-white/60")}>{name}</div> </div> </Button> ); })}apps/web/app/_components/AuthGuard.tsx (1)
15-17: 웹뷰 처리 로직이 적절히 구현되었으나 주석이 영문으로 작성되어 있습니다.웹뷰 환경 확인 및 리다이렉션 처리가 잘 구현되어 있습니다. 다만, "웹뷰 작업 추가" 주석을 한글로 작성한 것처럼 모든 주석은 일관성 있게 한글로 작성하는 것이 좋습니다.
packages/constants/index.ts (1)
53-55: 웹뷰 메시지 타입 상수가 잘 정의되었습니다.라우터 이벤트를 위한 메시지 타입이 명확하게 정의되어 있습니다. 상수의 네이밍과 구조가 일관성 있게 작성되었습니다.
향후 웹뷰 메시지 타입이 추가될 것을 고려하여 객체 형태로 정의한 것이 좋은 접근입니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/mobile/components/__tests__/__snapshots__/ThemedText-test.tsx.snapis excluded by!**/*.snappnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (46)
apps/mobile/app.json(1 hunks)apps/mobile/app/(route)/dashboard.tsx(1 hunks)apps/mobile/app/(route)/index.tsx(1 hunks)apps/mobile/app/(route)/meetings.tsx(1 hunks)apps/mobile/app/(route)/seats.tsx(1 hunks)apps/mobile/app/(route)/settings.tsx(1 hunks)apps/mobile/app/(tabs)/_layout.tsx(0 hunks)apps/mobile/app/(tabs)/explore.tsx(0 hunks)apps/mobile/app/(tabs)/index.tsx(0 hunks)apps/mobile/app/(tabs)/test.tsx(0 hunks)apps/mobile/app/_layout.tsx(1 hunks)apps/mobile/components/Collapsible.tsx(0 hunks)apps/mobile/components/ExternalLink.tsx(0 hunks)apps/mobile/components/HelloWave.tsx(0 hunks)apps/mobile/components/ParallaxScrollView.tsx(0 hunks)apps/mobile/components/__tests__/ThemedText-test.tsx(0 hunks)apps/mobile/constants/Colors.ts(1 hunks)apps/mobile/constants/routes.ts(1 hunks)apps/mobile/hooks/useHandleNavigationActions.ts(1 hunks)apps/mobile/hooks/useThemeColor.ts(1 hunks)apps/mobile/package.json(1 hunks)apps/mobile/tsconfig.json(2 hunks)apps/mobile/utils/getBaseUrl.ts(1 hunks)apps/mobile/utils/parseMessageEvent.ts(1 hunks)apps/web/app/(admin)/members/_components/search/SearchInput.tsx(1 hunks)apps/web/app/(admin)/members/_components/sidepanel/TeamDropdown.tsx(1 hunks)apps/web/app/(admin)/rooms/page.tsx(1 hunks)apps/web/app/_components/AuthGuard.tsx(1 hunks)apps/web/app/_hooks/useAppRouter.ts(1 hunks)apps/web/app/_hooks/useWebView.ts(1 hunks)apps/web/app/dashboard/_components/DashboardProvider.tsx(0 hunks)apps/web/app/meetings/page.tsx(1 hunks)apps/web/app/settings/_components/ProfileInfo.tsx(1 hunks)apps/web/app/settings/_components/ProfileInfoSkeleton.tsx(1 hunks)apps/web/app/settings/_components/modals/ManageTeamModal.tsx(1 hunks)apps/web/app/settings/_components/modals/MembersSettingModal.tsx(1 hunks)apps/web/app/settings/_components/modals/TeamSettingModal.tsx(1 hunks)apps/web/app/utils/sendRouterEvent.ts(1 hunks)apps/web/components/Gnb/GnbMenu.tsx(2 hunks)apps/web/global.d.ts(1 hunks)apps/web/lib/queryKey.ts(1 hunks)apps/web/tsconfig.json(1 hunks)packages/constants/index.ts(1 hunks)packages/ui/src/components/common/Button/index.tsx(1 hunks)packages/ui/src/utils/constants/pageNames.ts(1 hunks)turbo.json(1 hunks)
💤 Files with no reviewable changes (10)
- apps/mobile/app/(tabs)/test.tsx
- apps/mobile/app/(tabs)/explore.tsx
- apps/mobile/app/(tabs)/index.tsx
- apps/web/app/dashboard/_components/DashboardProvider.tsx
- apps/mobile/components/Collapsible.tsx
- apps/mobile/components/tests/ThemedText-test.tsx
- apps/mobile/components/HelloWave.tsx
- apps/mobile/components/ParallaxScrollView.tsx
- apps/mobile/components/ExternalLink.tsx
- apps/mobile/app/(tabs)/_layout.tsx
✅ Files skipped from review due to trivial changes (9)
- apps/web/app/meetings/page.tsx
- apps/web/app/(admin)/members/_components/search/SearchInput.tsx
- apps/web/app/settings/_components/modals/TeamSettingModal.tsx
- apps/web/app/settings/_components/ProfileInfoSkeleton.tsx
- apps/web/lib/queryKey.ts
- apps/web/tsconfig.json
- apps/web/app/settings/_components/ProfileInfo.tsx
- apps/web/app/settings/_components/modals/MembersSettingModal.tsx
- apps/web/app/(admin)/members/_components/sidepanel/TeamDropdown.tsx
🔇 Additional comments (13)
apps/web/app/settings/_components/modals/ManageTeamModal.tsx (1)
12-12: 디렉토리 구조 변경이 잘 반영되었습니다.admin 관련 컴포넌트들의 디렉토리 구조가 그룹화되어 더 체계적으로 정리되었습니다.
apps/web/global.d.ts (1)
1-9: 웹뷰 통신을 위한 타입 선언이 적절히 구현되었습니다.Window 인터페이스에 ReactNativeWebView 타입이 올바르게 정의되어 있으며, postMessage 메서드의 시그니처도 정확합니다.
apps/mobile/hooks/useThemeColor.ts (1)
8-8: 상수 네이밍 컨벤션이 잘 적용되었습니다!
Colors를COLORS로 변경한 것은 JavaScript/TypeScript의 상수 네이밍 컨벤션을 잘 따르고 있습니다.Also applies to: 12-12, 20-20
apps/mobile/app/(route)/index.tsx (1)
7-7: 로그인 상태 처리에 대한 구체적인 계획이 필요합니다.TODO 주석에 명시된 로그인 상태 처리에 대해 다음 사항들을 고려해주세요:
- 로그인 상태 확인 방법
- 비로그인 시 리다이렉션 처리
- 토큰 만료 처리
로그인 상태 처리 로직 구현에 도움이 필요하시다면 말씀해주세요.
apps/mobile/app/_layout.tsx (1)
13-13: TODO 주석 해결 필요스플래시 스크린 구현 여부에 대한 결정이 필요합니다.
스플래시 스크린 구현에 대한 요구사항을 확인하고 구현을 도와드릴까요?
apps/mobile/tsconfig.json (1)
9-10: 타입스크립트 설정이 적절히 개선되었습니다.
- @repo/types/* 경로 매핑이 추가되어 타입 모듈 해결이 개선되었습니다.
- node_modules 제외 설정으로 컴파일 성능이 향상될 것으로 예상됩니다.
Also applies to: 23-23
turbo.json (1)
15-17: 플랫폼별 API URL 환경 변수가 적절히 추가되었습니다.iOS와 Android 플랫폼에 대한 API URL 환경 변수가 명확하게 정의되어 있습니다.
apps/mobile/app.json (2)
23-24: 새로운 React Native 아키텍처 활성화 검토 필요새로운 React Native 아키텍처를 활성화하면 성능이 향상될 수 있지만, 기존 네이티브 모듈과의 호환성 문제가 발생할 수 있습니다. 프로덕션 배포 전에 철저한 테스트가 필요합니다.
다음 사항들을 확인해주세요:
- 모든 네이티브 모듈이 새로운 아키텍처와 호환되는지
- 성능 테스트 진행 여부
- 롤백 계획 수립 여부
Also applies to: 31-32
39-39: expo-font 플러그인 추가에 따른 구현 확인 필요expo-font 플러그인이 추가되었습니다. 이는 커스텀 폰트 사용을 위한 준비로 보입니다.
다음 사항들을 확인해주세요:
- 폰트 파일들이 프로젝트에 추가되었는지
- useFonts 훅을 사용하여 폰트 로딩을 구현했는지
- 폰트 로딩 중 폴백 UI가 구현되었는지
apps/mobile/package.json (3)
23-31: Expo SDK 52 업그레이드에 따른 마이그레이션 검토 필요Expo SDK가 52 버전으로 업그레이드되었으며, 관련 패키지들도 함께 업데이트되었습니다. 주요 변경사항들이 많으므로 마이그레이션 가이드를 참고하여 검토가 필요합니다.
다음 사항들을 확인해주세요:
- Expo SDK 52 마이그레이션 가이드 검토
- Breaking changes 영향도 분석
- 기존 기능들이 정상 동작하는지 테스트
35-41: React Native 및 관련 라이브러리 버전 업그레이드 검토 필요React Native가 0.76.5 버전으로 업그레이드되었으며, 관련 라이브러리들(gesture-handler, reanimated 등)도 함께 업데이트되었습니다.
다음 사항들을 확인해주세요:
- 애니메이션 관련 breaking changes 영향
- 제스처 핸들링 코드 정상 동작 여부
- WebView 업그레이드로 인한 기존 기능 영향도
46-56: 개발 도구 의존성 업데이트 및 추가 검토새로운 React Native CLI가 추가되었고, 타입스크립트 관련 패키지들이 업데이트되었습니다.
다음 사항들을 확인해주세요:
- 새로운 CLI 명령어들과 기존 스크립트와의 호환성
- 타입 정의 파일 업데이트 필요 여부
- Jest 설정 변경 필요 여부
packages/constants/index.ts (1)
52-52: 페이지네이션 상수가 적절히 정의되었습니다.페이지당 20개 항목은 일반적인 페이지네이션 크기로 적절해 보입니다.
| export const parseMessageEvent = (e: WebViewMessageEvent) => { | ||
| return JSON.parse(e.nativeEvent.data); | ||
| }; |
There was a problem hiding this comment.
메시지 파싱 시 에러 처리가 필요합니다.
JSON.parse는 잘못된 형식의 데이터를 받았을 때 예외를 발생시킬 수 있습니다. try-catch 블록을 추가하여 안전하게 처리하는 것이 좋습니다.
다음과 같이 수정하는 것을 제안드립니다:
export const parseMessageEvent = (e: WebViewMessageEvent) => {
- return JSON.parse(e.nativeEvent.data);
+ try {
+ return JSON.parse(e.nativeEvent.data);
+ } catch (error) {
+ console.error('메시지 파싱 중 오류 발생:', error);
+ return null;
+ }
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const parseMessageEvent = (e: WebViewMessageEvent) => { | |
| return JSON.parse(e.nativeEvent.data); | |
| }; | |
| export const parseMessageEvent = (e: WebViewMessageEvent) => { | |
| try { | |
| return JSON.parse(e.nativeEvent.data); | |
| } catch (error) { | |
| console.error('메시지 파싱 중 오류 발생:', error); | |
| return null; | |
| } | |
| }; |
| export const getBaseUrl = () => { | ||
| if (Platform.OS === "ios") return process.env.EXPO_PUBLIC_API_URL_IOS; | ||
| if (Platform.OS === "android") return process.env.EXPO_PUBLIC_API_URL_ANDROID; | ||
| }; |
There was a problem hiding this comment.
환경 변수 및 예외 처리 보완 필요
다음과 같은 개선이 필요합니다:
- iOS/Android 이외의 플랫폼에 대한 기본 반환값이 없습니다.
- 환경 변수가 정의되지 않은 경우에 대한 처리가 없습니다.
다음과 같이 수정하는 것을 제안합니다:
export const getBaseUrl = () => {
- if (Platform.OS === "ios") return process.env.EXPO_PUBLIC_API_URL_IOS;
- if (Platform.OS === "android") return process.env.EXPO_PUBLIC_API_URL_ANDROID;
+ if (Platform.OS === "ios") {
+ if (!process.env.EXPO_PUBLIC_API_URL_IOS) {
+ throw new Error("EXPO_PUBLIC_API_URL_IOS is not defined");
+ }
+ return process.env.EXPO_PUBLIC_API_URL_IOS;
+ }
+ if (Platform.OS === "android") {
+ if (!process.env.EXPO_PUBLIC_API_URL_ANDROID) {
+ throw new Error("EXPO_PUBLIC_API_URL_ANDROID is not defined");
+ }
+ return process.env.EXPO_PUBLIC_API_URL_ANDROID;
+ }
+ throw new Error(`Unsupported platform: ${Platform.OS}`);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const getBaseUrl = () => { | |
| if (Platform.OS === "ios") return process.env.EXPO_PUBLIC_API_URL_IOS; | |
| if (Platform.OS === "android") return process.env.EXPO_PUBLIC_API_URL_ANDROID; | |
| }; | |
| export const getBaseUrl = () => { | |
| if (Platform.OS === "ios") { | |
| if (!process.env.EXPO_PUBLIC_API_URL_IOS) { | |
| throw new Error("EXPO_PUBLIC_API_URL_IOS is not defined"); | |
| } | |
| return process.env.EXPO_PUBLIC_API_URL_IOS; | |
| } | |
| if (Platform.OS === "android") { | |
| if (!process.env.EXPO_PUBLIC_API_URL_ANDROID) { | |
| throw new Error("EXPO_PUBLIC_API_URL_ANDROID is not defined"); | |
| } | |
| return process.env.EXPO_PUBLIC_API_URL_ANDROID; | |
| } | |
| throw new Error(`Unsupported platform: ${Platform.OS}`); | |
| }; |
| export const sendRouterEvent = (params: SendRouterEventParams): void => { | ||
| if (typeof window !== "undefined" && window.ReactNativeWebView) | ||
| window.ReactNativeWebView.postMessage(JSON.stringify({ type: WEBVIEW_MESSAGE_TYPES.ROUTER_EVENT, ...params })); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
예외 처리 및 타입 안전성 개선 필요
메시지 구조체에 대한 타입 정의와 JSON 직렬화 예외 처리가 필요합니다.
+interface RouterEventMessage {
+ type: typeof WEBVIEW_MESSAGE_TYPES.ROUTER_EVENT;
+ path: string;
+}
+
export const sendRouterEvent = (params: SendRouterEventParams): void => {
if (typeof window !== "undefined" && window.ReactNativeWebView)
- window.ReactNativeWebView.postMessage(JSON.stringify({ type: WEBVIEW_MESSAGE_TYPES.ROUTER_EVENT, ...params }));
+ try {
+ const message: RouterEventMessage = {
+ type: WEBVIEW_MESSAGE_TYPES.ROUTER_EVENT,
+ path: params.path,
+ };
+ window.ReactNativeWebView.postMessage(JSON.stringify(message));
+ } catch (error) {
+ console.error("Failed to send router event:", error);
+ }
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const sendRouterEvent = (params: SendRouterEventParams): void => { | |
| if (typeof window !== "undefined" && window.ReactNativeWebView) | |
| window.ReactNativeWebView.postMessage(JSON.stringify({ type: WEBVIEW_MESSAGE_TYPES.ROUTER_EVENT, ...params })); | |
| }; | |
| interface RouterEventMessage { | |
| type: typeof WEBVIEW_MESSAGE_TYPES.ROUTER_EVENT; | |
| path: string; | |
| } | |
| export const sendRouterEvent = (params: SendRouterEventParams): void => { | |
| if (typeof window !== "undefined" && window.ReactNativeWebView) | |
| try { | |
| const message: RouterEventMessage = { | |
| type: WEBVIEW_MESSAGE_TYPES.ROUTER_EVENT, | |
| path: params.path, | |
| }; | |
| window.ReactNativeWebView.postMessage(JSON.stringify(message)); | |
| } catch (error) { | |
| console.error("Failed to send router event:", error); | |
| } | |
| }; |
There was a problem hiding this comment.
export const sendMessageToNative = <T>({ type, data }: Message<T>): void => {
if (window.ReactNativeWebView) {
window.ReactNativeWebView.postMessage(JSON.stringify({ type, data }));
}
};There was a problem hiding this comment.
@bokeeeey Thank you for sharing this pattern. The generic implementation is indeed cleaner and more reusable. I'll make note of this approach for future reviews.
✏️ Learnings added
Learnt from: bokeeeey
PR: codeit-internship-group-b/codeit-resources#196
File: apps/web/app/utils/sendRouterEvent.ts:7-10
Timestamp: 2025-01-13T05:18:41.174Z
Learning: For WebView message passing, prefer using a generic function with TypeScript generics:
```ts
export const sendMessageToNative = <T>({ type, data }: Message<T>): void => {
if (window.ReactNativeWebView) {
window.ReactNativeWebView.postMessage(JSON.stringify({ type, data }));
}
};
This pattern provides better type safety and reusability compared to specific message sending functions.
> Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure [review instructions](https://docs.coderabbit.ai/guides/review-instructions). You can manage existing learnings in the [app](https://app.coderabbit.ai/learnings).
</details>
<!-- This is an auto-generated reply by CodeRabbit -->
| export default function SettingsScreen() { | ||
| const baseUrl = getBaseUrl(); | ||
| const handleNavigationActions = useHandleNavigationActions(); | ||
|
|
||
| const requestOnMessage = (e: WebViewMessageEvent) => { | ||
| handleNavigationActions(e); | ||
| }; | ||
| return <WebView className="flex-1" source={{ uri: `${baseUrl}${ROUTES.SETTINGS}` }} onMessage={requestOnMessage} />; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
WebView 구현에 대한 개선 제안
WebView 컴포넌트에 다음과 같은 필수적인 기능들이 누락되어 있습니다:
- 로딩 상태 처리
- 에러 처리
- 네트워크 연결 상태 확인
다음과 같이 개선하는 것을 추천드립니다:
export default function SettingsScreen() {
const baseUrl = getBaseUrl();
const handleNavigationActions = useHandleNavigationActions();
+ const [isLoading, setIsLoading] = useState(true);
const requestOnMessage = (e: WebViewMessageEvent) => {
handleNavigationActions(e);
};
- return <WebView className="flex-1" source={{ uri: `${baseUrl}${ROUTES.SETTINGS}` }} onMessage={requestOnMessage} />;
+ return (
+ <>
+ <WebView
+ className="flex-1"
+ source={{ uri: `${baseUrl}${ROUTES.SETTINGS}` }}
+ onMessage={requestOnMessage}
+ onLoadStart={() => setIsLoading(true)}
+ onLoadEnd={() => setIsLoading(false)}
+ onError={(syntheticEvent) => {
+ const { nativeEvent } = syntheticEvent;
+ console.warn('WebView error: ', nativeEvent);
+ }}
+ />
+ {isLoading && <LoadingSpinner />}
+ </>
+ );Committable suggestion skipped: line range outside the PR's diff.
| import WebView, { WebViewMessageEvent } from "react-native-webview"; | ||
|
|
||
| import { ROUTES } from "@/constants/routes"; | ||
| import { useHandleNavigationActions } from "@/hooks/useHandleNavigationActions"; | ||
| import { getBaseUrl } from "@/utils/getBaseUrl"; | ||
|
|
||
| export default function MeetingsScreen() { | ||
| const baseUrl = getBaseUrl(); | ||
| const handleNavigationActions = useHandleNavigationActions(); | ||
|
|
||
| const requestOnMessage = (e: WebViewMessageEvent) => { | ||
| handleNavigationActions(e); | ||
| }; | ||
|
|
||
| return <WebView className="flex-1" source={{ uri: `${baseUrl}${ROUTES.MEETINGS}` }} onMessage={requestOnMessage} />; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
WebView 로직 재사용성 개선 필요
여러 라우트 파일에서 동일한 WebView 로직이 반복되고 있습니다. 이는 유지보수성을 저하시키고 코드 중복을 야기합니다.
공통 WebView 래퍼 컴포넌트를 생성하여 코드 중복을 제거하는 것을 추천드립니다:
// components/common/AppWebView.tsx
interface AppWebViewProps {
route: string;
onMessage?: (event: WebViewMessageEvent) => void;
}
export function AppWebView({ route, onMessage }: AppWebViewProps) {
const baseUrl = getBaseUrl();
const [isLoading, setIsLoading] = useState(true);
return (
<>
<WebView
className="flex-1"
source={{ uri: `${baseUrl}${route}` }}
onMessage={onMessage}
onLoadStart={() => setIsLoading(true)}
onLoadEnd={() => setIsLoading(false)}
onError={(syntheticEvent) => {
const { nativeEvent } = syntheticEvent;
console.warn('WebView error: ', nativeEvent);
}}
/>
{isLoading && <LoadingSpinner />}
</>
);
}이후 각 스크린에서 다음과 같이 사용할 수 있습니다:
export default function MeetingsScreen() {
const handleNavigationActions = useHandleNavigationActions();
return <AppWebView route={ROUTES.MEETINGS} onMessage={handleNavigationActions} />;
}| const handleNavigationActions = (e: WebViewMessageEvent) => { | ||
| const parsedMessage = parseMessageEvent(e); | ||
| if (!parsedMessage || parsedMessage.type !== WEBVIEW_MESSAGE_TYPES.ROUTER_EVENT) return; | ||
|
|
||
| const { path } = parsedMessage; | ||
| if (pathname === path) return; | ||
|
|
||
| const action = path === "back" ? StackActions.pop(1) : StackActions.push(`${DIR_NAME}${path}`); | ||
|
|
||
| navigation.dispatch(action); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
메시지 이벤트 처리 로직 개선 필요
메시지 이벤트 처리 시 다음 사항들을 고려해야 합니다:
- 잘못된 경로에 대한 예외 처리
- 네비게이션 상태 관리
- 타입 안전성 강화
다음과 같이 개선하는 것을 제안합니다:
const handleNavigationActions = (e: WebViewMessageEvent) => {
const parsedMessage = parseMessageEvent(e);
if (!parsedMessage || parsedMessage.type !== WEBVIEW_MESSAGE_TYPES.ROUTER_EVENT) return;
const { path } = parsedMessage;
+ if (typeof path !== 'string') {
+ console.error('Invalid path received:', path);
+ return;
+ }
+
if (pathname === path) return;
+ try {
const action = path === "back" ? StackActions.pop(1) : StackActions.push(`${DIR_NAME}${path}`);
navigation.dispatch(action);
+ } catch (error) {
+ console.error('Navigation failed:', error);
+ }
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleNavigationActions = (e: WebViewMessageEvent) => { | |
| const parsedMessage = parseMessageEvent(e); | |
| if (!parsedMessage || parsedMessage.type !== WEBVIEW_MESSAGE_TYPES.ROUTER_EVENT) return; | |
| const { path } = parsedMessage; | |
| if (pathname === path) return; | |
| const action = path === "back" ? StackActions.pop(1) : StackActions.push(`${DIR_NAME}${path}`); | |
| navigation.dispatch(action); | |
| }; | |
| const handleNavigationActions = (e: WebViewMessageEvent) => { | |
| const parsedMessage = parseMessageEvent(e); | |
| if (!parsedMessage || parsedMessage.type !== WEBVIEW_MESSAGE_TYPES.ROUTER_EVENT) return; | |
| const { path } = parsedMessage; | |
| if (typeof path !== 'string') { | |
| console.error('Invalid path received:', path); | |
| return; | |
| } | |
| if (pathname === path) return; | |
| try { | |
| const action = path === "back" ? StackActions.pop(1) : StackActions.push(`${DIR_NAME}${path}`); | |
| navigation.dispatch(action); | |
| } catch (error) { | |
| console.error('Navigation failed:', error); | |
| } | |
| }; |
| <Stack screenOptions={{ headerShown: false, animation: "default" }}> | ||
| <Stack.Screen name={`${DIR_NAME}${ROUTES.HOME}`} /> | ||
| <Stack.Screen name={`${DIR_NAME}${ROUTES.DASHBOARD}`} /> | ||
| <Stack.Screen name={`${DIR_NAME}${ROUTES.MEETINGS}`} /> | ||
| <Stack.Screen name={`${DIR_NAME}${ROUTES.SEATS}`} /> | ||
| <Stack.Screen name={`${DIR_NAME}${ROUTES.SETTINGS}`} /> | ||
| <Stack.Screen name={ROUTES.NOT_FOUND} /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
라우트 상수의 타입 안전성 강화 필요
DIR_NAME과 ROUTES 상수를 사용한 스크린 구성에서 타입 안전성을 개선해야 합니다.
다음과 같이 개선하는 것을 제안합니다:
+type RouteNames = keyof typeof ROUTES;
+
<Stack screenOptions={{ headerShown: false, animation: "default" }}>
- <Stack.Screen name={`${DIR_NAME}${ROUTES.HOME}`} />
- <Stack.Screen name={`${DIR_NAME}${ROUTES.DASHBOARD}`} />
- <Stack.Screen name={`${DIR_NAME}${ROUTES.MEETINGS}`} />
- <Stack.Screen name={`${DIR_NAME}${ROUTES.SEATS}`} />
- <Stack.Screen name={`${DIR_NAME}${ROUTES.SETTINGS}`} />
- <Stack.Screen name={ROUTES.NOT_FOUND} />
+ {(Object.keys(ROUTES) as RouteNames[]).map((route) => (
+ <Stack.Screen
+ key={route}
+ name={route === 'NOT_FOUND' ? ROUTES[route] : `${DIR_NAME}${ROUTES[route]}`}
+ />
+ ))}
</Stack>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Stack screenOptions={{ headerShown: false, animation: "default" }}> | |
| <Stack.Screen name={`${DIR_NAME}${ROUTES.HOME}`} /> | |
| <Stack.Screen name={`${DIR_NAME}${ROUTES.DASHBOARD}`} /> | |
| <Stack.Screen name={`${DIR_NAME}${ROUTES.MEETINGS}`} /> | |
| <Stack.Screen name={`${DIR_NAME}${ROUTES.SEATS}`} /> | |
| <Stack.Screen name={`${DIR_NAME}${ROUTES.SETTINGS}`} /> | |
| <Stack.Screen name={ROUTES.NOT_FOUND} /> | |
| type RouteNames = keyof typeof ROUTES; | |
| <Stack screenOptions={{ headerShown: false, animation: "default" }}> | |
| {(Object.keys(ROUTES) as RouteNames[]).map((route) => ( | |
| <Stack.Screen | |
| key={route} | |
| name={route === 'NOT_FOUND' ? ROUTES[route] : `${DIR_NAME}${ROUTES[route]}`} | |
| /> | |
| ))} | |
| </Stack> |
🚀 작업 내용
📝 참고 사항
🖼️ 스크린
2025-01-13.12.58.09.mov
🚨 관련 이슈 (이슈 번호)
✅ 체크리스트
Summary by CodeRabbit
다음은 릴리즈 노트입니다:
새로운 기능
개선 사항
변경 사항
버그 수정