[Feat]web router로직과 native navigation로직 연동 #199
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/web/app/_components/AuthGuard.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "@repo/eslint-config/next.js" to extend from. Please check that the name of the config is correct. The config "@repo/eslint-config/next.js" was referenced from the config file in "/apps/web/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. Walkthrough이 풀 리퀘스트는 주로 웹뷰 감지 및 라우팅 로직과 관련된 여러 파일의 변경 사항을 포함하고 있습니다. 팀 컨트롤러와 모델에서는 팀 순서 업데이트 로직이 수정되었으며, 웹 애플리케이션의 여러 구성 요소에서 Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
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 (
|
🚀 Preview URLBranch: 197-feat-web-router로직과-native-navigation로직-연동 Preview URL: https://codeit.click?pr=199 |
![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
🔭 Outside diff range comments (1)
apps/api/src/controllers/teamController.ts (1)
Line range hint
354-359: 트랜잭션 처리 및 에러 핸들링 개선이 필요합니다.데이터 일관성과 안정성을 위해 다음 개선사항들을 고려해주세요:
- 벌크 업데이트와 조회 작업을 트랜잭션으로 묶어 처리
- 데이터베이스 작업 실패 시의 구체적인 에러 핸들링 추가
- 불필요한 추가 쿼리 최적화
다음과 같이 개선할 수 있습니다:
- await Team.bulkWrite(bulkOperations); - - const reorderedTeams = await Team.find().sort({ order: 1 }).lean(); - - res.status(200).send({ - message: "성공적으로 업데이트되었습니다.", - teams: reorderedTeams, - }); + try { + const session = await Team.startSession(); + let reorderedTeams; + + await session.withTransaction(async () => { + await Team.bulkWrite(bulkOperations, { session }); + reorderedTeams = await Team.find() + .sort({ order: 1 }) + .lean() + .session(session); + }); + + await session.endSession(); + + res.status(200).send({ + message: "성공적으로 업데이트되었습니다.", + teams: reorderedTeams, + }); + } catch (error) { + res.status(500).send({ + message: "팀 순서 업데이트 중 오류가 발생했습니다.", + error: error instanceof Error ? error.message : "알 수 없는 오류", + }); + }
🧹 Nitpick comments (5)
apps/web/app/_hooks/useAppRouter.ts (1)
4-4: 웹뷰 감지 로직이 개선되었습니다.웹뷰와 웹 실행에 대한 주석이 명확하게 추가되었고,
useDetectWebView로의 마이그레이션이 잘 이루어졌습니다. 다만 한 가지 제안사항이 있습니다.주석에 각 실행 경로에서 어떤 동작이 발생하는지 더 자세히 설명하면 좋을 것 같습니다:
- // web view 실행 + // web view 실행: 네이티브에 라우팅 이벤트 전달 - // web 실행 + // web 실행: Next.js 라우터로 페이지 전환Also applies to: 11-11, 15-24
apps/web/app/settings/_components/SettingButtons.tsx (1)
10-10: 함수 이름을 더 명확하게 변경하는 것이 좋겠습니다.
handleClick은 너무 일반적인 이름입니다. 함수의 구체적인 목적을 나타내는 이름을 사용하는 것이 좋습니다.- const handleClick = (component: ModalComponentType<SettingsModalProps>): void => { + const handleSettingsModalOpen = (component: ModalComponentType<SettingsModalProps>): void => { openModal(component); }; // ... onClick={() => { - handleClick(component); + handleSettingsModalOpen(component); }}Also applies to: 23-23
apps/web/app/dashboard/_components/EmptyState.tsx (1)
25-36: 웹뷰 분기 처리가 잘 구현되었습니다.Button 컴포넌트의 조건부 렌더링이 깔끔하게 구현되었습니다. 다만 몇 가지 개선사항을 제안드립니다:
- 버튼 variant를 상수로 추출하면 좋을 것 같습니다:
const EMPTY_STATE_BUTTON_VARIANT = "Secondary" as const;
- 웹뷰/웹 분기에 따른 props를 별도 객체로 분리하면 가독성이 개선될 것 같습니다:
const buttonProps = isWebView ? { as: 'button' as const, type: 'button' as const, onClick: handleButtonClick, } : { as: Link, href: PAGE_NAME.MEETINGS, }; return ( <Button variant={EMPTY_STATE_BUTTON_VARIANT} {...buttonProps} > 미팅 잡기 </Button> );apps/web/app/_hooks/useDetectWebView.ts (1)
23-31: iOS 웹뷰 감지 로직 개선이 필요합니다.현재 Safari 감지 로직이 불완전할 수 있습니다. Safari 버전에 따라 정확도가 달라질 수 있으며, 일부 엣지 케이스를 놓칠 수 있습니다.
다음과 같이 개선해보세요:
-const isSafari = userAgent.includes("Safari") || /Version\/[\d.]+.*Safari/.test(userAgent); +const isSafari = /Version\/[\d.]+.*Safari/.test(userAgent) && !userAgent.includes("Chrome");apps/web/components/Gnb/GnbMenu.tsx (1)
Line range hint
34-57: 웹뷰 조건부 렌더링 로직 개선이 필요합니다.현재 구현에서는 웹뷰 상태에 따라 Link와 button 사이를 전환하고 있습니다. 이 부분을 더 명확하게 분리하여 관리하면 좋을 것 같습니다.
다음과 같은 방식으로 리팩토링을 고려해보세요:
-as={isWebView ? "button" : Link} -{...(isWebView - ? { - type: "button", - onClick: () => { - handleButtonClick(href); - }, - } - : { - href, - })} +{...getNavigationProps({ isWebView, href, handleButtonClick })}그리고 별도의 유틸리티 함수를 추가하세요:
interface NavigationProps { isWebView: boolean; href: string; handleButtonClick: (path: string) => void; } const getNavigationProps = ({ isWebView, href, handleButtonClick }: NavigationProps) => { if (isWebView) { return { as: "button", type: "button", onClick: () => handleButtonClick(href), }; } return { as: Link, href, }; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/api/src/controllers/teamController.ts(1 hunks)apps/api/src/models/teamModel.ts(1 hunks)apps/web/app/_components/AuthGuard.tsx(2 hunks)apps/web/app/_hooks/useAppRouter.ts(2 hunks)apps/web/app/_hooks/useDetectWebView.ts(1 hunks)apps/web/app/_hooks/useWebView.ts(0 hunks)apps/web/app/dashboard/_components/EmptyState.tsx(2 hunks)apps/web/app/settings/_components/SettingButtons.tsx(2 hunks)apps/web/components/Gnb/GnbMenu.tsx(2 hunks)apps/web/next.config.mjs(0 hunks)
💤 Files with no reviewable changes (2)
- apps/web/next.config.mjs
- apps/web/app/_hooks/useWebView.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: preview
🔇 Additional comments (3)
apps/web/app/_hooks/useDetectWebView.ts (2)
3-9: 인터페이스 정의가 명확하고 잘 구조화되어 있습니다.각 플랫폼별 상태를 명확하게 구분하여 타입 안정성을 보장합니다.
20-43: useEffect 의존성 배열 최적화가 필요합니다.
useEffect내부에서 사용되는 모든 외부 의존성이 의존성 배열에 포함되어 있는지 확인이 필요합니다. 현재는 빈 배열이지만,navigator.userAgent가 동적으로 변경될 가능성은 없는지 검토가 필요합니다.apps/web/components/Gnb/GnbMenu.tsx (1)
11-11: useDetectWebView 훅 통합이 적절히 이루어졌습니다.기존 useWebView에서 새로운 useDetectWebView로의 마이그레이션이 올바르게 수행되었습니다.
Also applies to: 34-34
| import { redirect, useRouter } from "next/navigation"; | ||
| import { useAuthStore } from "@/app/store/useAuthStore"; | ||
| import { useWebView } from "../_hooks/useWebView"; | ||
| import { useDetectWebView } from "../_hooks/useDetectWebView"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
웹뷰 리다이렉트 로직 개선이 필요합니다.
현재 구현에서 몇 가지 개선이 필요한 부분이 있습니다:
- 웹뷰 감지 상태가 변경될 경우를 대비하여
useEffect내부로 리다이렉트 로직을 이동하는 것이 안전합니다:
- if (isWebView) redirect(PAGE_NAME.DASHBOARD);
useEffect(() => {
+ if (isWebView) {
+ router.replace(PAGE_NAME.DASHBOARD);
+ return;
+ }
if (isLoggedIn) {
router.replace(PAGE_NAME.DASHBOARD);
}
setIsLoading(false);
- }, [isLoggedIn, router]);
+ }, [isLoggedIn, router, isWebView]);- 주석을 더 명확하게 작성하면 좋을 것 같습니다:
- // 웹뷰 작업 추가
+ // 웹뷰에서 접근 시 대시보드로 즉시 리다이렉트Also applies to: 16-17
|
|
||
| // 웹뷰 작업 추가 | ||
| const { isWebView } = useWebView(); | ||
| const { isWebView } = useDetectWebView(); |
There was a problem hiding this comment.
이 line은 내쪽에서 로직 짜놔서 없애도 돼!
🚀 작업 내용
📝 참고 사항
🖼️ 스크린샷
🚨 관련 이슈 (이슈 번호)
✅ 체크리스트
Summary by CodeRabbit
새로운 기능
useDetectWebView추가EmptyState컴포넌트에서 버튼의 동작을 웹뷰 환경에 따라 조정버그 수정
리팩터링
useWebView후크를 더 상세한useDetectWebView로 대체AuthGuard컴포넌트에서 불필요한 웹뷰 체크 로직 제거SettingButtons컴포넌트의 함수 이름 변경GnbMenu컴포넌트에서 웹뷰 감지 방식 업데이트기타 변경사항