Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds Apple as a login provider (type, constant, UI styling, and redirect handling via NEXT_PUBLIC_APPLE_AUTH_URL) and reworks the terms-agreement fetch effect and SignupContext's setAgreements to use functional state updates, removing the previous fetch-guard and eliminating an unnecessary effect dependency. ChangesApple Login Support
Terms Agreement Fetch and State Update Fix
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces Apple social login support across the application, including adding the Apple logo SVG, updating authentication constants, types, and login forms, and refactoring the terms agreement hook to use functional state updates. The review feedback highlights potential runtime errors in useTermsAgreement.ts if terms data is undefined, recommending optional chaining, and suggests aligning the Apple login URL assignment in useLoginForm.tsx with other providers for consistency.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| myTerms.terms.forEach(term => { | ||
| newAgreements[term.id] = term.agreed; | ||
| }); |
There was a problem hiding this comment.
myTerms.terms가 존재하지 않거나 undefined인 경우 forEach 호출 시 런타임 에러(TypeError)가 발생할 수 있습니다. API 응답의 안정성을 위해 옵셔널 체이닝(?.)을 사용하거나 기본값(|| [])을 제공하는 등 방어적 코딩을 적용하는 것이 좋습니다.
| myTerms.terms.forEach(term => { | |
| newAgreements[term.id] = term.agreed; | |
| }); | |
| myTerms?.terms?.forEach(term => { | |
| newAgreements[term.id] = term.agreed; | |
| }); |
| terms.forEach(term => { | ||
| initialAgreements[term.id] = false; | ||
| }); |
There was a problem hiding this comment.
terms가 존재하지 않거나 undefined인 경우 forEach 호출 시 런타임 에러가 발생할 수 있습니다. 안전한 실행을 위해 옵셔널 체이닝(?.)을 사용하거나 기본값(|| [])을 설정하여 방어적으로 처리하는 것이 좋습니다.
| terms.forEach(term => { | |
| initialAgreements[term.id] = false; | |
| }); | |
| terms?.forEach(term => { | |
| initialAgreements[term.id] = false; | |
| }); |
| case "apple": | ||
| if (!process.env.NEXT_PUBLIC_APPLE_AUTH_URL) { | ||
| toast.error("애플 로그인 주소 설정이 누락되었습니다."); | ||
| return; | ||
| } | ||
| authUrl = process.env.NEXT_PUBLIC_APPLE_AUTH_URL; | ||
| break; |
There was a problem hiding this comment.
다른 소셜 로그인 제공자(Google, Kakao, Naver)들과 동일한 패턴을 유지하여 코드의 일관성과 가독성을 높일 수 있습니다. apple 케이스에서도 환경 변수가 없을 때 빈 문자열을 할당하도록 하고, 하단의 공통 에러 처리 로직(if (authUrl) ... else ...)에서 처리하도록 단순화하는 것을 제안합니다.
case "apple":
authUrl = process.env.NEXT_PUBLIC_APPLE_AUTH_URL || "";
break;
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/base-ui/Join/steps/TermsAgreement/useTermsAgreement.ts (1)
63-68: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winMemoize
onNextbefore passing it intoTermsAgreement
SignupStepPageClientcreatesonNext={() => navigateTo(...)}inline, so its identity changes on every render. WithonNextinuseTermsAgreement’s dependency list, that will re-run the terms fetch and reset the loading state whenever the parent renders. Wrap the callback inuseCallback(or move it behind a stable ref) to keep the effect from retriggering.🤖 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 `@src/components/base-ui/Join/steps/TermsAgreement/useTermsAgreement.ts` around lines 63 - 68, The effect in useTermsAgreement is being retriggered because onNext is an unstable inline callback from SignupStepPageClient, which causes fetchTerms and loading state resets on every parent render. Memoize the onNext handler in SignupStepPageClient with useCallback (or store it in a stable ref) so the function identity stays stable, and keep useTermsAgreement’s dependency array from re-running unnecessarily.
🧹 Nitpick comments (1)
src/components/base-ui/Login/hooks/useLoginForm.tsx (1)
103-133: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate missing-config handling for apple vs other providers.
The
applecase pre-checks and early-returns with a specific toast message, whilekakao/naverrely on the shared fallback check at Lines 129-133 with a generic message. This is redundant logic producing inconsistent UX depending on provider.♻️ Proposed consolidation
case "apple": - if (!process.env.NEXT_PUBLIC_APPLE_AUTH_URL) { - toast.error("애플 로그인 주소 설정이 누락되었습니다."); - return; - } - authUrl = process.env.NEXT_PUBLIC_APPLE_AUTH_URL; + authUrl = process.env.NEXT_PUBLIC_APPLE_AUTH_URL || ""; break;🤖 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 `@src/components/base-ui/Login/hooks/useLoginForm.tsx` around lines 103 - 133, Consolidate the missing auth URL handling in handleSocialLogin so all providers use the same flow instead of a special-case apple branch plus the shared fallback. Remove the apple-specific early return in useLoginForm and centralize the toast/error handling after the switch, using provider-specific messaging only if needed; keep the redirect logic in the authUrl check path.
🤖 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.
Outside diff comments:
In `@src/components/base-ui/Join/steps/TermsAgreement/useTermsAgreement.ts`:
- Around line 63-68: The effect in useTermsAgreement is being retriggered
because onNext is an unstable inline callback from SignupStepPageClient, which
causes fetchTerms and loading state resets on every parent render. Memoize the
onNext handler in SignupStepPageClient with useCallback (or store it in a stable
ref) so the function identity stays stable, and keep useTermsAgreement’s
dependency array from re-running unnecessarily.
---
Nitpick comments:
In `@src/components/base-ui/Login/hooks/useLoginForm.tsx`:
- Around line 103-133: Consolidate the missing auth URL handling in
handleSocialLogin so all providers use the same flow instead of a special-case
apple branch plus the shared fallback. Remove the apple-specific early return in
useLoginForm and centralize the toast/error handling after the switch, using
provider-specific messaging only if needed; keep the redirect logic in the
authUrl check path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 48558fc1-04fe-436c-8a31-5ab4778feaec
⛔ Files ignored due to path filters (1)
public/appleLogo.svgis excluded by!**/*.svg
📒 Files selected for processing (6)
src/components/base-ui/Join/steps/TermsAgreement/useTermsAgreement.tssrc/components/base-ui/Login/hooks/useLoginForm.tsxsrc/components/base-ui/Settings/SocialLogin/SocialLoginCard.tsxsrc/constants/auth.tssrc/contexts/SignupContext.tsxsrc/types/member.ts
📌 개요 (Summary)
🛠️ 변경 사항 (Changes)
📸 스크린샷 (Screenshots)
(UI 변경 사항이 있다면 첨부해주세요)
✅ 체크리스트 (Checklist)
pnpm build)pnpm lint)Summary by CodeRabbit
New Features
Bug Fixes