[이율] Sprint11

[yulrang]

요구사항

• Github에 PR(Pull Request)을 만들어서 미션을 제출합니다.
• 피그마 디자인에 맞게 페이지를 만들어 주세요.
• 기존의 스프린트 미션 8에 이어서 React, Typescript를 사용합니다.

체크리스트 [기본]

회원가입

• 유효한 정보를 입력하고 스웨거 명세된 “/auth/signUp”으로 POST 요청해서 성공 응답을 받으면 회원가입이 완료됩니다.
• 회원가입이 완료되면 “/login”로 이동합니다.
• 회원가입 페이지에 접근시 로컬 스토리지에 accessToken이 있는 경우 ‘/’ 페이지로 이동합니다.

로그인

• 회원가입을 성공한 정보를 입력하고 스웨거 명세된 “/auth/signIp”으로 POST 요청을 하면 로그인이 완료됩니다.
• 로그인이 완료되면 로컬 스토리지에 accessToken을 저장하고 “/” 로 이동합니다.
• 로그인/회원가입 페이지에 접근시 로컬 스토리지에 accessToken이 있는 경우 ‘/’ 페이지로 이동합니다.

메인

• 로컬 스토리지에 accessToken이 있는 경우 상단바 ‘로그인’ 버튼이 판다 이미지로 바뀝니다.

주요 변경사항

• 회원가입, 로그인 기능을 구현하였고, 일단 30분이 지났을 때마다 refresh하여 로그인 상태가 지속되도록 만들었습니다.
• 자유게시판 글쓰기, 댓글쓰기, 좋아요 기능을 구현하였습니다.

스크린샷

멘토에게

• refresh 요청을 어떤 조건일때 보내야하는지 모르겠어서 일단 토큰 만료기간인 30분이 지나면 요청하도록했습니다. axios를 사용하지 않았을때 어떻게 로직을 짜야할지 궁금합니다.

[1005hoon]

@yulrang

율님, 쉽지 않은 프로젝트였을텐데 고생 많으셨습니다 ㅎㅎ

우선 마크업적인 부분은 전반적으로 좋아보여요.

프로젝트를 개선시킬 수 있는 방향을 몇가지 핀포인트 해보자면,

1. 변수와 함수 선언시 명확한 가치를 전달하도록 작성
2. 콤포넌트와 함수의 역할과 책임을 명확하게 분리
3. 너무 큰 콤포넌트는 관심사에 따라 분리

해볼 수 있으면 좋겠습니다!

이에 대해서는 코드단 리뷰로 남겨두었으니 한번 참고 부탁드릴게요.

또한 refresh 토큰에 관해서는 authprovider 리뷰에 남겨두었습니다.

고생 많으셨습니다 !

[1005hoon]

BoardList 콤포넌트에 사용될 prop에 대한 인터페이스를 articleListProps라고 하신 이유가 있나요?
특별한 이유가 없다면 콤포넌트 이름에 Props를 추가하는 형태로 타입 설정을 해보면 어떨까 싶습니다.
추가로, 인터페이스 또는 타입 이름을 선언하는 경우, PascalCase를 유지하는게 타입스크립트 진영 컨벤션인데요, 이 컨벤션을 따르면 어떨까요?

[1005hoon]

@yulrang

useAuth의 목적이 무엇인가요? useAuth hook의 역할범위가 조금 애매한 듯 한데요
우선 커스텀 훅의 경우, 재활용되는 로직을 추상화해서 사용하기 위해 활용하는 기능이지요.
이 관점에서, useAuth라는 녀석은 생각해보았을 때 로그인 된 사용자의 세션 또는 토큰을 관리하고, 이 값들을 변경하기 위한 액션들을 제공해주는 목적을 지닐텐데요.
지금 사용되는 상황을 보면 단순 인증 / 인가 값에 대한 관리 외에도, 이에따른 로직 처리를 해주며 하나의 auth-guard 형태의 기능까지 제공하고 있지요.
이는 useAuth 훅의 역할 범위를 모호하게 하고, 이후 인증 / 인가 형태에 따라 특정 라우트가 보호되어야 하는 경우 확장하기 힘들게 하는 방해요소가 될 수 있습니다.

한번 useAuth가 명확하게 어떤 작업을 해야 할지, 그리고 어떻게 리팩토링 해보면 좋을지 고민해볼까요?
하지만 지금은

[1005hoon]

사용자 정보와 이 정보를 불러오는 동안의 로딩 상태는 동일한 도메인 요소로 묶어 관리하기 적합하지 않습니다.
따로 분리를 해두면 어떨까요?

const [user, setUser] = useState<User | null>(null)
const [isLoading, setIsLoading] = useState(false)
const [isAuthed, setIsAuthed] = useState(false)

[1005hoon]

여기 try 와 finally는 존재하는데, 발생한 에러에 대한 핸들링은 보이지 않네요!
에러 핸들링이 필요해보입니다.

또한 변수의 변경 범위는 최대한 지역적일수록 유지보수하기 좋은데요,
현 상황은 let nextUser라는 변수가 try catch 외부에 선언되어 있고, 이 값을 어디서든 지속적으로 변경해 활용할 수 있는 상황입니다. 지금 당장은 크게 로직이 복잡하지 않아 별 이슈가 없지만, 이런 패턴은 로직 복잡도가 높아질수록 변수 오염 확률이 높아져 의도치 않은 오류를 발생하기 쉽게 해줍니다.

따라서 변수는 가능한 한 지역적으로 활용해보면 어떨까요?

async function getMe() {
  setIsLoading(() => true)
  const accessToken = localStorage.getItem("accessToken");

  if (!accessToken) {
    console.warn("No access token found")
    return
  }

  try {
    const response = await fetch(...)
    const json = await response.json()

    if (!response.ok) {
      const errorMessage = json.error || response.statusText // 서버에서 반환한 에러가 있다면 - { error :... } 형태로 제공하기로 약속했다는 전제 -  서버에서 반환한 에러를 담고, 아니라면 network response를 담는다
      throw new Error(errorMessage)
    }

   const { data } = json // 서버에서 { data : User } 형태로 반환하기로 약속했다는 전제
   setUser(() => data)
  } catch(e) {
    // 에러핸들링
  } finally {
    setIsLoading(() => false)
  }
}

[1005hoon]

로그인 함수 호출 시 전달되는 데이터는 user auth 정보일텐데요, Record<string , any> 로 관리하기 보다는 이를위한 validation을 두어보는건 어떨까요?

[1005hoon]

로그인 된 사용작 ㅏ없다면 빈 화면을 보여주는게 맞을까요?

[1005hoon]

페이지에서 처리하고 있는 일이 너무 많습니다.
다음과 같이 세분화 해 콤포넌트로 나누면 어떨까요?

1. 모든 댓글을 보여주는 콤포넌트
2. 새로운 댓글을 작성할 수 있는 콤포넌트
3. 선택된 데이터를 보여주는 콤포넌트

[1005hoon]

회원가입 오류 시 무조건 로그인페이지로 이동시키기보단,
회원가입 시도가 제대로 처리되지 않았음을 사용자에게 피드백 해주고, 그에대한 액션은 사용자가 알아서 하도록 해주는게 더 좋지 않을까요?

[1005hoon]

join이라는 url보다, register과 같은 url이 더 적합하지 않을까 생각됩니다!

[1005hoon]

동일한 로직이 많이 발생하고 있는데요,
fetch 로직만을 처리하는 공통 함수를 만들어 관리하는건 어떨까요?