Skip to content

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

Merged
merged 10 commits into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"typescript.tsdk": "node_modules/typescript/lib"
}
4 changes: 2 additions & 2 deletions src/app/(Main)/talk/page.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import TalkSection from './components/TalkSection';
import TitleSection from './components/TitleSection';
import TalkSection from './TalkSection';
import TitleSection from './TitleSection';
import { PageAnimation } from '@/components/PageAnimation';

export default function Talk() {
Expand Down
30 changes: 30 additions & 0 deletions src/app/auth/AuthComponent.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
'use client';

import { useRouter } from 'next/navigation';

import { getRegister } from '@/apis';
import useUserState from '@/context/userState';
import useDidMount from '@/hooks/useDidMount';

interface AuthComponentProps {
code: string;
}

export default function AuthComponent({ code }: AuthComponentProps) {
const { setUser } = useUserState();
const router = useRouter();

useDidMount(async () => {
try {
const response = await getRegister(code);
setUser(response);
localStorage.setItem('accesstoken', response.token.accessToken);
alert(`로그인에 성공했어요!`);
router.back();
} catch (error) {
router.push('/');
}
});

return null;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. import 구문에서 'next/navigation'이 아니라 'next/router'를 사용해야 합니다.
  2. useDidMount 대신 useEffect 훅을 사용하여 비동기 작업을 처리할 수 있습니다.
  3. 예외 처리에서 router.push('/')를 계속 반복해서 사용하는 대신, 에러 발생 시 특정 메시지를 보여주는 방안을 고려할 수 있습니다.
  4. 클라이언트 쪽 쿠키 설정에 대한 검증 및 보안을 강화할 필요가 있습니다.
  5. 코드에 주석을 추가하여 기능 및 처리 방법을 명확히 설명하는 것이 좋습니다.

24 changes: 24 additions & 0 deletions src/app/auth/logout/LogoutComponent.tsx
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;
}
24 changes: 2 additions & 22 deletions src/app/auth/logout/page.tsx
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

한가지 컴포넌트로 이루어진 페이지를 만드는 이유를 잘 모르겠습니다..!
가능하면 파일 갯수를 줄이고 싶은데, 본 페이지에 LogoutComponent 코드를 넣는건 어려울까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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를 구분하고자 하는 컨벤션을 사용하는 건 어떨까요?

의견 자유롭게 제시해주세요 !!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

모든 페이지를 RSC로 두는 것에 대해서는 찬성합니다..!

폴더구조에 대해서, page.tsx도 하나의 컴포넌트인 만큼 해당 페이지에 종속되는 컴포넌트들은 같은 폴더 내에 (components 폴더를 매번 따로 만들지 않고) 정의하는 방식으로 폴더의 depth를 줄여나가면 더 좋을 것 같습니다!

Copy link
Collaborator Author

@guesung guesung Aug 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

링크

좋습니다 :) 구두로 논의한 방식으로 컨벤션에 추가했습니다 !

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7d2acb3 수정한 커밋입니다 !

Copy link
Member

Choose a reason for hiding this comment

The 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 />;
}
30 changes: 10 additions & 20 deletions src/app/auth/page.tsx
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} />;
}
36 changes: 36 additions & 0 deletions src/app/auth/withdraw/WithdrawComponent.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
'use client';

import { useRouter, useSearchParams } from 'next/navigation';

import { patchDeleteAccount } from '@/apis';
import useUserState from '@/context/userState';
import useDidMount from '@/hooks/useDidMount';

export default function WithdrawComponent() {
const router = useRouter();
const { token, clearUser } = useUserState();
const withdrawalReason = useSearchParams().get('reason');

useDidMount(async () => {
if (token && withdrawalReason) {
const response = await patchDeleteAccount(
token.accessToken,
token.refreshToken,
withdrawalReason
);
console.log(response);
}

// 회원탈퇴 api 될 떄 까지만
// if (token) {
// patchLogout(token.accessToken, token.refreshToken).then(() => {
// alert('로그아웃 되었습니다');
// });
// }
clearUser();
localStorage.removeItem('accesstoken');
router.replace('/menu');
});

return null;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드 패치의 간단한 코드 리뷰를 제공하겠습니다.

  1. import { useRouter, useSearchParams } from 'next/navigation'; 이 부분에서 'next/navigation' 모듈을 사용하는 것은 잘못된 경로입니다. 올바른 경로인 'next/router'를 사용해야 합니다.

  2. useDidMount 훅은 내장 라이브러리에 없는데, 커스텀 훅으로 추측됩니다. 해당 훅이 어떻게 구현되었는지 점검할 필요가 있습니다.

  3. 비동기 작업을 수행하는 useDidMount 훅 내에서 에러 처리를 하지 않고 있습니다. 작업이 실패하면 적절한 방법으로 처리하는 코드가 빠져있습니다.

  4. patchLogout 함수가 정의되어 있지 않아 별도의 로그아웃 처리가 이루어지지 않습니다. 이에 관련된 수정이 필요합니다.

  5. router.replace('/menu');는 회원 탈퇴 직후에 실행되므로 사용자 경험 면에서 즉각적인 리디렉션에 대한 고민이 필요할 수 있습니다.

  6. 보안 측면에서 클라이언트 쿠키 값을 단순히 삭제하기만 하는 것보다 안전한 방식으로 처리하는 것이 좋습니다.

  7. 코드 리팩터링 시, 주석에 오타가 있습니다. "때"의 한자 표기 오류입니다.

  8. WithdrawComponent 함수는 JSX 엘리먼트를 반환하지 않음에 유의해야 합니다. 기능적으로 return null;이 의도대로 동작하는지 확인할 필요가 있습니다.

개선 사항:

  • 모듈 경로 수정 (next/navigation -> next/router)
  • 비동기 작업의 에러 처리 추가
  • patchLogout 함수 관련 처리 추가
  • 클라이언트 쿠키의 안전한 삭제 처리 추가
  • 사용자 리디렉션 부분에 대한 더 나은 해결책 검토 및 적용
  • useDidMount 훅의 구체적인 구현 리뷰 및 개선

주의: 위의 제안은 코드 리뷰의 일부로, 실제 상황에 맞게 구체적으로 작업을 진행해야 합니다.

33 changes: 2 additions & 31 deletions src/app/auth/withdraw/page.tsx
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 />;
}
1 change: 1 addition & 0 deletions tsconfig.tsbuildinfo

Large diffs are not rendered by default.