♻️ Refactor login route into FSD structure#54
Conversation
Restructures the login page following Feature-Sliced Design (FSD) architecture for improved maintainability and testability. Changes: - Split monolithic login route (151 lines) into layered FSD structure - Created LoginPage component in pages layer for composition - Extracted business logic to useLoginView hook in widgets/login-view/model - Created pure presentation LoginView component in widgets/login-view/ui - Maintained 100% feature parity with existing implementation - All functionality preserved: authentication, redirects, error handling, password visibility toggle Architecture improvements: - Clear separation of concerns (presentation, logic, composition) - Testable hook-based business logic - Reusable LoginView widget - Consistent with existing FSD patterns (home, add-videos, player) Verified: - TypeScript type checking passes - All 431 unit tests pass - Production build succeeds - E2E tests confirm correct behavior (login, logout, error handling)
WalkthroughAdds a presentational LoginView and a useLoginView hook, introduces a LoginPage that composes them, and simplifies the login route to render LoginPage and export meta as a MetaFunction. Client-side state, validation, submit handling, and redirect logic moved into the hook. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant Route as LoginRoute
participant Page as LoginPage
participant Hook as useLoginView
participant UI as LoginView
participant Auth as AuthStore
participant Nav as Router/Navigate
U->>Route: GET /login?redirectTo=...
Route->>Page: render LoginPage
Page->>Hook: initialize (state, handlers)
Hook->>Nav: if already authenticated -> navigate(redirectTo or "/")
Page->>UI: render with props from Hook
U->>UI: fills form & submits
UI->>Hook: onSubmit(email, password)
Hook->>Hook: validate, set loading
Hook->>Auth: login(email, password)
alt success
Auth-->>Hook: auth success
Hook->>Nav: navigate(redirectTo or "/")
else failure
Auth-->>Hook: error
Hook->>Hook: set error message
end
Hook->>Hook: set loading = false
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/pages/login/ui/LoginPage.tsx(1 hunks)app/routes/login.tsx(1 hunks)app/widgets/login-view/model/useLoginView.ts(1 hunks)app/widgets/login-view/ui/LoginView.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: TypeScript에서 any를 사용하지 않고 모든 함수에 포괄적인 타입 힌트를 제공한다 (strict TS)
로그에 비밀값을 남기지 않도록 모든 콘솔 출력은 민감정보를 마스킹/제거한다
**/*.{ts,tsx}: Use the ~/* path alias (from tsconfig.json) for internal imports
Prefer explicit interfaces when defining object shapes in TypeScript (strict mode)
Avoid using any in TypeScript (project runs in strict mode)
Use two-space indentation throughout TypeScript/TSX files
Name components, hooks, and providers with PascalCase
Name helpers and state setters with camelCase
File names should mirror their primary export
Files:
app/widgets/login-view/ui/LoginView.tsxapp/widgets/login-view/model/useLoginView.tsapp/routes/login.tsxapp/pages/login/ui/LoginPage.tsx
**/*.{ts,tsx,md,mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
코드 주석과 문서는 영어로 작성한다
Files:
app/widgets/login-view/ui/LoginView.tsxapp/widgets/login-view/model/useLoginView.tsapp/routes/login.tsxapp/pages/login/ui/LoginPage.tsx
app/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all React Router app source under app/ following the repository’s feature-sliced layout
Files:
app/widgets/login-view/ui/LoginView.tsxapp/widgets/login-view/model/useLoginView.tsapp/routes/login.tsxapp/pages/login/ui/LoginPage.tsx
app/widgets/**
📄 CodeRabbit inference engine (AGENTS.md)
Put UI compositions in app/widgets/
Files:
app/widgets/login-view/ui/LoginView.tsxapp/widgets/login-view/model/useLoginView.ts
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Split multiline JSX props across lines per Stylistic ESLint rules
Files:
app/widgets/login-view/ui/LoginView.tsxapp/routes/login.tsxapp/pages/login/ui/LoginPage.tsx
app/pages/**
📄 CodeRabbit inference engine (AGENTS.md)
app/pages/**: Keep route shells in app/pages/ (pages expose route shells only)
Keep routes thin in app/pages/ (delegate business logic to modules/hooks)
Files:
app/pages/login/ui/LoginPage.tsx
🧬 Code graph analysis (4)
app/widgets/login-view/ui/LoginView.tsx (5)
app/components/ui/card.tsx (5)
Card(78-78)CardHeader(78-78)CardTitle(78-78)CardDescription(78-78)CardContent(78-78)app/components/ui/alert.tsx (2)
Alert(66-66)AlertDescription(66-66)app/components/ui/label.tsx (1)
Label(22-22)app/components/ui/input.tsx (1)
Input(21-21)app/components/ui/button.tsx (1)
Button(59-59)
app/widgets/login-view/model/useLoginView.ts (1)
app/stores/auth-store.ts (2)
useIsAuthenticated(102-102)useAuthStore(18-98)
app/routes/login.tsx (4)
app/routes/add-videos.tsx (1)
meta(10-13)app/routes/player.$id.tsx (1)
meta(63-75)app/routes/setup.tsx (1)
meta(11-16)app/pages/login/ui/LoginPage.tsx (1)
LoginPage(4-10)
app/pages/login/ui/LoginPage.tsx (2)
app/widgets/login-view/model/useLoginView.ts (1)
useLoginView(18-93)app/widgets/login-view/ui/LoginView.tsx (1)
LoginView(21-112)
Addresses CodeRabbit review feedback on PR #54. Security fixes: - Validate redirectTo query parameter to prevent open redirect attacks - Only allow same-origin paths starting with single '/' - Reject protocol-relative '//' and external URLs - Applied to both auto-redirect (useEffect) and post-login redirect Accessibility improvements: - Add aria-label to password visibility toggle button - Add title attribute for tooltip support - Screen readers can now announce button purpose Verified: - All quality checks pass (lint, typecheck, test, build) - E2E tests confirm login flow works correctly - No breaking changes to existing functionality
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/widgets/login-view/model/useLoginView.ts (1)
35-38: DRY up redirect sanitization.The same redirect validation block appears twice. Centralizing it keeps the security hardening in one place and avoids accidental drift later.
+const resolveRedirectTo = (params: URLSearchParams) => { + const raw = params.get('redirectTo'); + return raw && raw.startsWith('/') && !raw.startsWith('//') ? raw : '/'; +}; + export function useLoginView(): UseLoginViewResult { @@ - const rawRedirectTo = searchParams.get('redirectTo'); - const redirectTo = rawRedirectTo && rawRedirectTo.startsWith('/') && !rawRedirectTo.startsWith('//') - ? rawRedirectTo - : '/'; - navigate(redirectTo, { replace: true }); + navigate(resolveRedirectTo(searchParams), { replace: true }); @@ - const rawRedirectTo = searchParams.get('redirectTo'); - const redirectTo = rawRedirectTo && rawRedirectTo.startsWith('/') && !rawRedirectTo.startsWith('//') - ? rawRedirectTo - : '/'; - navigate(redirectTo, { replace: true }); + navigate(resolveRedirectTo(searchParams), { replace: true });Also applies to: 70-73
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/widgets/login-view/model/useLoginView.ts(1 hunks)app/widgets/login-view/ui/LoginView.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/widgets/login-view/ui/LoginView.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: TypeScript에서 any를 사용하지 않고 모든 함수에 포괄적인 타입 힌트를 제공한다 (strict TS)
로그에 비밀값을 남기지 않도록 모든 콘솔 출력은 민감정보를 마스킹/제거한다
**/*.{ts,tsx}: Use the ~/* path alias (from tsconfig.json) for internal imports
Prefer explicit interfaces when defining object shapes in TypeScript (strict mode)
Avoid using any in TypeScript (project runs in strict mode)
Use two-space indentation throughout TypeScript/TSX files
Name components, hooks, and providers with PascalCase
Name helpers and state setters with camelCase
File names should mirror their primary export
Files:
app/widgets/login-view/model/useLoginView.ts
**/*.{ts,tsx,md,mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
코드 주석과 문서는 영어로 작성한다
Files:
app/widgets/login-view/model/useLoginView.ts
app/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all React Router app source under app/ following the repository’s feature-sliced layout
Files:
app/widgets/login-view/model/useLoginView.ts
app/widgets/**
📄 CodeRabbit inference engine (AGENTS.md)
Put UI compositions in app/widgets/
Files:
app/widgets/login-view/model/useLoginView.ts
🧬 Code graph analysis (1)
app/widgets/login-view/model/useLoginView.ts (1)
app/stores/auth-store.ts (2)
useIsAuthenticated(102-102)useAuthStore(18-98)
1 participant
Summary
Refactors the login page to follow Feature-Sliced Design (FSD) architecture, bringing it in line with the existing FSD patterns used in other pages (home, add-videos, player).
Changes
Architecture Refactoring
New File Structure
Key Improvements
useLoginViewhook (unit testable)LoginViewcomponent can be used independentlyFeature Parity
All existing functionality preserved:
redirectToquery parameter supportTesting
Quality Checks
bun run lint:fix- No new linting errorsbun run typecheck- All TypeScript types validbun run test- All 431 tests passingbun run build- Production build successfulE2E Testing (Playwright)
Related
Part of the ongoing FSD architecture migration:
Ready to merge - All checks passed, E2E tested, fully backward compatible.
Summary by CodeRabbit
New Features
Refactor