[김정현] Sprint10

[kjh9852]

요구사항

기본

상품 등록 페이지

• 상품 등록 페이지 주소는 “/addboard” 입니다.
• 게시판 이미지는 최대 한개 업로드가 가능합니다.
• 각 input의 placeholder 값을 정확히 입력해주세요.
• 이미지를 제외하고 input 에 모든 값을 입력하면 ‘등록' 버튼이 활성화 됩니다.
• 회원가입, 로그인 api를 사용하여 받은accessToken을 사용하여 게시물 등록을 합니다.
• ‘등록’ 버튼을 누르면 게시물 상세 페이지로 이동합니다.

상품 상세 페이지

• 상품 상세 페이지 주소는 “/board/{id}” 입니다.
• 댓글 input 값을 입력하면 ‘등록' 버튼이 활성화 됩니다.
• 활성화된 ‘등록' 버튼을 누르면 댓글이 등록됩니다

주요 변경사항

• 이전 미션 코드 리뷰 사항 반영해보았습니당

스크린샷

멘토에게

• 타입스크립트 적용하는게 아직 많이 어렵습니당..
• 로직도 한 파일에 있는 부분이 있어서 분리해보겠습니다
• 정렬하는 버튼 부분에서 (DropDown) enum으로 정렬 옵션을 설정해보았는데 괜찮은 방식일까요??
• 데이터를 받아오는 부분에서 한개의 커스텀훅으로 처리를 해볼려고 하는데 코드가 복잡해지는 느낌이 듭니당..
  그냥 여러 커스텀훅으로 분리를 하는게 나을까요??

[lisarnjs]

우선 api 커스텀훅 분리는 처음부터 전체적으로 묶을 생각보다는 api 하나씩 커스텀 훅으로 분리해보는 걸 추천드리고, 그 뒤에 커스텀훅들 사이에서 공통적인 로직이 눈에 보인다면 그걸 또 하나의 커스텀훅으로 분리해서 리팩토링 하는 과정으로 가시는 것이 불필요한 확장성도 방지할 수 있고, 코드 구현 자체가 훨씬 쉬워질 거에요!

enum관련된 아티클 하나 첨부해드릴게요! 사용이 나쁘다는 것은 아닙니다만 이런 의견도 있다 정도만 알아두시면 좋을 것 같아요!
https://velog.io/@hhhminme/%EB%84%A4-Enum-%EB%88%84%EA%B0%80-Typescript%EC%97%90%EC%84%9C-Enum%EC%9D%84-%EC%93%B0%EB%83%90

이번 한 주도 화이팅이에요!

[lisarnjs]

isActive={!isActive} 컴포넌트에 isActive를 반전시켜서 값을 넘겨주는 것 같아서 isActive라는 이름 대신 다른 이름으로 prop를 넘겨주는 것은 어떨까요?!

[kjh9852]

아니면 isActive 조건을 바꿔보겠습니다!

[lisarnjs]

이 부분에서 /images/upload 부분 함수를 따로 분리해주는 것이 좋겠네요!
예를 들면 이런식으로 파일을 분리할 수 있겠네요!

// axiosInstance.ts
import axios from 'axios';

const axiosInstance = axios.create({
  baseURL: process.env.NEXT_PUBLIC_API_BASE_URL, // 환경 변수로 설정된 API 기본 URL
  timeout: 10000, // 10초 타임아웃
  headers: {
    'Content-Type': 'application/json',
  },
});

export default axiosInstance;

// api.ts
import axiosInstance from './axiosInstance';

export const uploadImage = async (image: File, token: string) => {
  const formData = new FormData();
  formData.append('image', image);

  const response = await axiosInstance.post('/images/upload', formData, {
    headers: {
      Authorization: `Bearer ${token}`,
    },
  });

  return response.data.url;
};

export const createArticle = async (title: string, content: string, imageUrl: string, token: string) => {
  const response = await axiosInstance.post('/articles', {
    title,
    content,
    image: imageUrl,
  }, {
    headers: {
      Authorization: `Bearer ${token}`,
    },
  });

  return response.data.id;
};

[lisarnjs]

export enum SortOptionType {
  Recent = "recent",
  Like = "like",
}

sortText: SortOptionType; // 이렇게 정의해주려고 하셨던 용도가 아니였을까요!?

[lisarnjs]

최신순,좋아요순 도 정의가 필요하다면 코드의 일관성을 위해 이렇게 추가하면 좋을 것 같아요

const SortOptionLabels = {
  [SortOptionType.Recent]: "최신순",
  [SortOptionType.Like]: "좋아요순",
};

이렇게 하면 sortText의 타입을 명확하게 정의할 수 있을 뿐 아니라, SortOptionLabels를 통해서 쉽게 관리도 할 수 있을 거 같아요!

[lisarnjs]

우리 위에서 SortOptionType으로 옵션 정해준 거 불러오게 되면 오타날 일도 없고 코드 일관성도 생길 거 같지 않나요!?

const sortText = options.orderBy === SortOptionType.Recent ? SortOptionTypeLabel.Recent : SortOptionTypeLabel.Like;

[lisarnjs]

나중에 if-else문이 길어진다면 switch문으로 변경하는 것이 대안이 될 수 있다는 점!

[lisarnjs]

요기서만 IPostList로 I가 붙었네용 ㅎㅎ 네이밍도 일관성있게 PostListWrapper와 같은 이름을 사용하시면 좋을 것 같아요!

[kjh9852]

이름 정하는게 너무 어렵네용.. ㅠㅠ
postList: PostListProps; 로 정의만 다시 해준거라서 더 이름짓기가 어려운것 같아요

[lisarnjs]

SSR환경에서 서버 사이드에서는 localStorage를 읽을 수 없기 때문에 나중에 안전성을 생각해서

 useEffect(() => {
    const storedToken = localStorage.getItem('token');
    setToken(storedToken);
  }, []);

처럼 useEffect안에서 접근하는 것이 안전할 것 같네요!

[lisarnjs]

GetFn가 여러 함수 타입을 포함하고 있어서 타입이 명확하지 않게 되고, 이는 어디선가 에러를 발생시킬 수 있어요!
그래서 이럴 때 사용하는게 제네릭 타입입니다 ㅎㅎ

type GetFn<T> = (params: any, id?: string | string[] | undefined) => Promise<T>; 

요론식으로 제네릭 함수로 만들어주면 좋을 것 같아요 👍

[lisarnjs]

요기도 초기값을 설정해주는 것이 좋을 것 같아요!

[lisarnjs]

이미 로그인된 사용자인지 확인하는 로직이 추가되면 좋겠네요! 불필요한 로그인 시도를 방지할 수 있을 것 같아요 👍