chore: 어드민 vinext app router 전환#524
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Walkthrough
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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. 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/admin/src/components/layout/AdminSidebar.tsx (1)
31-41:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win1) 활성 메뉴에 현재 페이지 상태(
aria-current)를 노출해 주세요.
- 지금은 시각적으로만 활성 메뉴가 구분되고, 스크린리더에는 현재 위치 정보가 전달되지 않습니다.
- 활성 항목에
aria-current="page"를 추가하면 탐색성이 바로 개선됩니다.접근성 보완 패치
- <a key={menu.label} href={menu.to} className={menuClassName}> + <a + key={menu.label} + href={menu.to} + className={menuClassName} + aria-current={isActive ? "page" : undefined} + >🤖 Prompt for 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. In `@apps/admin/src/components/layout/AdminSidebar.tsx` around lines 31 - 41, The active menu link is only indicated visually; update the anchor returned in AdminSidebar (the <a> for each menu using menu, activeMenu and isActive) to include aria-current="page" when isActive is true (leave it unset/undefined otherwise) so screen readers receive the current-page state; ensure the attribute is applied alongside the existing className/menuClassName and menu.icon usage.
🤖 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/admin/src/app/auth/login/page.tsx`:
- Around line 13-23: The redirectIfAuthenticated function currently calls
ensureSessionToken() but doesn't handle exceptions, which can leave the UI stuck
on the loading screen; wrap the ensureSessionToken call in a try/catch inside
redirectIfAuthenticated (preserving the isMounted guard) and in the catch
setCanRenderLogin(true) so the login UI can render on failure, ensuring any
thrown error also causes the same fallback behavior as the no-token path.
In `@apps/admin/src/app/page.tsx`:
- Around line 7-13: The redirectBySession async function calls
ensureSessionToken() but doesn't handle rejections, so if ensureSessionToken
throws the page can hang; wrap the await ensureSessionToken() call in a
try/catch inside redirectBySession (or add a .catch) and on any error perform
window.location.replace("/auth/login") as a safe fallback (optionally log the
error), keeping the existing useEffect and window.location.replace usage.
In `@apps/admin/src/components/features/auth/RequireAdminSession.tsx`:
- Around line 16-26: The checkSession function should handle failures from
ensureSessionToken by wrapping the await call in a try/catch; on any thrown
error (or a falsy token) ensure you bail out cleanly by redirecting to
"/auth/login" and returning, and guard all state updates/redirects with the
isMounted check (e.g., inside checkSession in RequireAdminSession.tsx) so
loading doesn't hang and setIsAuthorized is only called on success.
---
Outside diff comments:
In `@apps/admin/src/components/layout/AdminSidebar.tsx`:
- Around line 31-41: The active menu link is only indicated visually; update the
anchor returned in AdminSidebar (the <a> for each menu using menu, activeMenu
and isActive) to include aria-current="page" when isActive is true (leave it
unset/undefined otherwise) so screen readers receive the current-page state;
ensure the attribute is applied alongside the existing className/menuClassName
and menu.icon usage.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 54193e19-dcf2-4537-b7db-1112b75f5222
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
.gitignoreapps/admin/VINEXT_MIGRATION_REPORT.mdapps/admin/package.jsonapps/admin/src/app/auth/login/page.tsxapps/admin/src/app/bruno/page.tsxapps/admin/src/app/chat-socket/page.tsxapps/admin/src/app/layout.tsxapps/admin/src/app/login/page.tsxapps/admin/src/app/page.tsxapps/admin/src/app/providers.tsxapps/admin/src/app/scores/page.tsxapps/admin/src/components/features/auth/AdminLoginPage.tsxapps/admin/src/components/features/auth/RequireAdminSession.tsxapps/admin/src/components/features/bruno/BrunoApiPageContent.tsxapps/admin/src/components/features/chat-socket/ChatSocketPageContent.tsxapps/admin/src/components/features/scores/ScoresPageContent.tsxapps/admin/src/components/layout/AdminLayout.tsxapps/admin/src/components/layout/AdminSidebar.tsxapps/admin/src/lib/auth/session.tsapps/admin/src/routeTree.gen.tsapps/admin/src/router.tsxapps/admin/src/routes/__root.tsxapps/admin/src/routes/index.tsxapps/admin/src/routes/login.tsxapps/admin/vite.config.ts
💤 Files with no reviewable changes (6)
- apps/admin/src/routes/login.tsx
- apps/admin/src/routes/__root.tsx
- apps/admin/src/routeTree.gen.ts
- apps/admin/src/routes/index.tsx
- apps/admin/src/router.tsx
- apps/admin/src/lib/auth/session.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/admin/src/components/features/mentor-applications/MentorApplicationsPageContent.tsx (1)
627-636: ⚡ Quick win2) 페이지 버튼 렌더링 범위를 제한해 주세요.
totalPages전체를 버튼으로 그리면 페이지 수가 커질 때 렌더 비용과 사용성이 급격히 떨어집니다. 현재 페이지 기준의 윈도우(예: 7개)만 노출하는 방식이 더 안전합니다.변경 예시
+const PAGE_WINDOW = 7; @@ -{Array.from({ length: totalPages }, (_, idx) => ( +{Array.from( + { + length: Math.min(PAGE_WINDOW, totalPages), + }, + (_, offset) => { + const half = Math.floor(PAGE_WINDOW / 2); + const start = Math.max(1, Math.min(page - half, totalPages - PAGE_WINDOW + 1)); + const pageNumber = start + offset; + return pageNumber; + }, +).map((pageNumber) => ( <Button - // biome-ignore lint/suspicious/noArrayIndexKey: pagination buttons are static. - key={idx + 1} - onClick={() => handlePageChange(idx + 1)} - variant={page === idx + 1 ? "default" : "secondary"} + key={pageNumber} + onClick={() => handlePageChange(pageNumber)} + variant={page === pageNumber ? "default" : "secondary"} > - {idx + 1} + {pageNumber} </Button> -))} +))}🤖 Prompt for 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. In `@apps/admin/src/components/features/mentor-applications/MentorApplicationsPageContent.tsx` around lines 627 - 636, Rendering all pages with Array.from({ length: totalPages }, ...) causes performance/usability issues; limit the rendered page buttons to a sliding window (e.g., 7 items) around the current page instead. Update the pagination render logic in MentorApplicationsPageContent to compute a start and end based on page and totalPages (keep first/last pages and add ellipses when clipped), then map that smaller range to Button components calling handlePageChange and using page to set variant; replace the existing Array.from(...) loop with this windowed generation so only the nearby page numbers (plus optional first/last) are rendered.
🤖 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/admin/src/components/features/mentor-applications/MentorApplicationsPageContent.tsx`:
- Around line 339-347: The handlers handleApprove and handleReject currently
call approveMutation.mutateAsync(...) and rejectMutation.mutateAsync(...)
without catching rejections; wrap each mutateAsync call in a try/catch so the
handler consumes and handles thrown errors (e.g., show toast.error with the
error message and return) instead of letting the promise rejection propagate;
update both handleApprove and handleReject to await mutateAsync inside try
blocks and handle failures in the catch blocks, referencing
approveMutation.mutateAsync and rejectMutation.mutateAsync respectively.
---
Nitpick comments:
In
`@apps/admin/src/components/features/mentor-applications/MentorApplicationsPageContent.tsx`:
- Around line 627-636: Rendering all pages with Array.from({ length: totalPages
}, ...) causes performance/usability issues; limit the rendered page buttons to
a sliding window (e.g., 7 items) around the current page instead. Update the
pagination render logic in MentorApplicationsPageContent to compute a start and
end based on page and totalPages (keep first/last pages and add ellipses when
clipped), then map that smaller range to Button components calling
handlePageChange and using page to set variant; replace the existing
Array.from(...) loop with this windowed generation so only the nearby page
numbers (plus optional first/last) are rendered.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ab637754-0000-439d-a17c-11e1c05291b6
📒 Files selected for processing (8)
apps/admin/src/app/auth/login/page.tsxapps/admin/src/app/mentor-applications/page.tsxapps/admin/src/app/page.tsxapps/admin/src/components/features/auth/RequireAdminSession.tsxapps/admin/src/components/features/mentor-applications/MentorApplicationsPageContent.tsxapps/admin/src/components/layout/AdminSidebar.tsxapps/admin/src/lib/api/admin.tsapps/admin/src/types/mentorApplications.ts
✅ Files skipped from review due to trivial changes (1)
- apps/admin/src/app/mentor-applications/page.tsx
Comment on lines
+339
to
+347
| const handleApprove = async (application: MentorApplicationListItem) => { | ||
| const applicationId = getApplicationId(application); | ||
| if (!applicationId) { | ||
| toast.error("신청 ID를 확인할 수 없습니다."); | ||
| return; | ||
| } | ||
|
|
||
| await approveMutation.mutateAsync(applicationId); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
TanStack Query v5에서 onError 콜백을 설정해도 mutateAsync는 실패 시 reject를 유지하나요? 공식 문서/레퍼런스 링크와 함께 확인해 주세요.
💡 Result:
결론: TanStack Query v5에서 useMutation의 onError 콜백을 설정해도, mutateAsync는 실패 시(에러가 발생해 mutation promise가 거절되면) 호출한 쪽에서의 reject(throw/catch)가 유지됩니다. 근거: - useMutation의 onError는 “mutation encounters an error”일 때 실행되며, onError에서 promise를 반환하면 “awaited and resolved before proceeding”된다고 문서에 명시되어 있습니다 [1][2]. 즉 onError는 mutate/mutateAsync의 실패 처리를 “대신 삼켜서 성공으로 바꾸는 것”이 아니라, 실패한 경우의 부수 효과(콜백)를 제공하는 역할입니다. - 반대로 mutateAsync는 “promise which will resolve on success or throw on an error”라고 문서/가이드 예시에서 설명합니다 [3]. 따라서 mutateAsync를 await/then 하는 호출자는 실패 시 reject(=throw)가 그대로 발생합니다. - useMutation reference에서도 mutateAsync는 “Similar to mutate but returns a promise which can be awaited”라고만 정의되며 [1][2], onError가 reject의 전파를 중단/무효화한다는 언급은 없습니다. 실패 시 호출자가 catch를 해야 한다는 패턴(가이드의 try/catch 예시)도 존재합니다 [3]. 참고(동작 확인을 위한 코드 패턴): try { await mutation.mutateAsync(vars) // 실패하면 여기서 throw/reject } catch (e) { // 호출자에서 reject 처리 } 그리고 동시에 useMutation({ onError })는 실패 시 onError 콜백도 함께 실행됩니다 [1][2][3].
Citations:
- 1: https://github.com/tanstack/query/blob/main/docs/framework/react/reference/useMutation.md
- 2: https://tanstack.com/query/v5/docs/framework/react/reference/useMutation
- 3: https://tanstack.com/query/latest/docs/framework/react/guides/mutations
TanStack Query v5 mutateAsync 실패 시 예외 전파를 핸들러에서 종료해 주세요
TanStack Query v5에서 mutateAsync는 실패 시 호출자에서 throw/reject가 그대로 유지되고, onError는 콜백을 실행하는 역할이라 승인/반려 핸들러 쪽에서 try/catch로 Promise 거절을 마무리하는 편이 안전합니다. (예: https://tanstack.com/query/v5/docs/framework/react/reference/useMutation , https://tanstack.com/query/latest/docs/framework/react/guides/mutations)
- 승인(
handleApprove)에서approveMutation.mutateAsync(...)에try/catch추가 - 반려(
handleReject)에서rejectMutation.mutateAsync(...)에try/catch추가
변경 예시
const handleApprove = async (application: MentorApplicationListItem) => {
const applicationId = getApplicationId(application);
if (!applicationId) {
toast.error("신청 ID를 확인할 수 없습니다.");
return;
}
- await approveMutation.mutateAsync(applicationId);
+ try {
+ await approveMutation.mutateAsync(applicationId);
+ } catch {
+ // onError에서 사용자 피드백 처리됨
+ }
};
@@
const handleReject = async (mentorApplicationId: string) => {
const normalizedReason = rejectReason.trim();
if (!normalizedReason) {
toast.error("거절 사유를 입력해주세요.");
return;
}
- await rejectMutation.mutateAsync({ mentorApplicationId, rejectedReason: normalizedReason });
+ try {
+ await rejectMutation.mutateAsync({ mentorApplicationId, rejectedReason: normalizedReason });
+ } catch {
+ // onError에서 사용자 피드백 처리됨
+ }
};📝 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.
Suggested change
| const handleApprove = async (application: MentorApplicationListItem) => { | |
| const applicationId = getApplicationId(application); | |
| if (!applicationId) { | |
| toast.error("신청 ID를 확인할 수 없습니다."); | |
| return; | |
| } | |
| await approveMutation.mutateAsync(applicationId); | |
| }; | |
| const handleApprove = async (application: MentorApplicationListItem) => { | |
| const applicationId = getApplicationId(application); | |
| if (!applicationId) { | |
| toast.error("신청 ID를 확인할 수 없습니다."); | |
| return; | |
| } | |
| try { | |
| await approveMutation.mutateAsync(applicationId); | |
| } catch { | |
| // onError에서 사용자 피드백 처리됨 | |
| } | |
| }; |
🤖 Prompt for 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.
In
`@apps/admin/src/components/features/mentor-applications/MentorApplicationsPageContent.tsx`
around lines 339 - 347, The handlers handleApprove and handleReject currently
call approveMutation.mutateAsync(...) and rejectMutation.mutateAsync(...)
without catching rejections; wrap each mutateAsync call in a try/catch so the
handler consumes and handles thrown errors (e.g., show toast.error with the
error message and return) instead of letting the promise rejection propagate;
update both handleApprove and handleReject to await mutateAsync inside try
blocks and handle failures in the catch blocks, referencing
approveMutation.mutateAsync and rejectMutation.mutateAsync respectively.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
관련 이슈
작업 내용
apps/admin라우팅을 TanStack Start/TanStack Router 구조에서 Vinext 호환src/appApp Router 구조로 전환했습니다.components/features하위 client component로 분리하고,layout,providers, page segment를 추가했습니다.RouterProvider,routeTree.gen.ts,src/routes/**, TanStack Router/Start/Nitro 의존성을 제거했습니다.vinext dev/build/start스크립트와 Vinext Vite 설정을 적용했습니다.apps/admin/VINEXT_MIGRATION_REPORT.md에 라우트 매핑과 검증 결과를 정리했습니다.특이 사항
22.x가 아니라v23.10.0이라 engine warning이 출력됩니다.@tailwindcss/vite는 Vite 8 peer range를 아직 공식 포함하지 않아 peer warning이 출력되지만, admin build와 라우트 QA는 통과했습니다.Unknown으로 표시됩니다. 현재 Vinext 정적 분석 한계 안내이며 빌드는 성공합니다./,/auth/login,/scores등 API client import 라우트는VITE_API_SERVER_URL이 없으면 기존 로직에 의해 500이 나므로, 수동 QA는.env.example기준 예시값을 주입해 진행했습니다.빌드 시간 검수
v22.20.0, 산출물 삭제 후 cold build 5회 반복origin/main7b6628e8,pnpm --filter @solid-connect/admin build906207b6,pnpm --filter @solid-connect/admin build6.596s→ Vinext1.871s로 약71.6%단축, 약3.53x빠름6.050s→ Vinext1.740s로 약71.2%단축, 약3.48x빠름5.426s, Vinext1.713s.output3508KB+ Nitro cache168KB, Vinextdist2988KB리뷰 요구사항 (선택)
검증
pnpm --filter @solid-connect/admin lint:checkpnpm --filter @solid-connect/admin typecheckpnpm --filter @solid-connect/admin exec vinext checkpnpm --filter @solid-connect/admin buildVITE_API_SERVER_URL=https://api.example.com VITE_S3_BASE_URL=https://s3.example.com pnpm --filter @solid-connect/admin dev후/,/login,/auth/login,/scores,/bruno,/chat-socketHTTP 200 확인