Skip to content

[1단계 - 음식점 목록] - 상추(이상희) 미션 제출합니다.#185

Merged
Creative-Lee merged 31 commits intowoowacourse:sanghee01from
sanghee01:step1
Mar 14, 2025
Merged

[1단계 - 음식점 목록] - 상추(이상희) 미션 제출합니다.#185
Creative-Lee merged 31 commits intowoowacourse:sanghee01from
sanghee01:step1

Conversation

@sanghee01
Copy link
Copy Markdown
Member

@sanghee01 sanghee01 commented Mar 7, 2025

안녕하세요 도밥! 이번에 점심 뭐 먹지 미션 리뷰를 받게 된 상추입니다! 잘 부탁드립니다 ☺️

학습 목표

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

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

제출 전 체크 리스트

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

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

1) 이번 단계에서 가장 많이 고민했던 문제와 해결 과정에서 배운 점

1. 컴포넌트 구조 설계

컴포넌트를 어떻게 나누고, 이를 일관성 있게 관리할 수 있을지에 대한 고민이 있었습니다. 특히, JavaScript만을 사용하여 컴포넌트를 어떻게 모듈화할지에 대한 방향을 정하는 것이 쉽지 않았습니다.

처음에는 각 컴포넌트를 함수로 만들고, createElement나 innerHTML을 이용하여 내부 요소를 생성하는 방식을 고려했습니다. 하지만 이러한 방식은 컴포넌트 간 일관성이 떨어질 수 있다는 문제가 있었습니다. 이를 해결하기 위해 클래스 기반의 컴포넌트 구조를 도입하고, 공통적인 기능을 제공하는 코어 컴포넌트(./core/Component.js)를 설계하였습니다. 모든 컴포넌트가 이 코어 컴포넌트를 상속하도록 하여 컴포넌트의 생명 주기(setup, render, onRender) 메서드의 실행 순서를 보장하고, 코드의 일관성을 유지할 수 있도록 했습니다.

또한, 컴포넌트 내부에서 다른 컴포넌트를 포함하는 방식을 구현하는 과정에서 어려움이 있었습니다. 처음에는 자식 컴포넌트가 부모 요소를 직접 append하는 방식을 고려했지만, 이는 부모-자식 간의 종속성을 강하게 만들어 유지보수에 어려움을 줄 수 있었습니다. 이에 따라 부모 컴포넌트가 자식 컴포넌트를 관리하는 방식으로 변경하여 종속성을 줄이고 코드의 유지보수성을 높였습니다.

2. 상태 관리 및 렌더링 방식

공통적인 상태를 어떻게 관리할 것인가도 중요한 고민이었습니다. 예를 들어, 사용자가 모달을 통해 식당을 추가하면, 이 정보가 식당 목록에 반영되어야 합니다. 이를 위해 전역에서 상태를 관리하는 Application 컴포넌트를 만들고, restaurantList라는 상태를 정의한 후 이를 식당 목록과 모달 컴포넌트에 props로 전달하는 방식으로 문제를 해결했습니다.

또한, 상태가 변경되었을 때 어떻게 리렌더링을 처리할 것인지에 대한 고민도 있었습니다. 이를 위해 코어 컴포넌트에서 setState 메서드를 구현하여, 상태가 변경될 때마다 render를 호출하는 방식으로 컴포넌트를 다시 렌더링하도록 했습니다. 이 방식 덕분에 상태 변경이 발생하면 자동으로 UI가 업데이트되도록 만들 수 있었습니다.

3. this 바인딩 문제

컴포넌트 내부에서 이벤트 핸들러를 설정할 때, this의 동작 방식을 제대로 이해하지 못해 문제가 발생했습니다. 특정 이벤트 핸들러에서 this가 의도와 다르게 설정되어 예상과 다른 동작이 발생하는 경우가 있었습니다. 이는 함수가 호출될 때의 컨텍스트에 따라 this가 결정된다는 점을 간과했기 때문이었습니다. 이를 해결하기 위해 bind 처리를 통해 원하는 컨텍스트를 유지하도록 했으며, 이를 통해 this의 동작 원리를 더 명확하게 이해할 수 있었습니다.

4. createElement와 innerHTML 선택

컴포넌트를 동적으로 생성하는 방식에서도 고민이 있었습니다. createElement를 사용하면 태그를 직접 생성해야 해서 가독성이 떨어진다는 단점이 있었습니다. 반면, innerHTML을 사용하면 HTML을 한 번에 업데이트할 수 있어 가독성이 좋아지지만, XSS 공격에 취약하고, 성능 저하의 우려가 있었습니다. 그래서 앞으로 개선할 점으로 innerHTML로 내부 돔 트리를 문자열로 삽입하기 전에 이스케이프 처리 (<,>같은 XSS 보안에 악용하는데 활용할 수 있는 문자) 하는 것을 생각하고 있습니다.

2) 이번 리뷰를 통해 논의하고 싶은 부분

  1. e2e 테스트 코드를 작성하는 게 아직 어색하다보니, 끼워맞추기로 작성된 느낌이 있습니다. 예를 들어, 모달의 취소 버튼을 클릭한 후 메인 화면으로 돌아가는지를 특정 문자가 존재하는지로 판단하고 있는데, 이보다 더 나은 방식이 있을지 궁금합니다.

     it("취소 버튼을 누르면 메인 화면으로 돌아간다.", () => {
        cy.get("#modal-cancel").click();
    
        cy.contains("새로운 음식점");
      });
  2. 현재 컴포넌트들을 모두 Component 클래스를 상속받는 방식으로 만들고 있습니다. 하지만 클린 코드 관련 서적에서는 상속보다는 조합(composition)을 활용하라고 권장하는 경우가 많습니다. 상속을 사용할 경우 부모 컴포넌트의 특정 메서드나 속성이 변경되면, 이를 상속받는 모든 자식 컴포넌트에도 영향을 미쳐 예상치 못한 문제가 발생할 가능성이 있기 때문입니다. 현재 코드에서 상속을 유지하는 것이 적절한지, 혹은 조합을 활용하는 것이 더 나은 선택일지 의견을 듣고 싶습니다.

  3. 정적인 요소들도 동적으로 컴포넌트로 만들어서 append하는 것이 적절한지 고민이 있습니다. 현재 방식이 유지보수성과 가독성 면에서 좋을 수는 있지만, 프로젝트 규모가 커지면 불필요한 DOM 조작이 많아져 성능상의 문제가 발생할 가능성이 있을 것 같습니다. 이런 경우, 정적인 요소들은 HTML에 미리 작성해두는 것이 더 적절할지, 아니면 가독성을 고려해 현재 방식대로 유지하는 것이 좋을지 고민이 됩니다.

  4. 마지막으로, 현재 컴포넌트 구조가 적절하게 나누어졌는지에 대한 피드백도 받고 싶습니다. 구조적으로 더 개선할 수 있는 부분이 있다면 의견 부탁드립니다!


✅ 리뷰어 체크 포인트

1단계

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

2단계

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

sanghee01 and others added 12 commits March 4, 2025 15:42
Co-authored-by: guesung <gueit214@gachon.ac.kr>
Co-authored-by: guesung <gueit214@gachon.ac.kr>
Co-authored-by: guesung <gueit214@gachon.ac.kr>
Co-authored-by: guesung <gueit214@gachon.ac.kr>
Co-authored-by: guesung <gueit214@gachon.ac.kr>
Co-authored-by: guesung <gueit214@gachon.ac.kr>
Co-authored-by: guesung <gueit214@gachon.ac.kr>
Co-authored-by: guesung <gueit214@gachon.ac.kr>
Co-authored-by: Kuesung Park <gueit214@naver.com>
Co-authored-by: guesung <gueit214@gachon.ac.kr>
Co-authored-by: Kuesung Park <gueit214@naver.com>
Co-authored-by: guesung <gueit214@gachon.ac.kr>
@sanghee01 sanghee01 marked this pull request as draft March 7, 2025 04:47
@sanghee01 sanghee01 changed the title [1단계 - 음식점 목록] - 상추(이상희) 미션 제출합니다. (Draft) [1단계 - 음식점 목록] - 상추(이상희) 미션 제출합니다. Mar 7, 2025
@sanghee01 sanghee01 marked this pull request as ready for review March 7, 2025 07:58
Copy link
Copy Markdown

@Creative-Lee Creative-Lee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 상추 👋 (상추라... 상추를 좋아하시나요? ㅋㅋㅋ... 유추가 어렵군요)
점심뭐먹지 미션 리뷰어 도밥입니다~! 잘 부탁드려요💪


가장 많이 고민하셨던 부분에 대해 이야기해보자면,

1️⃣ 컴포넌트 구조 설계
처음에는 각 컴포넌트를 함수로 만들고, createElement나 innerHTML을 이용하여 내부 요소를 생성하는 방식을 고려했습니다. 하지만 이러한 방식은 컴포넌트 간 일관성이 떨어질 수 있다는 문제가 있었습니다.
이를 해결하기 위해 클래스 기반의 컴포넌트 구조를 도입하고, 공통적인 기능을 제공하는 코어 컴포넌트(./core/Component.js)를 설계하였습니다. 모든 컴포넌트가 이 코어 컴포넌트를 상속하도록 하여 컴포넌트의 생명 주기(setup, render, onRender) 메서드의 실행 순서를 보장하고, 코드의 일관성을 유지할 수 있도록 했습니다.

함수 + createElement / innerHTML 을 사용했을 때, 일관성이 떨어졌던 구체적인 사례가 있었으면 좋았을 것 같아요. 지금의 정보만으로는 클래스 + 코어 컴포넌트 설계에 대한 이유로 크게 와닿지는 않는것 같아요. 기존 구조가 어땠는지 궁금하네요.

또한 컴포넌트에 생명주기와 같은 개념이 필요하게 된 이유가 무엇인지,
왜 그 메서드들의 실행순서가 보장되어야 했는지,
마지막으로 어떤 부분에서 기존 코드 대비 일관성이 유지되었다고 생각했는지도 궁금합니다.

또한, 컴포넌트 내부에서 다른 컴포넌트를 포함하는 방식을 구현하는 과정에서 어려움이 있었습니다. 처음에는 자식 컴포넌트가 부모 요소를 직접 append하는 방식을 고려했지만, 이는 부모-자식 간의 종속성을 강하게 만들어 유지보수에 어려움을 줄 수 있었습니다. 이에 따라 부모 컴포넌트가 자식 컴포넌트를 관리하는 방식으로 변경하여 종속성을 줄이고 코드의 유지보수성을 높였습니다.

의존의 방향을 바꾸어서 적절히 해결해주셨다고 생각해요.
관리방식이 변경됨에 따라, 아마도 자식 컴포넌트를 재사용 할 수 있게 되었을 것 같네요.
다만, Modal 컴포넌트에 의존과 관련되 의문이 있어 코멘트를 남겼으니 확인해주세요~!

2️⃣ 상태 관리 및 렌더링 방식
공통적인 상태를 어떻게 관리할 것 인가도 중요한 고민이었습니다. 예를 들어, 사용자가 모달을 통해 식당을 추가하면, 이 정보가 식당 목록에 반영되어야 합니다. 이를 위해 전역에서 상태를 관리하는 Application 컴포넌트를 만들고, restaurantList라는 상태를 정의한 후 이를 식당 목록과 모달 컴포넌트에 props로 전달하는 방식으로 문제를 해결했습니다.
또한, 상태가 변경되었을 때 어떻게 리렌더링을 처리할 것인지에 대한 고민도 있었습니다. 이를 위해 코어 컴포넌트에서 setState 메서드를 구현하여, 상태가 변경될 때마다 render를 호출하는 방식으로 컴포넌트를 다시 렌더링하도록 했습니다. 이 방식 덕분에 상태 변경이 발생하면 자동으로 UI가 업데이트되도록 만들 수 있었습니다.

적절한 고민을 해주셨는데요.
랜더링 관점에서 해결해주신 방식(setState 구현 및 render 호출) 외에 어떤 방법들을 고민했었는지도 알려주시면 좋을 것 같아요.
최종적으로 선택한 이유도 궁금하고요.
(관련해서는 랜더링 성능 관점에서의 코멘트를 코드에 남겨드렸습니다.)

4️⃣ this 바인딩 문제
컴포넌트 내부에서 이벤트 핸들러를 설정할 때, this의 동작 방식을 제대로 이해하지 못해 문제가 발생했습니다. 특정 이벤트 핸들러에서 this가 의도와 다르게 설정되어 예상과 다른 동작이 발생하는 경우가 있었습니다. 이는 함수가 호출될 때의 컨텍스트에 따라 this가 결정된다는 점을 간과했기 때문이었습니다. 이를 해결하기 위해 bind 처리를 통해 원하는 컨텍스트를 유지하도록 했으며, 이를 통해 this의 동작 원리를 더 명확하게 이해할 수 있었습니다.

한번쯤은 맞아봐야 비로소 이해하게되는 this문제를 겪으셨군요. 고생하셨습니다😭
겪어 봤으니, 앞으로 디버깅이 수월할거라고 생각합니다~! ㅎㅎ


논의하고 싶었던 부분도 마저 보면,

e2e 테스트 코드를 작성하는 게 아직 어색하다보니, 끼워맞추기로 작성된 느낌이 있습니다. 예를 들어, 모달의 취소 버튼을 클릭한 후 메인 화면으로 돌아가는지를 특정 문자가 존재하는지로 판단하고 있는데, 이보다 더 나은 방식이 있을지 궁금합니다.

우선 모달의 취소버튼 클릭으로 홈으로 돌아가야 한다를 하나의 e2e 테스트로 작성해주신 이유가 궁금해요. 상추가 e2e 테스트의 범위나 기준을 어떻게 잡으셨는지를 먼저 듣고싶습니다.
'describe를 쪼갠 기준' 이라던지, '이 흐름을 테스트해야겠다' 라던지, 생각하셨던 부분을 알려주시면 좋겠습니다😄 (특정 행동의 판단 기준에 대해서는 그 다음에 이야기 나눠보도록해요~!)

현재 컴포넌트들을 모두 Component 클래스를 상속받는 방식으로 만들고 있습니다. 하지만 클린 코드 관련 서적에서는 상속보다는 조합(composition)을 활용하라고 권장하는 경우가 많습니다. 상속을 사용할 경우 부모 컴포넌트의 특정 메서드나 속성이 변경되면, 이를 상속받는 모든 자식 컴포넌트에도 영향을 미쳐 예상치 못한 문제가 발생할 가능성이 있기 때문입니다. 현재 코드에서 상속을 유지하는 것이 적절한지, 혹은 조합을 활용하는 것이 더 나은 선택일지 의견을 듣고 싶습니다.

PR 상단 '고민했던 부분'에서 상추는 필요한 구조화를 코어 클래스 Component를 통해 이뤄냈다고 말씀하셨고, 이에 상속을 사용하셨어요.
서적의 권장사항이나 저의 조언 이전에, 상추가 생각했을때는 지금의 상속 구조가 어떻게 느껴지나요? 또, 상추는 왜 상속을 사용하셨나요? 그 기준에 대해 먼저 설명해주시면 좋겠어요.
(당장 코드에서 상속 vs 조합 선택한다면? - 이 질문에는 '저도 잘 모르겠습니다' 라고 답변하겠습니다... 😅 정말 저도 잘 모르겠어요)

정적인 요소들도 동적으로 컴포넌트로 만들어서 append하는 것이 적절한지 고민이 있습니다. 현재 방식이 유지보수성과 가독성 면에서 좋을 수는 있지만, 프로젝트 규모가 커지면 불필요한 DOM 조작이 많아져 성능상의 문제가 발생할 가능성이 있을 것 같습니다. 이런 경우, 정적인 요소들은 HTML에 미리 작성해두는 것이 더 적절할지, 아니면 가독성을 고려해 현재 방식대로 유지하는 것이 좋을지 고민이 됩니다.

음... 가뜩이나 다양한 구현이 가능한 js에서 정답은 없다고 생각해요.
다른것보다는 상추가 왜 정적인 요소임에도 동적으로 append 했는지가 중요할것 같은데요~!
지금의 방식이 가독성과 유지보수가 편리하다 라고 생각하신게 맞지요?
다음으로는 이런 이점들이 단점을 상쇄할만큼 더 중요하다라고 생각하셨는지요?

마지막으로, 현재 컴포넌트 구조가 적절하게 나누어졌는지에 대한 피드백도 받고 싶습니다. 구조적으로 더 개선할 수 있는 부분이 있다면 의견 부탁드립니다!

전체적으로 조각들을 잘 나누어주셨다고 생각합니다.
여기서 조금 더 개선해보자면, Modal, Header 같은 컴포넌트인데요
현재는 음식점이라는 도메인 내용을 잔뜩 품고 있어서 다른 도메인에서 재사용이 불가능할 것 같아요.
공통컴포넌트로 분리 해보는 시도를 하셔도 좋겠습니다. (e.g. 지금의 Button 컴포넌트처럼)


고생많으셨습니다 상추 💪
PR 답문과 코드 코멘트로 질문을 엄청 해두었는데, 모두 상추의 생각이 궁금해서 남긴 질문이니 꼭 답변해주셨으면 좋겠습니다~!
코멘트도 반영해서 재요청주세요😄

<div class="form-item ${
this.props?.isRequired ? "form-item--required" : ""
}">
<label for="category text-caption">${this.props.label}</label>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이러면 라벨태그의 기능이 정상적으로 동작하나요?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

앗 이부분 놓쳤었네요..! 수정 했씁니다!! label for 안에는 하나의 값만 들어가야 한다는 것도 알게 됐네요.

fix: input label이 정상적으로 동작되도록 수정


setup() {
this.setState({
restaurantList: [
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

기본 데이터들은 별도로 관리되어도 될것 같아요.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +66 to +73
template() {
return `
${new Header({ title: "오늘 뭐 먹지" }).template()}
${new RestaurantList({
restaurantList: this.state.restaurantList,
}).template()}
`;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

컴포넌트의 초기화를 여러곳에서 하는것 같아 보이는데요
template 함수 return문에 다른 컴포넌트의 template return결과를 넣는것과
onRender 함수에서 직접 this.element.appendChild() 하는 것은 어떤 차이가 있나요?

new Modal().template()이 App의 template 함수에 들어가면 안되는건가 의문이 들었어요.

Comment on lines +75 to +80
addRestaurant(restaurant) {
this.setState({
...this.state,
restaurantList: [...this.state.restaurantList, restaurant],
});
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

음식점을 하나 추가했을 뿐인데,
Header와 RestaurantList 클래스의 새 인스턴스를 초기화 하고 DOM을 새로 그려야하는데요
구조를 설계하실 때 랜더링 비용에 대한 부분도 고려 하셨을까요?

Comment on lines +29 to +30
cy.get("#description").type("설명입니다");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit]
url이 빠진 이유가 있을까요~?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아뇨..!! 누락된 것 같습니다..😇 추후 e2e 다시 작성할 때 수정해볼게요!

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이렇게 한번에 파일을 묶어 export 한 이유가 있으실까요? 😄

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 step2 요구사항을 만족한 후 학습해봅시다.
(우선순위는 낮습니다. - 어떤 장단이 있는지 정도만 파악하면 될것 같아요.)

Comment on lines +18 to +19
if (!this.#element) this.#element = document.createElement("div");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

DOM 구조를 더 깔끔하게 정리할 수 있을 것 같아요.
물론 사용자는 알 수 없겠지만, 우리는 알고 있으니까요 ㅋㅋ..

(ps. 18~ 19 line 구현을 바꾸라는 말이 아닙니다~!!)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

어맛.. 이 부분을 놓쳤네요 수정했습니다!!
다만 뭔가 중첩된 div도 제거하면 좋을 것 같은데 이건 추후 고려해볼게요!!

refactor: 불필요한 html 태그 제거

Comment on lines +4 to +5
const $app = document.querySelector("#app");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit]
#app 이란 dom이 없다면 어떻게 될까요?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 step2 요구사항을 만족한 후 조금 더 고민해봅시다~!
(TS와도 연관있는 부분이라 수정될수도 있겠네요)

  • querySelector로 찾아온 dom은 항상 존재한다고 가정할 수 있을지?

Comment on lines +91 to +93
const $addRestaurantButton = this.parent.querySelector(".gnb__button");
const $modal = this.element.querySelector(".modal");
const $modalCancelButton = this.element.querySelector("#modal-cancel");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

반복되는 querySelector 함수를 작은 유틸 함수로 만들어 놓으면 중복을 줄이고 코드를 깔끔하게 정리할 수 있어요.

const $overlay = $('.overlay')
const $resultRow = $$('.result__row')

js에서 $$$를 관용적으로 querySelector, querySelectorAll 등에 사용하곤 한답니다. (jquery의 영향인것으로 알고있어요)
(브라우저 콘솔에서도 사용 가능한데, 이스터에그같은 개념인걸로 알고있습니다 ㅋㅋ)

#state;
#props;

constructor(props, parent) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parent는 어떤 용도인가요?

@sanghee01
Copy link
Copy Markdown
Member Author

안녕하세요 도밥!! ㅎㅎ 제 닉네임이 상추🥬인 이유는 별다른 이유는 없고... 닉네임 고민하다가 제 이름 중에 '상'자가 들어가서 상....상... 상추..!! 해서 정해졌답니다 ㅎㅎ 😆

도밥이 PR 답변 본문에 작성해주신 질문에 대한 답변과, 코드는 급한대로 자잘한 부분만 수정했어요!! 수정된 부분은 도밥이 코멘트 달아주신부분에 댓글로 달아놨습니다. 나머지는 step2에서 고민해볼게요!! 기다려주셔서 정말 감사합니다🥹


함수 + createElement / innerHTML 을 사용했을 때, 일관성이 떨어졌던 구체적인 사례가 있었으면 좋았을 것 같아요. 지금의 정보만으로는 클래스 + 코어 컴포넌트 설계에 대한 이유로 크게 와닿지는 않는것 같아요. 기존 구조가 어땠는지 궁금하네요.

함수 방식과 class 중에 어떤 방식을 채택해야할지, 또 채택한다면 어떻게 구현해나가야할지 정말 고민이 됐고 막연했는데요,

사실 컴포넌트 설계 초반엔 거의 페어의 도움으로 아이디어를 생각해내서, 왜 그런 방식으로 구현해갔는지는 제가 완전하게 설명할 수 있을지 모르겠네요..😭 그래도 제가 페어에게 설득 당한대로, 또 제가 따로 이해한대로 최대한 말씀드려보겠습니다!!

사실 저는 함수 + createElement / innerHTML 을 사용해서 컴포넌트를 만들어 본 경험이 없어서, 일관성이 떨어졌던 구체적인 사례가 무엇이 있는지 잘 모르긴 하는데요..!

그래도 나름 제 추측을 해보자면, 해당 방식대로 한다면, 그 컴포넌트에만 필요한 기능들을 각각 만들어서, 컴포넌트 간의 일관성이 없을 것 같습니다.

예를 들어, 현재 Header 컴포넌트를 보면, 그저 정적으로 DOM을 입히기만 하면 돼서 DOM 코드를 반환하는 함수만 가지면 됩니다.

근데, Modal 컴포넌트는 DOM을 반환하는 것 뿐만 아니라, 음식점 목록에 음식 목록을 추가해야하므로 음식점 목록에 대한 상태관리도 해야합니다.

그러다보면 각 컴포넌트간에 일관성이 없어질 것 같습니다. 만약 컴포넌트의 각 기능(DOM 반환, 상태 업데이트 등)을 따로 유틸 함수로 만들어 사용한다해도, 각 컴포넌트는 각자 필요한 함수들을 불러와서 사용할 것입니다. 그래서 컴포넌트들이 일관성이 없을 것 같다는 생각이 듭니다!

만약 class를 사용한다면, 부모 class에 모든 기능들을 선언해놓고, 자식들은 부모로부터 상속받아 필요한 기능만 오버라이딩해서 사용하면 되므로, 일관적이라고 생각이 듭니다.

따라서 class가 이렇게 상태와 메서드로 관리하기 용이하기에 class로 작성하게 된 것 같습니다!

또한 컴포넌트에 생명주기와 같은 개념이 필요하게 된 이유가 무엇인지, 왜 그 메서드들의 실행순서가 보장되어야 했는지.

맨 처음에는 컴포넌트를 현재 코드 기준 template 메서드, 즉, DOM 요소를 반환하기만 하는 메서드만 있었습니다. 그렇게하니.. 기능 구현 과정에서 필요한 생명주기 메서드가 필요하더군요! 그 생각 흐름을 말씀드리자면 다음과 같아요.

  1. 지금은 DOM 요소만 반환하고 있는데, 만약 그 return할 DOM 요소가 event를 감지하고 있어야 한다면? 혹은 render 된 후에 특정 기능을 처리해야한다면? → template를 render 메서드로 실행한 뒤에, onRender 메서드로 eventListener 함수를 입히는 등, 후처리를 하자!

  2. 근데 onRender 메서드가 실행되려면, 일단 해당 DOM요소가 있어야하니 render 메서드를 먼저 실행 시켜야 해!

  3. 어? 음식점 추가를 하고 음식 리스트 목록에 업데이트는 어떻게 해야하지? → 음식점 목록을 상태(state)로 만들고, setState 메서드를 만들어 상태를 업데이트를 관리하자! 일단 초기 상태를 등록해두기 위해 setup 메서드를 만들자!

그래서 지금 Component 클래스를 보시면 앞서 말한 메서드들로 구성되어 있습니다!

자식들은 이 부모 컴포넌트(Components)를 상속받고, 초기화 될 때 constructor 내부에 있는 필드값들이 자동으로 설정되고, 메서드들도 실행이 돼서, 필요한 메서드들만 오버라이딩해서 사용하면 됩니다! 이 방식이 유지보수하기도 좋고 확장성에도 좋다고 느껴져 개인적으로 정말 용이하다고 느껴졌네요!!

마지막으로 어떤 부분에서 기존 코드 대비 일관성이 유지되었다고 생각했는지도 궁금합니다.

이는 첫번째 질문의 답변으로 말씀드린 것 같네용!

랜더링 관점에서 해결해주신 방식(setState 구현 및 render 호출) 외에 어떤 방법들을 고민했었는지도 알려주시면 좋을 것 같아요. 최종적으로 선택한 이유도 궁금하고요.

홍홍.. 이것 또한 사실 막연하게 생각했다가.. 자랑스러운 저의 페어가 아이디어를 고안해줬는데요, 이것도 제가 이해한대로 말씀드려보겠습니다! 다른 방법은 생각하지 못했습니다.

일단 상태(state)가 업데이트 되려면, 업데이트 된 상태가 될 변수에 담아야하고, 그 변수를 기반으로 다시 화면을 업데이트 해줘야 합니다.

다시 화면을 업데이트 하는건 기존에 Component 클래스에 있던 메서드인 render를 활용하면 되겠다는 생각이 들었습니다.

그래서 ‘setState 구현 및 render 호출 방식’이 이미 있는 state 필드 변수와 메서드를 활용하면 되기에 해당 방식으로 선택했습니다!

우선 모달의 취소버튼 클릭으로 홈으로 돌아가야 한다를 하나의 e2e 테스트로 작성해주신 이유가 궁금해요. 상추가 e2e 테스트의 범위나 기준을 어떻게 잡으셨는지를 먼저 듣고싶습니다. 'describe를 쪼갠 기준' 이라던지, '이 흐름을 테스트해야겠다' 라던지, 생각하셨던 부분을 알려주시면 좋겠습니다😄 (특정 행동의 판단 기준에 대해서는 그 다음에 이야기 나눠보도록해요~!)

제가 생각했던 e2e 테스트 범위와 기준은

‘사용자가 이 사이트를 사용한다면 어떤 시나리오를 진행할까?’으로 잡았습니다.

일어날 수 있는 시나리오를 크게 ‘식당 목록 보이기’, ‘식당 추가하기’로 잡았어서 describe를 이 둘로 쪼갰습니다. 그리고 시나리오대로 테스트코드를 작성했습니다! 그 시나리오는 다음과 같고, 해당 내용으로 테스트를 작성했습니다.

  1. 사이트에 들어가면 식당 목록이 보여야한다.
  2. 식당 추가 버튼을 누르면 모달창이 떠야한다
  3. 모달창에서 식당 정보들을 입력하고 추가 버튼을 누르면 식당 리스트가 추가된다
  4. 모달창에서 취소버튼을 누르면 메인 화면으로 돌아간다.

그래서 지금 e2e 테스트 부분 코드를 보면 크게 위 4가지로 되어있는 것 같습니다..!

PR 상단 '고민했던 부분'에서 상추는 필요한 구조화를 코어 클래스 Component를 통해 이뤄냈다고 말씀하셨고, 이에 상속을 사용하셨어요. 서적의 권장사항이나 저의 조언 이전에, 상추가 생각했을때는 지금의 상속 구조가 어떻게 느껴지나요? 또, 상추는 왜 상속을 사용하셨나요? 그 기준에 대해 먼저 설명해주시면 좋겠어요. (당장 코드에서 상속 vs 조합 선택한다면? - 이 질문에는 '저도 잘 모르겠습니다' 라고 답변하겠습니다... 😅 정말 저도 잘 모르겠어요)

하하 일단, 저희가 상속을 사용한 이유는 앞선 질문에 대한 답변으로 잘 설명한 것 같네요..!!!!

사실 제가 테코톡 주제로 ‘상속과 조합’을 잡아서 관련돼서 당시에 공부를 하고 있었는데용ㅎㅎㅎㅎ…

사실 이 질문을 남길때 쯤에는 아직 제대로 공부하지 못한 상태였습니다. 다만 책의 목차나 대강적으로 찾아보면 ’상속을 사용하지말고 조합을 사용하라’ 라는 문장이 많이 보여서 너무너무 궁금증이 들어서 좀 급박하게 질문을 드린 것 같네요ㅎㅎ..ㅎ..!!!😓

근데 지금 발표까지 마치며 생각해보니, 상속은 ‘is-a’ 관계일때는 사용해도 괜찮고, 현재 부모 컴포넌트를 상속받아 사용하는 구조가 ‘is-a’ 관계인 것 같아 상속을 사용해도 괜찮은 것 같아요! 또한, 현 상황에서는 조합보다 상속이 더 자연스럽고 가독성과 유지보수성이 좋을 것 같습니다.

음... 가뜩이나 다양한 구현이 가능한 js에서 정답은 없다고 생각해요. 다른것보다는 상추가 왜 정적인 요소임에도 동적으로 append 했는지가 중요할것 같은데요~! 지금의 방식이 가독성과 유지보수가 편리하다 라고 생각하신게 맞지요? 다음으로는 이런 이점들이 단점을 상쇄할만큼 더 중요하다라고 생각하셨는지요?

네, 가독성과 유지보수가 편리할 것 같아서 동적으로 append한게 맞습니다!

아무래도.. 현재 미션 규모에서는 성능상의 문제는 미미할것 같다는 생각이 들어서 모두 동적으로 만들었습니다.

다만, 큰 규모의 프로젝트에서는 어떻게 사용될까? 궁금해서 여쭤본건데, 생각해보니 .. 역시 상황에 따라 다르겠군용 ㅋㅋ..

Copy link
Copy Markdown

@Creative-Lee Creative-Lee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 상추~ㅎㅎ
테코톡까지 병행하신다고 고생많으셨습니다!
논의했던 것처럼 우선stpe2 진행하고계시면 천천히 step1리뷰 남겨두도록 할게요!
(중요한 부분 위주로 반영해주신걸로 보입니다)
화이팅입니다! 달려보셔요👍🏃‍♂️‍➡️

@Creative-Lee Creative-Lee merged commit 2b4ef04 into woowacourse:sanghee01 Mar 14, 2025
Copy link
Copy Markdown

@Creative-Lee Creative-Lee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

step2에서 마저 생각해보면 좋을 코멘트 표시해두었습니다~!

PR 본문 답변에 대해서는

  1. 함수 + createElement / innerHTML 요런방식으로 작성했을 때 일관성이 떨어졌던 사례.
    추측해서 말씀해주신 부분에 대해서 합당한 사고라고 생각합니다.
    여기에 더해서 위와 같은 방법으로 작성했을 때의 장점 또한 추측해보시면 도움이 될 것 같습니다~!

  2. 생명주기 개념 도입 배경, 랜더링 관점에서 setState 구현 및 render 호출 외에 생각해본것
    일련의 시간 순서대로 로직을 실행해야 했을 때, 이때 이미 DOM을 템플릿 형태로 return 하도록 하는 구조가 완성되어 있었던 것 같아요. DOM을 return하고 실제 부모가 되는 컴포넌트에(DOM) 부착 하고 나서야 이어서 이벤트를 부착할 수 있으니 onRender 같은 후처리 콜백이 필요했을것이고요.
    이런 동작은 모든 컴포넌트에 동일하게 필요하니, 최종적으로 (어딘가 익숙한) 생명주기 메서드들이 탄생했다고 이해했습니다. 이후 state와 setState등의 랜더링 처리도 비슷한 구조에서 자연스럽게 추가되었다고 이해했고요. 코드의 작성 이유가 어느정도 이해되는 흐름이긴합니다.
    한편으로는 구조에 대한 의심을 가지고, 컴포넌트가 DOM을 직접 생성하고 그 안에서 이벤트를 달고 그 DOM을 내보내는 등의 구조 변경 시도도 해봤으면 어땠을까? 하는 조금 아쉬운 마음도 들었네요 ㅎㅎ

  3. e2e 테스트 기준
    이 부분은 적당한 기준을 잘 잡고 계신것 같네요.
    최초 질문에 대한 답변을 드리자면,
    사실 해당 케이스에 대한 확인을 위해 메인 화면에 있는 DOM의 문자열 존재 여부를 확인하는 것은 어쩔 수 없다고 생각해요.(딱히 방법이 없으니...) 그나마 단단하게 수정한다면,
    클릭 이후에 모달 DOM이 사라졌는지도 확인하고, 메인화면에서는 가장 수정이 덜 일어날 것 같은 DOM (케바케지만, header의 타이틀 같은것이 될수 있겠죠)을 선택해서 확인하는것도 좋겠네요.

import InputBox from "./InputBox.js";
import { FOOD_CATEGORY } from "../constants/constants.js";

class Modal extends Component {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코멘트는 step2 요구사항을 만족한 후 시간이 남으면 그때 더 고민해보셔도 좋겠습니다 ㅎㅎ
(공통 컴포넌트로 변화시키는 과정에서 얻는 것들이 있을거에요~!)

Comment on lines +96 to +98
$addRestaurantButton.addEventListener("click", function () {
$modal.classList.remove("hidden");
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 컴포넌트의 역할과 그에 따른 코드 흐름에 관련된 코멘트이기에 step2 완료 후 최우선으로 반영해봅시다.

Comment on lines +105 to +107
document.addEventListener("keydown", function () {
if (event.key === "Escape") $modal.classList.add("hidden");
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

왜 그냥 써도 됐는지 학습까지 해보시기 바랍니당! (다음에 또 같은 실수를 할지도 몰라요 ㅋㅋ)

Comment on lines +111 to +112
form.addEventListener("submit", (event) => {
event.preventDefault();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분도 step2 요구사항을 만족한 후 조금 더 고민해봅시다~!
(우선순위는 낮습니다)

Comment on lines +114 to +118
const $categoryInput = this.element.querySelector("#category");
const $name = this.element.querySelector("#name");
const $distance = this.element.querySelector("#distance");
const $description = this.element.querySelector("#description");
const $link = this.element.querySelector("#link");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분도 step2 요구사항을 만족한 후 최우선으로 학습해봅시다! (간단한데, 효과는 커서 추천드려요)
114~118라인은 전부 사라질겁니다!

Comment on lines +128 to +129

this.props.addRestaurant(modalInput);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 step2 요구사항을 만족한 후 조금 더 고민해봅시다~!

  • 필수 입력검증에서 그치지 않고 추가적으로 검증해야 할 것이 있다면 해봅시다.
    • 링크 입력에 숫자를 입력하면 어떻게 될까요?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 step2 요구사항을 만족한 후 학습해봅시다.
(우선순위는 낮습니다. - 어떤 장단이 있는지 정도만 파악하면 될것 같아요.)

Comment on lines +4 to +5
const $app = document.querySelector("#app");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 step2 요구사항을 만족한 후 조금 더 고민해봅시다~!
(TS와도 연관있는 부분이라 수정될수도 있겠네요)

  • querySelector로 찾아온 dom은 항상 존재한다고 가정할 수 있을지?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants