-
Notifications
You must be signed in to change notification settings - Fork 89
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
[2단계 - 자주 가는 음식점] 센트(김영우) 미션 제출합니다 #79
Conversation
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.
안녕하세요 센트~
두번째 스텝 진행하느라 고생 많으셨습니다.
바로 질문에 답변드립니다.
1. 컴포넌트 분리가 적절히 이루어졌는지 여부
첫 리뷰 때 컴포넌트가 적절히 분리되지 않은 것 같다는 리뷰를 남겨 주셔서 컴포넌트란 어떤 식으로 분리되는 것이 적절할 지 고민하며 코드를 작성해보았습니다. 혹시 이번 단계에서는 컴포넌트 분리가 적절히 이루어졌는지 궁금합니다!
우선 지난 스텝에서 이벤트 핸들링 위치를 각 컴포넌트 안으로 옮겨 주신 것만으로 충분한 개선이었다고 생각해요. 실제 엘리먼트를 그려주고 이벤트를 붙여주는 건 각 컴포넌트가 담당하고 App에서 전반적인 제어를 담당하도록 했다는 점에서요~ 여기서 조금 더 개선해본다면 App에서 모든 것을 다하지 않고 하위 컴포넌트에서 할 수 있는 일도 있을 것 같습니다. 러프하게 예를 들어본다면 restaurant-list 안에서 filter나 detail 모달을 보여주는 동작을 온전히 담당하도록 해볼 수도 있을 것 같습니다.
지금도 충분하니 요건 시간이 되실 때 고민해보시면 좋겠습니다.
2. e2e 테스트 코드가 적절히 사용되었는지 여부
요거는 코멘트 확인해주시면 되겠습니다!
3. 타입 단언 줄이기
타입 단언을 사용하신 곳을 보시면 거의 엘리먼트 타입 관련인듯해요.
아마 타입스트립트에서 제공하는 엘리먼트 타입이 충분치 않아서 어려우셨을 듯합니다.
타입 단언이 필요하다는 건 ts 컴파일러 입장에서 확신을 할 수 없다는 의미입니다.
사용자의 입력을 받았든, 라이브러리쪽에서 충분한 인터페이스를 제공해주지 못하고 있든 그 값이 어떤 타입인지 알만한 정보가 부족하다는 뜻이에요.
type SomeValue = 'value1' | 'value2'
someInput.addEventListner('change', (event) => {
const value: SomeValue = event.target.value
/*
* 여기에서 타입에러가 날 여지가 있습니다.
* 1. event.target에 대한 타입 정보가 충분하지 않을 수 있습니다.
* 2. 사용자의 입력을 의미하는 taget.value의 타입은 string이므로 'value1' | 'value2'라는 보장이 없습니다.
*/
})
ts컴파일러가 모른다면 알려주어야겠죠. 다만 타입 단언은 최후의 수단입니다.
확신할 수 없는데 맹목적으로 믿고 컴파일을 하도록 한다면 그 결과물은 js로 개발 한 것과 다를 게 없습니다.
type guard
, type predicate
이 두 가지 키워드로 공부해보시면 좋겠습니다.
아 그리고 다음과 같은 케이스도 보이더라구요.
const val = someObj?.someMethod() as string
someMethod가 string을 반환하는 함수인데 val이 string | undefined 로 잡혀서 타입 단언을 하신 것으로 보여요.
그런데 옵셔널 체이닝을 한 이상 undefined 타입을 가지게 되는 건 자연스러운 일입니다.
타입단언을 무작정 쓰게 되면 이렇게 자연스러운 타입도 가려버리게 됩니다.
그리고 이런데서 보통 버그가 발생해요.
(모르고 있다가 배포나갔는데 런타임 에러가 난다면 아찔하겠죠..?)
이 경우에는 string | undefined 타입 그대로 사용하거나(사용처에서 타입 가드) 널병합 연산자 등으로 확실하게 string을 넣어주거나 해주셔야합니다.
const val = someObj?.someMethod() ?? ''
as string => ?? ''
'.modal-container' => 'form'
즐겨찾기 필터링 테스트: like attribute가 true인지 여부 판별로 테스트 카테고리 필터링 테스트: category attribute가 선택한 카테고리와 일치하는지 여부 판별로 테스트
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.
안녕하세요 센트~
확인이 늦어서 죄송합니다ㅠㅠ
깔끔하게 잘 반영해주셨네요~
이번 미션은 이쯤에서 마무리해도 좋을 것 같습니다! 고생 많으셨어요!!
안녕하세요 브콜!! 센트입니다!
주말에 쉬시는데 방해되지 않게 최대한 빨리 해서 올려보려고 했는데 구현이 생각보다 오래 걸려서 결국 오늘 PR을 보내게 되네요😭😭
배포 링크
배포 사이트에 접속하셨을 때 혹시 localstorage에 지난 미션 배포를 확인하실 때 사용하셨던 데이터가 남아있다면 디테일 모달, 즐겨찾기 기능이 작동하지 않을 수 있습니다. 실행하시기 전에 localstorage를 비워주시고 해주시면 감사드리겠습니다!!
제가 이번 단계를 구현하면서 가장 신경썼던 부분은 컴포넌트를 적절히 분리해 각각의 역할을 수행할 수 있도록 하는 것이었습니다. 추가된 요구사항을 구현하기 위한 방식을 다음과 같이 생각해보았습니다.
like
를 추가해 즐겨찾기 여부를 식별할 수 있도록 한다.showState
에 boolean 타입의like
를 추가하고, 이를 변경함으로써 즐겨찾기한 음식점만을 필터링할 수 있도록 한다.innerText
등을 이용하여 추가해준다.이번 단계를 진행하며 저는 다음과 같은 궁금증이 생기게 되었습니다.
컴포넌트 분리가 적절히 이루어졌는지 여부
첫 리뷰 때 컴포넌트가 적절히 분리되지 않은 것 같다는 리뷰를 남겨 주셔서 컴포넌트란 어떤 식으로 분리되는 것이 적절할 지 고민하며 코드를 작성해보았습니다. 혹시 이번 단계에서는 컴포넌트 분리가 적절히 이루어졌는지 궁금합니다! 잘 이루어지지 않았다면 브콜이 생각하는 개선 방향 또한 이야기 해주시면 감사드리겠습니다😀😀
e2e 테스트 코드가 적절히 사용되었는지 여부
이번에 cypress를 통해 e2e 테스트를 처음으로 진행해보았습니다. 여러 기능들 중 핵심 기능이라고 생각하는 음식점 추가 기능, 각종 필터링 기능, 음식점 삭제 기능을 테스트하는 코드를 작성해보았습니다. 처음 해보는 e2e 테스트이다 보니 작성한 테스트 코드가 적절한지 여부를 판별하기 어려웠습니다. 브콜이 보았을 때 제 테스트 코드가 어떤지 궁금합니다!
타입 단언 줄이기
지난번에 타입 단언이 너무 많다는 리뷰를 해주셨는데 아직 타입스크립트가 미숙해 이 부분을 적절히 수정하지 못했습니다 ㅠㅠ 혹시 타입 단언을 줄이기 위해서 브콜은 어떤 방식을 주로 사용하시는지 궁금합니다😭😭😭