-
Notifications
You must be signed in to change notification settings - Fork 1
refactor: #105 회원가입 로직 리팩토링 #106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe changes centralize authentication-related constants by introducing a new constants module and refactoring code throughout the application to use these constants instead of hardcoded strings. A custom React hook is added for email validation, replacing inline validation logic. Social login URLs are now managed via constants, and related click handlers are unified. The use of state management hooks is streamlined, and effect dependencies are updated for clarity. Overall, the update standardizes key usage, abstracts repeated logic, and improves maintainability. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SignInComponent
participant useCheckEmailHook
participant ConstantsModule
participant LocalStorage
participant OAuthProvider
User->>SignInComponent: Enter email / Click social login
SignInComponent->>useCheckEmailHook: Validate email input
useCheckEmailHook-->>SignInComponent: Return validity and value
SignInComponent->>ConstantsModule: Get ACCESS_TOKEN / OAUTH URLs
SignInComponent->>OAuthProvider: Redirect for social login
OAuthProvider-->>SignInComponent: Return accessToken in URL
SignInComponent->>LocalStorage: Store accessToken using ACCESS_TOKEN
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
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. 🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/app/oauth2/redierct/index.tsx (1)
22-22: Consider adding dependencies to useEffectThe empty dependency array means this effect runs only once on mount, but it uses
accessToken,setLogin, andnavigatewhich should ideally be included in the dependency array.- }, []); + }, [accessToken, setLogin, navigate]);src/app/signin/index.tsx (1)
88-88: Consider abstracting password validationWhile using
isValidEmailfrom the hook is good, the password validation (password.length > 5) is still hardcoded. Consider creating a similar hook or at least using a constant for minimum password length.- state={isValidEmail && password.length > 5} + state={isValidEmail && password.length >= MIN_PASSWORD_LENGTH}Where
MIN_PASSWORD_LENGTHwould be defined in your constants file:// Add to src/utils/constant.ts export const MIN_PASSWORD_LENGTH = 6;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/api/index.ts(3 hunks)src/app/oauth2/redierct/index.tsx(2 hunks)src/app/signin/index.tsx(5 hunks)src/components/layout/header.tsx(2 hunks)src/hooks/useCheckEmail.ts(1 hunks)src/utils/constant.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
src/api/index.ts (1)
src/utils/constant.ts (1)
ACCESS_TOKEN(16-16)
src/components/layout/header.tsx (1)
src/utils/constant.ts (1)
ACCESS_TOKEN(16-16)
src/hooks/useCheckEmail.ts (1)
src/utils/regex.ts (1)
EMAIL_REGEX(2-2)
src/app/oauth2/redierct/index.tsx (2)
src/store/user/index.ts (1)
loginState(3-3)src/utils/constant.ts (1)
ACCESS_TOKEN(16-16)
src/app/signin/index.tsx (3)
src/store/user/index.ts (1)
loginState(3-3)src/hooks/useCheckEmail.ts (1)
useCheckEmail(4-19)src/utils/constant.ts (2)
ACCESS_TOKEN(16-16)OAUTH(16-16)
🔇 Additional comments (19)
src/hooks/useCheckEmail.ts (1)
1-19: Well-structured custom hook for email validation.This is a clean implementation of a custom hook that encapsulates email validation logic. The hook properly manages state, uses memoization with useCallback to prevent unnecessary re-renders, and provides a clear interface for components to use.
Some positive aspects of this implementation:
- Good separation of concerns by extracting email validation logic into a reusable hook
- Proper use of useState for managing both email value and validation state
- Effective memoization of the change handler with useCallback
- Clear typing for the event handler
- Consistent validation using the imported EMAIL_REGEX
src/utils/constant.ts (1)
1-16: Good centralization of constants.The creation of a central constants file is a good practice for maintaining consistent values across the application. The file is well-organized with appropriate comments for each section.
A few notes:
- Using
btoa()for the ACCESS_TOKEN adds a layer of obfuscation which is good for security- The
as constassertion for EMAIL_PURPOSE ensures type safety- Environment variables are properly used for constructing OAuth URLs
src/components/layout/header.tsx (2)
21-21: Good use of centralized constant.Importing the ACCESS_TOKEN constant from the central constants file aligns with the refactoring goal of the PR.
256-256: Consistent use of ACCESS_TOKEN constant.Replacing the hardcoded string with the imported constant improves maintainability and consistency across the codebase.
src/api/index.ts (3)
1-1: Good use of centralized constant.Importing the ACCESS_TOKEN constant from the central constants file aligns with the refactoring goal of the PR.
34-34: Consistent use of ACCESS_TOKEN constant in error handler.Replacing the hardcoded string with the imported constant improves maintainability and consistency across the codebase.
44-44: Consistent use of ACCESS_TOKEN constant in request handler.Replacing the hardcoded string with the imported constant improves maintainability and consistency across the codebase.
src/app/oauth2/redierct/index.tsx (4)
2-3: Good refactoring of imports and state managementThe change to import
ACCESS_TOKENfrom constants and usinguseSetAtominstead ofuseAtomare both good improvements. UsinguseSetAtomis more efficient when you only need to update state without reading it.
12-12: Improved state management approachUsing
useSetAtominstead of destructuring fromuseAtomis more efficient as this component only needs to set the login state, not read it.
16-16: Using centralized constant for token storageGood replacement of hardcoded string with the
ACCESS_TOKENconstant, improving maintainability.
19-20: More user-friendly error messageThe error message is more clear for users when no access token is found.
src/app/signin/index.tsx (8)
15-17: Good architecture improvements with hooks and constantsThe changes to use
useSetAtom, centralized constants (ACCESS_TOKEN,OAUTH), and the customuseCheckEmailhook improve code organization and reusability.
21-22: Good refactoring of state managementUsing
useSetAtomfor login state and delegating email validation to a custom hook simplifies the component and improves separation of concerns.
25-30: Using centralized constant for token handlingGood use of the
ACCESS_TOKENconstant for consistency across the application.
32-34: Well-abstracted social login handlerCreating a dedicated function for social login redirection improves code maintainability and reduces duplication.
39-39: Consistent token constant usageGood use of the
ACCESS_TOKENconstant for token storage.
75-75: Using custom hook's handler improves code simplicityGood refactoring to use the email change handler from the custom hook.
133-133: Good use of constants for social login URLsUsing the
OAUTH.KAKAOconstant with the unified handler improves maintainability.
143-143: Good use of constants for social login URLsUsing the
OAUTH.GOOGLEconstant with the unified handler improves maintainability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/pull_request_template.md (2)
1-1: Use proper Markdown headings instead of emphasisThe template currently uses bold text (
**제목**) for section titles, which triggers markdownlint’s MD036 rule and can reduce semantic clarity. Consider switching to heading syntax (e.g.,### 목적) to improve structure, accessibility, and consistency.Also applies to: 5-5, 9-9, 13-13
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
1-1: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
3-3: Remove or clarify standalone list markersStandalone hyphens (
-) render as empty list items, which can confuse contributors and linters. If these are placeholders, replace them with explicit instructions (e.g.,<내용을 입력하세요>) or remove them to clean up the template.Also applies to: 7-7, 15-15
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/pull_request_template.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
.github/pull_request_template.md
1-1: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
5-5: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
9-9: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
13-13: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
Cllaude99
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다!
| @@ -0,0 +1,16 @@ | |||
| /** 권한 관련 */ | |||
| const ACCESS_TOKEN = btoa("FLOWBIT_ACT"); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btoa를 사용하여 ASCII 문자열로 처리하신 점이 인상 깊었습니다. 해당 함수에 대해서도 처음으로 알게되었어요!
목적
작업 내용
src/api/index.ts
src/utils/constant.ts
src/app/signin/index.tsx
src/components/layout/header.tsx
FLOWBIT_ACT이라고 명명했는데 너무 명시적으로 이름을 설정하는걸 방지하고자 ASCII 문자열로 변경했어요src/hooks/useCheckEmail.ts
src/utils/constant.ts
필수 리뷰어
이슈 번호
Summary by CodeRabbit
New Features
Refactor
Chores