[1단계 - 맛있는 맛집 미션] - 마빈(김재영) 미션 제출합니다.

[spoyodevelop]

안녕하세요, 초파. 일단 바쁜시간 짬 내주셔서 리뷰를 해주신것 감사합니다.
조금 PR을 일찍 올리는 이유도 있어요. 여유시간이 많을수록, 더 많이 얻어가겠지요.
그럼, 리뷰 부탁드립니다.

학습 목표

이번 미션을 통해 다음과 같은 학습 경험들을 쌓는 것을 목표로 합니다.

• UI와 도메인 영역을 분리할 수 있는 설계를 고민해보고, 목적에 맞게 객체와 함수를 활용
• TDD 방식으로 개발하며, 단위 테스트 기반으로 점진적인 리팩터링

제출 전 체크 리스트

• 기능 요구 사항을 모두 구현했고, 정상적으로 동작하는지 확인했나요?
• 기본적인 프로그래밍 요구 사항을 준수하고 있는지 확인했나요?
• 테스트 코드는 모두 정상적으로 실행되나요?
• (해당하는 경우) 배포한 데모 페이지에 정상적으로 접근할 수 있나요?
  • 배포한 링크 기입: https://spoyodevelop.github.io/javascript-lunch

리뷰 요청 & 논의하고 싶은 내용

질문 하나 하나가, 중요할수도 있고, 그렇게 중요하지 않을수도 있으나, 답변이 조금 원활하고, 중요도가 높다고 생각하는 것을 먼저 답변해주시고, 남는 시간이 있으면, 추가적으로 답변 해주시면 됩니다.

E2E 시나리오 테스트 파일 관리 질문

E2E 테스트에 음식점 추가 폼이 잘 동작하는 지에 대한 여러 시나리오들이 있는데 E2E 테스트이다 보니깐 테스트 하나가 내용이 길어서 한 파일에 다 넣으면 가독성이 떨어지고 유지보수 하기 힘들 것 같다는 생각이 들었습니다.

또한, 저와 페어의 의견이 갈린 지점이 있습니다. 저는 최대한 테스트를 잘게 쪼개서 어느정도 유닛 테스트를 한뒤, 통합, e2e 테스트를 하는데요.

그런데, 만약 테스트 예산이 한정이 되어있다는하에, 만약, 최종적으로 e2e에서 모든 것을 체크한다고 하면, 기존에 있었던 통합, 유닛 테스트는 필요가 없다고 생각할수 있을까요?

cypress는 e2e 테스트를 하는, 그리고 실제로의 구현 방식도 유저가 클릭하는 것을 그대로 모방하여 매크로 처럼 동작을 하는 것 같던데, 이것 툴 자체가 유닛, 통합 테스트가 번거롭게 작용하는 것도 한몫하는 것 같구요.

쵸파의 생각이 어떤지 궁금합니다. cypress로 유닛, 통합도 하시는 편인가요?

DOM 접근 비용 관련 질문

기존 이벤트 핸들러 코드에서는 모달에 음식점 추가 폼(DOM 앨리먼트)이 없다면 새로 생성하고,
음식점 추가 폼이 있다면 DOM을 추가하지 않는 방식을 사용했었습니다.

하지만 이렇게 하면 이벤트가 발생할 때마다 restaurant-add-form(음식점 추가 폼)이 있는지 DOM에 접근해서 확인을 해줘야 해서 isFirstRender라는 상태를 생성해 모달이 열렸던 적이 있다면 폼을 생성하지 않는 방식으로 DOM 요소에 접근하는 방식을 최소화하였는데 혹시 더 좋은 방안이 있을까요??

• as-is(restaurant-add-form DOM이 있는지 체크)

function handleBottomSheetToggle(event) {
  const modal = document.querySelector(".modal");

  if (event.target.closest(".restaurant-add-button")) {
    modal.showModal();

    const restaurantAddForm = document.querySelector(".restaurant-add-form");

    if (!restaurantAddForm) {
      const modalContainer = document.querySelector(".modal-container");
      const restaurantFrom = createRestaurantForm();
      modalContainer.appendChild(restaurantFrom);
    }
  }

  if (event.target.closest(".modal-backdrop")) {
    modal.close();
  }
}

• to-be(isFirstRender 상태 사용)

function bottomSheetController() {
  let isFirstRender = false;

  function handleBottomSheetToggle(event) {
    const modal = document.querySelector(".modal");

    if (event.target.closest(".restaurant-add-button")) {
      modal.showModal();

      if (!isFirstRender) {
        const modalContainer = document.querySelector(".modal-container");
        const restaurantFrom = createRestaurantForm();

        modalContainer.appendChild(restaurantFrom);
        isFirstRender = true;
      }
    }

    if (event.target.closest(".modal-backdrop")) {
      modal.close();
    }
  }

  return { handleBottomSheetToggle };
}

css 워터폴

동료랑 진행을 코드를 짜던 중에, main css에 컴포넌트의 css파일을 import하는 과정이 있었습니다.

전혀 생각을 안하고 있었는데, 하나하나 임포트를 하고 어느정도의 워터폴이 생기는 것이 보였습니다.

결국은 조금 코드가 의아할수 있지만, html에 css를 임포트 하는 방식으로 진행을 했습니다.

애초에 처음부터 이것이 의미가 있는 작업인지, 혹시 워터폴이 발생할수 있는지에 대한 궁금증이 존재합니다.

피그마에 있는 그대로 구현 하는것이 좋은가요?

동료랑 이야기를 하던 중에, 현재 구현 사항에서는 사실상 컴포넌트 분리가 우선시되고, 기능 동작, 구현 사항에는 '피그마에 있는 기본값 레스토랑 리스트(피양콩컴퍼니,친친)'같은 것을 구현하라고 적혀지지 않았다는 것을 발견했습니다.

조금, 고민과 이야기를 나누었는데, 저는 클라이언트 입장에서 보면 굉장히 당황해 할것이라고 생각해, 해당 기본값 리스트를 구현을 해두는것이 중요하다고 생각했지만, 동료의 입장으로는 해당 사항이 step2에 구현해도 늦지 않다라고 생각 하고 있더군요.

한편, 동료의 입장도 이해가 됩니다. step2에 정렬을 하게 되면, 기존에 보관했던 레스트랑 리스트의 포멧과 자료 구조를 바꿔야 할수 있으니까요.

혹시 비슷한 사례가 있으면, 이야기를 나누어 줄수 있으실까요? 그리고, 이런 약간의 구현 사항의 불일치가 있어도, 클라이언트와 개발자의 소통의 레벨에 따라서 유연하게 넘어가는 편일까요?

---

✅ 리뷰어 체크 포인트

1단계

• UI를 컴포넌트 단위로 분리했는가?
• 재사용 가능한 컴포넌트를 고려했는가?
• 주요 기능 시나리오에 대한 E2E 테스트를 작성했는가?

2단계

• 오류 상황(예: 잘못된 입력)에 대해서도 테스트 케이스를 정의했는가?
• 타입 정의를 적절히 사용하여 컴파일 오류 없이 안정적으로 동작하는가?
• 로컬/배포 환경에서 테스트가 정상적으로 동작하는지 확인했는가?

[bassyu]

안녕하세요 마빈!
엄청 빠르게 미션 제출해주셨네요! 👏 👏 👏
이번 미션 잘 부탁드립니다~!

코드

깔끔하게 잘 구현해주셨네요! 👍
특히 html 요소의 메서드를 적극 활용해주신 것이 좋네요 💯

논의

E2E 시나리오 테스트 파일 관리 질문

UI는 워낙 사이드 이팩트가 많아서 E2E 테스트가 용이할 때가 많습니다!
그래서 저라면 복잡한 동작의 UI라면 E2E 테스트를,
결과가 명확하고 파라미터가 많고 엣지 케이스가 많은 함수라면 단위 테스트를 선택할 것 같습니다.
그리고 둘 다 아니라면, 둘 중 하나에 해당하는 테스트에 더 집중하는 것도 중요하다고 생각합니다~!

DOM 접근 비용 관련 질문

성능에 큰 이슈가 있지 않았고, 이미 다른 querySelector도 많기 때문에, 저는 가독성을 위해 오히려 최적화를 하지 않고 그냥 둘 것 같습니다~!

다른 방법으로는 form, input을 html로 미리 그려주는 방법이 있을 것 같은데 이 방법은 어떤가요~?

css 워터폴

css의 import 구분을 사용하고, 그 css 파일을 public에 넣은 경우 맞을까요?
그러면 워터풀로 동작하는 것이 맞습니다!
css 파일을 src 파일로 옮기고, npm run build, preview를 확인해보세요!

css 코드를 지금처럼 관리하게 되면 워터풀 말고도 어떤 단점이 있을까요?
이어서 번들러가 어떤 역할을 하는지 정리해 보는 것도 추천합니다~!

피그마에 있는 그대로 구현 하는것이 좋은가요?

목데이터 말씀하시는 것 맞을까요?
목데이터가 있으면, 저도 리뷰하기 용이하고 마빈도 리팩터링하거나 테스트하기에 편할 것 같습니다!

---

소소하게 코멘트 드렸으니, 다음 스텝에 반영해주셔도 좋을 것 같습니다!
2단계에서 더 많은 이야기 나눠봐요!
수고하셨습니다 마빈! 💪 💪

[bassyu]

오 요런 것도 있었군요 👀

[spoyodevelop]

같이 페어 했던 써밋이 추천해준 linter+formatter 통합 솔루션입니다. 전반적으로 포맷팅이 매우 빠르게 동작하며, 따로 하나하나 규칙을 지정해 줄 필요 없이,

"rules": {
      "recommended": true
    }

이렇게 하면, 스마트 하게, 조금 이상한 코드 있으면 짚어 줍니다.
한번 써봤는데, 나름 좋더라구요.

[bassyu]

사소하지만 지금은 'false'라는 클래스가 들어가겠네요!
createElement 에서 falsy한 값을 필터링 하는 것 어떤가요?

Suggested change

	className: ["form-item", `${required && "form-item--required"}`],
	className: ["form-item", required && "form-item--required"],

[spoyodevelop]

해당 부분 수정 했습니다. 일관성을 위해서, required에 기본값을 false로 주는 것도 집어 넣었어요.

[bassyu]

본문에도 코멘트 드렸지만, css 파일의 위치를 다시 고민해보면 좋을 것 같아요!

[spoyodevelop]

전반적으로 재배치 했고, component에 이제 각각 css가 붙어 있습니다.

[bassyu]

html과 createElement는 어떤 기준으로 나눠주신 건지 궁금합니다~!

[spoyodevelop]

일단 컴포넌트 단위로, 재사용 할수 있는것은 createElement를 사용했고요,
아닌것은 그냥 html태그로 적어 넣었습니다.

추가적으로, 동적으로 element를 바꾸는 것들은 createElement를 사용해서 구현했습니다.

[bassyu]

엣지 케이스 좋네요 👍 👍
300자를 미리 알려주는 UI가 있으면 좋을 것 같습니다~!

[spoyodevelop]

placeHolder요! 추가 했습니다.
토스트 팝업도 추가 할 예정이긴 합니다.

[bassyu]

폴더에 파일 하나만 있는 경우가 많네요!
전체적으로 딱 1뎁스만 줄이는 것은 어떤가요~?

[spoyodevelop]

전반적인 css 파일 체계를 수정했습니다. 지금은 크게 보실필요 없지만, 2단계 때 확인해보세요!