-
Notifications
You must be signed in to change notification settings - Fork 77
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
[7팀 김원표] [Chapter 1-1] 프레임워크 없이 SPA 만들기 #30
base: main
Are you sure you want to change the base?
Conversation
해결은 했지만, 왜때문에 해결했는지 아직 모름..
ProfilePage.js 의 원인은 아직 찾지 못함.. - 계층 문제 - 이메일 값 세팅 문제
왜때문인지 도저히 모르겠는데..
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.
작성하신 코드 너무너무 잘봤습니다~
코드 리뷰를 한 번 작성해보았는데, 어떠실런지 궁금하군요
ps. core-js는 어떤 부분에서 사용하신 걸까요? 제가 한 번도 사용해본 적이 없어서 궁금하네요
|
||
const logoutLink = document.getElementById("logout"); | ||
if (logoutLink) { | ||
logoutLink.addEventListener("click", (e) => { |
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.
저희가 심화 학습 부분에서 이벤트위임에 관한 내용을 읽어보시면 조금 더 깔끔하게 코드를 구현할 수 있을 것이라 생각이듭니다.
e.preventDefault(); | ||
removeUser(); | ||
|
||
window.location.href = "/login"; |
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.
이 부분도 아래의 Profile.js-line 77 과 같이 수정하셔도 무방할 것 같아요!
src/store/index.js
Outdated
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.
이부분도 깔끔하게 작성하셔서 좋았습니다.
만약 리팩토링 원하시면 클래스로 한번 바꿔보시는 건 어떠실지 슬쩍 추천해봅니다 ㅎㅎ
</ul> | ||
</nav>`; | ||
|
||
const logoutLink = document.getElementById("logout"); |
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.
logoutLink가 동일한 것이 여러 번 선언된걸로 보여요! 하나로 줄이시는 리팩토링 연습을 해보시면 어떨까 추천드립니다!
과제 체크포인트
기본과제
1) 라우팅 구현:
2) 사용자 관리 기능:
3) 프로필 페이지 구현:
4) 컴포넌트 기반 구조 설계:
5) 상태 관리 초기 구현:
6) 이벤트 처리 및 DOM 조작:
7) 라우팅 예외 처리:
심화과제
1) 해시 라우터 구현
2) 라우트 가드 구현
3) 이벤트 위임
과제 셀프회고
정확한 개념을 모르고 사용하던 부분들이 많아 과제를 진행하며 테스트 코드가 통과될 때 개념을 이해했고 활용했구나 생각하며 개념을 정립할 수 있었습니다.
처음에는 개발서버를 기준으로 과제를 진행하면서 잘 동작한다고 생각했지만, 테스트서버에서는 모든 테스트가 실패하는 상황을 보았습니다. 이후에는 테스트 페이지만을 보며 작업을 진행했지만, 그 덕분에 동작을 정확히 확인할 수 있었던 것은 아니었습니다. 실제로는 해시라우터는 정상적으로 동작했지만 반환 시 히스토리라우터로 돌아가는 문제가 있었음을 알게 되었습니다. 이를 통해 테스트 케이스의 중요성과, 단순히 테스트를 통과하는 것 이상의 의미가 있음을 깨닫게 되었습니다.
기술적 성장
해시라우터의 동작 원리를 학습하며, 처음 스케치 단계에서는 route.js에서 router와 hashRouter를 따로 구현하고 main.hash.js에서 해시와 히스토리를 구분하려 했습니다. 하지만, 비슷한 코드를 두번 반복하느니 main.js에서 두 라우팅 방식을 모두 지원하도록 구현했습니다. 이 과정에서 type으로 라우팅을 관리한 점은 깔끔하지는 않지만 개인적으로 만족스러운 도전이었습니다.
또한, 개념을 알지 못한 채 구현했던 상태 관리가 테스트를 통과하면서 역으로 "이것이 상태 관리 시스템이었구나"라고 깨닫게 되었고, 이를 통해 상태 관리의 개념을 정립할 수 있었습니다.
코드 품질
특히, main.js에서 해당 로직을 처리한 이유는 다른 컴포넌트들이 해시라우터와 히스토리라우터에 상관없이 재사용될 수 있도록 설계하기 위함입니다. 이를 위해 type으로 경로의 유형을 관리하면서도, 컴포넌트에 넘겨주는 값은 기본 경로로 통일해 처리할 수 있게끔 구현하였습니다.
이 접근은 다른 컴포넌트들의 재사용성과 유연성을 높이는 데 효과적이었지만, 경로 관리 로직을 더욱 일반화하거나 재사용성을 극대화하는 방식으로 리팩토링이 필요할 것 같습니다.
학습 효과 분석
이벤트 위임 방식을 더 효과적으로 활용하기 위해서는 DOM이 변경되는 과정과 브라우저에서 렌더링이 이루어지는 순서에 대한 추가 학습이 필요하다고 생각합니다. 특히, 동적 요소가 추가되거나 제거될 때 DOM 업데이트가 어떤 방식으로 진행되는지 이벤트 버블링이 이 과정에서 어떻게 동작하는지를 추가적으로 학습할 계획입니다.
과제 피드백
E2E 테스트를 처음 진행하면서 HTML의 계층 구조, placeholder와 같은 사소한 요소까지 테스트 통과 여부에 영향을 미친다는 점을 알게 되었습니다. 특히,
npx playwright test --update-snapshots
명령어를 사용하여 현재 계층 구조를 기준으로 테스트 코드를 업데이트하면 테스트를 통과할 수 있음을 확인할 수 있었습니다.과제를 진행하며 로직상의 문제는 없었지만, 구조상의 차이로 인해 발생하는 문제가 과연 중요한가에 대해 고민했습니다. 동시에 프론트엔드 개발자로서 UI 요구사항을 명확히 하고 이를 기반으로 철저히 테스트를 진행하는 것이 올바른 접근인지에 대한 의문도 갖게 되었습니다.
따라, 과제를 구현하는 점에 있어서 개념을 확립해나갈 수 있어서 좋았고, 테스트 코드에 대해서 생각해볼 수 있어서 좋았습니다.
리뷰 받고 싶은 내용
저는 현재 패턴을 사용하지 않고(?) 고려하지 않고(?) 코드를 작성했는데, 나중에 코드 유지보수나 확장성을 고려할 때 설계 패턴을 적용하는 것이 더 나을까요? 그렇다면 어떤 패턴을 적용하는 것이 좋을까요?
main.js에서 hash와 history를 type으로 구분하여 분기 처리를 했는데 이 부분을 전역 상태 관리 도구를 사용해 구현하면 더 나은 방법이 될 수 있을까요? 전역 상태 관리로 구현할 경우 어떤 장점이나 단점이 있을지 그리고 적용 시 고려해야 할 점이 있다면 알려주세요..