-
Notifications
You must be signed in to change notification settings - Fork 3
Refactor: 회원가입 페이지 컴포넌트 리팩토링 : RSC, RCC 분리, async-await문으로 리팩토링 #77
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
Changes from all commits
2e72d3d
432442b
5b61b9c
0f01e6e
f2cc55a
2b4d1c5
8030cfb
7d2acb3
557e50a
ee6b0ee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
{ | ||
"typescript.tsdk": "node_modules/typescript/lib" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,11 @@ | ||
'use client'; | ||
|
||
import BookmarkEmpty from './BookmarkEmpty'; | ||
import { useBookmarkFeedQuery } from '@/apis'; | ||
import FeedSection from '@/components/Feed/FeedSection'; | ||
|
||
interface BookmarkSecionI { | ||
accesstoken: string; | ||
} | ||
|
||
export default function BookmarkSecion({ accesstoken }: BookmarkSecionI) { | ||
const query = useBookmarkFeedQuery(accesstoken); | ||
export default function BookmarkSecion() { | ||
const query = useBookmarkFeedQuery(); | ||
|
||
return ( | ||
<FeedSection query={query}> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
좀더 간결하고 효율적인 코드를 위한 변경 사항들을 제안 드렸습니다. |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,11 @@ | ||
'use client'; | ||
|
||
import BookmarkEmpty from './BookmarkEmpty'; | ||
import BookmarkSecion from './BookmarkSecion'; | ||
import useUserState from '@/context/userState'; | ||
import { ACCESS_TOKEN } from '@/constants'; | ||
import { getServerCookie } from '@/utils'; | ||
|
||
export default function BookmarkPage() { | ||
const { token } = useUserState(); | ||
export default async function BookmarkPage() { | ||
const token = await getServerCookie(ACCESS_TOKEN); | ||
|
||
if (token) return <BookmarkSecion accesstoken={token?.accessToken} />; | ||
else return <BookmarkEmpty />; | ||
if (token) return <BookmarkSecion />; | ||
return <BookmarkEmpty />; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,11 +4,12 @@ import Link from 'next/link'; | |
|
||
import LogoutModal from '@/components/Login/LogoutModal'; | ||
import { useOverlay } from '@/components/Overlay/useOverlay'; | ||
import { ACCESS_TOKEN } from '@/constants'; | ||
import { menuList } from '@/constants/data'; | ||
import useUserState from '@/context/userState'; | ||
import { getClientCookie } from '@/utils'; | ||
|
||
export default function MenuListSection() { | ||
const { isLogin } = useUserState(); | ||
const token = getClientCookie(ACCESS_TOKEN); | ||
const { open } = useOverlay(); | ||
|
||
function handleLogout() { | ||
|
@@ -30,12 +31,12 @@ export default function MenuListSection() { | |
<div key={idx} className="py-12" /> | ||
) | ||
)} | ||
{isLogin && ( | ||
{token && ( | ||
<> | ||
<div className="cursor-pointer py-12" onClick={handleLogout}> | ||
<div className="py-12 cursor-pointer" onClick={handleLogout}> | ||
<span id="subtitle-1">로그아웃</span> | ||
</div> | ||
<Link href={'/menu/withdraw'} className="cursor-pointer py-12"> | ||
<Link href={'/menu/withdraw'} className="py-12 cursor-pointer"> | ||
<span id="subtitle-1" className="text-tertiary"> | ||
탈퇴하기 | ||
</span> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 코드의 리뷰를 짧게 작성해 드리겠습니다.
개선을 위해:
등을 고려하면 좋을 것입니다. |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,11 @@ | ||
'use client'; | ||
|
||
import MenuListSection from './MenuListSection'; | ||
import Header from '@/components/Header'; | ||
import LoginSection from '@/components/Login/LoginSection'; | ||
|
||
export default function MenuPage() { | ||
export default async function MenuPage() { | ||
return ( | ||
<div className="px-20"> | ||
<Header close={true} title="메뉴" /> | ||
<Header close title="메뉴" /> | ||
<LoginSection /> | ||
<MenuListSection /> | ||
</div> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 코드 패치의 개선점과 잠재적 버그 위험에 대한 간단한 코드 리뷰입니다:
코드는 보호해야할 특별한 문제점은 없어 보입니다. |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
'use client'; | ||
|
||
import { useRouter } from 'next/navigation'; | ||
|
||
import { getRegister } from '@/apis'; | ||
import { ACCESS_TOKEN, EMAIL, NICKNAME } from '@/constants'; | ||
import useDidMount from '@/hooks/useDidMount'; | ||
import { setClientCookie } from '@/utils'; | ||
|
||
interface AuthComponentProps { | ||
code: string; | ||
} | ||
|
||
export default function AuthComponent({ code }: AuthComponentProps) { | ||
const router = useRouter(); | ||
|
||
useDidMount(async () => { | ||
try { | ||
const { | ||
token: { accessToken }, | ||
email, | ||
nickname, | ||
} = await getRegister(code); | ||
|
||
setClientCookie(ACCESS_TOKEN, accessToken); | ||
setClientCookie(EMAIL, email); | ||
setClientCookie(NICKNAME, nickname); | ||
|
||
alert(`로그인에 성공했어요!`); | ||
router.push('/'); | ||
} catch (error) { | ||
router.push('/'); | ||
} | ||
}); | ||
|
||
return null; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
'use client'; | ||
|
||
import { useRouter } from 'next/navigation'; | ||
|
||
import { patchLogout } from '@/apis'; | ||
import useUserState from '@/context/userState'; | ||
import useDidMount from '@/hooks/useDidMount'; | ||
|
||
export default function LogoutComponent() { | ||
const { token, clearUser } = useUserState(); | ||
const router = useRouter(); | ||
|
||
useDidMount(async () => { | ||
if (token) { | ||
await patchLogout(token.accessToken, token.refreshToken); | ||
alert('로그아웃 되었습니다'); | ||
} | ||
clearUser(); | ||
localStorage.removeItem('accesstoken'); | ||
router.back(); | ||
}); | ||
|
||
return null; | ||
} |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 한가지 컴포넌트로 이루어진 페이지를 만드는 이유를 잘 모르겠습니다..! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 컨벤션을 맞추기 위해서 따로 뺐습니다. 모든 페이지는 RSC(React Server Component)로 두고, 그 하위에 RSC와 RCC(React Client Component)로 구분해야 추후 확장에 있어서 RSC와 RCC를 구분하여 컴포넌트를 추가할 수 있기 때문입니다. 만약 이 page.tsx 컴포넌트가 RCC이면, 추후 RSC로 구현이 가능한 컴포넌트가 추가되었을 때 구조를 다시 바꿔야 한다는 불편함이 예상됩니다. 그래서 모든 page.tsx컴포넌트는 RSC로 두고, 하위 컴포넌트에서 RSC와 RCC를 구분하고자 하는 컨벤션을 사용하는 건 어떨까요? 의견 자유롭게 제시해주세요 !! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 모든 페이지를 RSC로 두는 것에 대해서는 찬성합니다..! 폴더구조에 대해서, page.tsx도 하나의 컴포넌트인 만큼 해당 페이지에 종속되는 컴포넌트들은 같은 폴더 내에 (components 폴더를 매번 따로 만들지 않고) 정의하는 방식으로 폴더의 depth를 줄여나가면 더 좋을 것 같습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 7d2acb3 수정한 커밋입니다 ! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 557e50a 추가된 컨벤션에 맞춰서 리팩토링 진행했습니다! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,25 +1,5 @@ | ||
'use client'; | ||
|
||
import { useRouter } from 'next/navigation'; | ||
import { useEffect } from 'react'; | ||
|
||
import { patchLogout } from '@/apis'; | ||
import useUserState from '@/context/userState'; | ||
import LogoutComponent from './LogoutComponent'; | ||
|
||
export default function Page() { | ||
const { token, clearUser } = useUserState(); | ||
const router = useRouter(); | ||
|
||
useEffect(() => { | ||
if (token) { | ||
patchLogout(token.accessToken, token.refreshToken).then(() => { | ||
alert('로그아웃 되었습니다'); | ||
}); | ||
} | ||
clearUser(); | ||
localStorage.removeItem('accesstoken'); | ||
router.back(); | ||
}); | ||
|
||
return; | ||
return <LogoutComponent />; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,26 +1,16 @@ | ||
'use client'; | ||
|
||
import { useRouter, useSearchParams } from 'next/navigation'; | ||
import { useEffect } from 'react'; | ||
import { redirect } from 'next/navigation'; | ||
|
||
import { getRegister } from '@/apis'; | ||
import useUserState from '@/context/userState'; | ||
import AuthComponent from './AuthComponent'; | ||
|
||
export default function Page() { | ||
const code = useSearchParams().get('code'); | ||
const { setUser } = useUserState(); | ||
const router = useRouter(); | ||
|
||
useEffect(() => { | ||
if (code) { | ||
getRegister(code).then((response) => { | ||
setUser(response); | ||
localStorage.setItem('accesstoken', response.token.accessToken); | ||
alert(`로그인에 성공했어요!`); | ||
router.back(); | ||
}); | ||
} | ||
}); | ||
interface AuthPageProps { | ||
searchParams: { | ||
code: string; | ||
}; | ||
} | ||
|
||
return; | ||
export default function AuthPage({ searchParams: { code } }: AuthPageProps) { | ||
if (!code) redirect('/'); | ||
return <AuthComponent code={code} />; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
'use client'; | ||
|
||
import { useRouter, useSearchParams } from 'next/navigation'; | ||
|
||
import { patchDeleteAccount } from '@/apis'; | ||
import { ACCESS_TOKEN } from '@/constants'; | ||
import useDidMount from '@/hooks/useDidMount'; | ||
import { getClientCookie, removeClientCookie } from '@/utils'; | ||
|
||
export default function WithdrawComponent() { | ||
const router = useRouter(); | ||
const token = getClientCookie(ACCESS_TOKEN); | ||
const withdrawalReason = useSearchParams().get('reason'); | ||
|
||
useDidMount(async () => { | ||
if (token && withdrawalReason) { | ||
const response = await patchDeleteAccount(withdrawalReason); | ||
console.log(response); | ||
} | ||
|
||
// 회원탈퇴 api 될 떄 까지만 | ||
// if (token) { | ||
// patchLogout(token.accessToken, token.refreshToken).then(() => { | ||
// alert('로그아웃 되었습니다'); | ||
// }); | ||
// } | ||
removeClientCookie(ACCESS_TOKEN); | ||
router.replace('/menu'); | ||
}); | ||
|
||
return null; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 코드 패치의 간단한 코드 리뷰를 제공하겠습니다.
개선 사항:
주의: 위의 제안은 코드 리뷰의 일부로, 실제 상황에 맞게 구체적으로 작업을 진행해야 합니다. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,34 +1,5 @@ | ||
'use client'; | ||
|
||
import { useRouter, useSearchParams } from 'next/navigation'; | ||
import { useEffect } from 'react'; | ||
|
||
import { patchDeleteAccount, patchLogout } from '@/apis'; | ||
import useUserState from '@/context/userState'; | ||
import WithdrawComponent from './WithdrawComponent'; | ||
|
||
export default function Page() { | ||
const router = useRouter(); | ||
const { token, clearUser } = useUserState(); | ||
const withdrawalReason = useSearchParams().get('reason'); | ||
|
||
useEffect(() => { | ||
if (token && withdrawalReason) { | ||
patchDeleteAccount(token.accessToken, token.refreshToken, withdrawalReason).then( | ||
(response) => { | ||
console.log(response); | ||
} | ||
); | ||
} | ||
// 회원탈퇴 api 될 떄 까지만 | ||
// if (token) { | ||
// patchLogout(token.accessToken, token.refreshToken).then(() => { | ||
// alert('로그아웃 되었습니다'); | ||
// }); | ||
// } | ||
clearUser(); | ||
localStorage.removeItem('accesstoken'); | ||
router.replace('/menu'); | ||
}); | ||
|
||
return; | ||
return <WithdrawComponent />; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,16 +6,17 @@ import { Icon } from '../Button/Icon'; | |
import { deleteBookmark, postBookmark } from '@/apis'; | ||
import LoginModal from '@/components/Login/LoginModal'; | ||
import { useOverlay } from '@/components/Overlay/useOverlay'; | ||
import { ACCESS_TOKEN } from '@/constants'; | ||
import { ICON } from '@/constants/icon'; | ||
import useUserState from '@/context/userState'; | ||
import { getClientCookie } from '@/utils'; | ||
|
||
interface BookmarkButtonI { | ||
poseId: number; | ||
isMarked: boolean; | ||
} | ||
export default function BookmarkButton({ poseId, isMarked }: BookmarkButtonI) { | ||
const { open } = useOverlay(); | ||
const { token } = useUserState(); | ||
const token = getClientCookie(ACCESS_TOKEN); | ||
const [marked, setMarked] = useState(isMarked); | ||
|
||
function onClick() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 코드 리뷰:
|
||
|
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.
이 코드 패치를 간단히 코드 리뷰해드리겠습니다.
고쳐야 할 부분:
useBookmarkFeedQuery
함수의 인자로accessToken
을 요청하는 것이 사라졌는데, 이는 기존 코드와 호환성 문제를 일으킬 수 있습니다.options
매개변수가 선택적으로 변경되었는데, 해당 변경에 대한 적절한 설명 또는 주석이 필요합니다.개선 제안:
accesstoken
매개변수가 완전히 없어진 부분에 대한 변경사항을 충분히 문서화하거나 코멘트 달아주면 좋습니다.