[김석주]TS Todo Refactor 과제

[ksjdev]

🖥️배포 주소

https://dummy-rosy.vercel.app/

📌 과제 설명

지난 FEDC5-3 Todo 과제를 TS로 리팩토링한 프로젝트입니다.

✅ PR 포인트 & 궁금한 점

TS 관련 질문 혹은 궁금한 점

• any타입을 사용하지 않으려고 노력했습니다, 다만 그러다보니 어색하게 선언된 타입이 있을 수도 있다는 생각이 드는데, 타입 선언에 관련해서 의견 있으시면 자유롭게 주시면 감사드리겠습니다.
• 이번 과제에서 제네릭을 써야하나? 라는 의문과 만약에 쓴다면 어떻게 써야하는 가? 에 대해서 고민하다가 일단 모두 단순한 type으로 구현했는데요. 혹시 관련해서 조언이나 의견 있으시면 자유롭게 주시면 감사드리겠습니다.
• 타입스크립트 프로젝트는 처음이라 부족한 부분이 많다고 생각합니다. 관련해서 조언이나 의견이 있으시다면 자유롭게 기술해주시면 감사드리겠습니다.

TS가 아닌 내용에 대한 질문 혹은 궁금한 점

• const = () => {} 형식의 함수 표현식을 통해서 구현을 했습니다. 다른 부분에서는 어려움은 크게 없었지만 이상하게 todoList.ts의 state를 사용하는 과정에서 app.ts에서 todoList.state = nextState와 같이 직접 바꿔줘야하는 경우가 생겼는데, 원인은 여전히 찾아보고 있지만 해당 부분이 코드리뷰 하실 때 어색하실 수도 있다는 생각에 미리 알려드립니다. 또한 관련해서 조언 혹은 피드백 주시면 감사드리겠습니다.
• 파일 분리와 코드 작성이 깔끔하게 잘 되었나요? 그리고 여전히 파일 내부에서도 분리와 병합, 그리고 코드의 흐름에 대한 고민이 있는데 관련해서 조언이나 피드백이 있으시다면 자유롭게 부탁드리겠습니다.

개발 흐름이나 발생한 문제 해결 사항은 Flow.md 파일에 기술해두었습니다. 혹시 궁금하신 내용이 있다면 참고하셔도 좋을 것 같습니다. Flow.md

감사합니다.

[yjc2021]

석주님 안녕하세요! 이번에 주로 TS 변환 관점에서 리뷰를 진행했습니다 :) state 흐름이나, 컴포넌트 간 상호작용 피드백이 없는 점 죄송함다...ㅠㅠ
컴포넌트 props와 state 에 대한 타입 정의가 깔끔하게 하나의 파일에 명시되어 있어서 타입을 이해하고, 타입들 간의 관계를 파악하기 편했어요! 또 기본적으로 모든 함수의 매개변수와 return 타입, 변수 타입도 꼼꼼하게 정의하셔서 마이그레이션 정말 잘 된 것 같아요 :) 수고하셨습니다~

TS 관련 질문 혹은 궁금한 점

any타입을 사용하지 않으려고 노력했습니다, 다만 그러다보니 어색하게 선언된 타입이 있을 수도 있다는 생각이 드는데, 타입 선언에 관련해서 의견 있으시면 자유롭게 주시면 감사드리겠습니다.

• 전반적으로 타입 선언을 잘 하신 것 같아요! 반복되는 타입의 재사용도 고려해보시면 좋을 것 같습니다 :)

이번 과제에서 제네릭을 써야하나? 라는 의문과 만약에 쓴다면 어떻게 써야하는 가? 에 대해서 고민하다가 일단 모두 단순한 type으로 구현했는데요. 혹시 관련해서 조언이나 의견 있으시면 자유롭게 주시면 감사드리겠습니다.

• 사실 저도 이 프로젝트에서는 모든 컴포넌트들이 동일한 state을 다루기 때문에 제네릭의 사용이 필요가 없어보입니다. 대신 다루는 state이 다른 컴포넌트들의 타입을 재사용하고 싶으시면 제네릭 사용도 고민해보시면 좋을 것 같아요:) (제 코드 types/index.ts 에서 시도해봤어요!)

타입스크립트 프로젝트는 처음이라 부족한 부분이 많다고 생각합니다. 관련해서 조언이나 의견이 있으시다면 자유롭게 기술해주시면 감사드리겠습니다.

• 저도 아직 ts는 어색하네요...ㅎㅎ 전 이번에 하면서 굳이 .d.ts 파일이나 namespace 없이도 타입 선언을 할 수 있을 거 같은데, 얘네들은 언제 써야하는거지? 라는 의문이 생겼습니다. 석주님도 고민해보시고 다음 회의 때 얘기해보면 좋을 것 같아요 :)

TS가 아닌 내용에 대한 질문 혹은 궁금한 점

const = () => {} 형식의 함수 표현식을 통해서 구현을 했습니다. 다른 부분에서는 어려움은 크게 없었지만 이상하게 todoList.ts의 state를 사용하는 과정에서 app.ts에서 todoList.state = nextState와 같이 직접 바꿔줘야하는 경우가 생겼는데, 원인은 여전히 찾아보고 있지만 해당 부분이 코드리뷰 하실 때 어색하실 수도 있다는 생각에 미리 알려드립니다. 또한 관련해서 조언 혹은 피드백 주시면 감사드리겠습니다.

• 이 부분은 피드백을 못 드렸어요 ㅠ 조금 더 생각해보겠습니다!

파일 분리와 코드 작성이 깔끔하게 잘 되었나요? 그리고 여전히 파일 내부에서도 분리와 병합, 그리고 코드의 흐름에 대한 고민이 있는데 관련해서 조언이나 피드백이 있으시다면 자유롭게 부탁드리겠습니다.

• 저는 Utils 폴더가 src/components/Utils 보다 src/Utils가 더 자연스러운 것 같아요! Utils는 컴포넌트는 아니다 보니까 협업 시 동료들이 헷갈릴 수도 있을 거 같아요 :)

[yjc2021]

저도 defaultValue=[]로 처음에 해놨는데 타입이 never로 유추가 돼서

export type setStorage<T> = (key: string, value: T) => void;
export type getStorage<T> = (key: string, defaultValue: T) => T;

export const setItem: setStorage<TodosType> = ...;
export const getItem: getStorage<TodosType> = ...;

로 해봤습니다! 조금 오버엔지니어링인 것 같기도 해서 석주님은 어떻게 생각하시는지 궁금합니다 :)

[ksjdev]

오히려 타입스크립트를 쓰는 훨씬 더 명확한 이유가 있는 코드라고 생각합니다! 반영해보겠습니다! 감사합니다!👍

[yjc2021]

많은 컴포넌트 Params 타입에서

{
   $target: HTMLElement | null;
}

이 반복되는 거 같은데 이를 interface로 정의하고, 컴포넌트 props마다 이를 extends하는 건 어떨까요?
저라면 state이 있는 컴포넌트와 state이 없는 컴포넌트 타입으로 먼저 나누고 싶어서

type StatelessComponentProps = {
   $target: HTMLElement | null;
}
type StatefulComponentProps = {
  $target: HTMLElement | null;
  initialState: StateType;
}

이렇게 나누고 각 컴포넌트에서 알맞게 extends 할 거 같아요. 석주님은 두 방식 중에 어떤 게 더 나은 것 같나요? :)

[ksjdev]

오호 이 부분은 용재님의 의견도 좋네요! 사실 extends를 잘 떠올리지 못했는데 알려주셔서 감사합니다!
아무래도 좋은 코드의 규칙 중 하나인 재사용을 위해서라면 용재님처럼 작성하는 방식이 더 효율적인 것 같다고 생각합니다! 특히 지금은 내부 코드가 짧지만 만약 길어질 경우 더더욱 유용할 것 같네요!

[yjc2021]

P5
이건 TS관련 사항은 아닌데, 저희 팀 종운님이 에러 핸들링 할때 에러 메세지를 상수로 잘 정리하셨더라구요! 해당 코드 로직 석주님도 한번 보시면 좋을 것 같아서 공유합니다 :)

// constants.ts
const KNOWN_ERROR_MESSAGES = {
  'Movie not found!': '영화를 찾을 수 없습니다.',
  'Too many results.': '검색 결과가 너무 많습니다.',
  'Incorrect IMDb ID.': '올바르지 않은 IMDb ID입니다.'
};

export const ERROR_MESSAGES = new Proxy(KNOWN_ERROR_MESSAGES, {
  get: function (target: { [key: string]: string }, prop: string) {
    if (prop.length < 1) {
      return '알 수 없는 에러입니다.';
    }
    return target[prop] ?? prop;
  }
});

종운님이 정의하지 못한 에러 메시지까지 상수로 활용하기 위해 Proxy를 사용하셨더라구요. 실제 에러 메세지 상수를 가져올 때는 KNOWN_ERROR_MESSAGES 를 사용하지 않고 ERROR_MESSAGES 만 사용합니다

[ksjdev]

멋진 방식이네요! 저도 흡수해서 적용해보도록 하겠습니다!