[17팀 정소윤][Chapter 1-1] 프레임워크 없이 SPA 만들기 #9
Conversation
- error.html -> Error.js - login.html -> Login.js - main.html -> Home.js - profile.html -> Profile.js
- navigate 파라미터 event 대신 target path로 전달받을 수 있도록 수정 - a 태그 click event 등록 시 event.preventDefault 호출
- '/', '/profile' 접근 시 user 없으면 '/login'으로 이동
- logout 버튼 a 태그 => button 태그로 수정
- 페이지명 + Page.js
- useNavigate, useAuth, useProfile
- 기존 render 내부 => 변경 addEvent 내부
- 페이지 컴포넌트 렌더링 root -> main 기준으로 수정 - 페이지별 header/footer 조건부 렌더링 - 기존 HomePage, ProfilePage 내 Header/Footer 태그 삭제
- pages/Login.js, pages/Profile.js
- 기존: 로그인 시에만 접근 가능 - 변경: 비로그인도 '/' 접근 가능. 네비게이션 홈|로그인 보이도록 분기처리 추가
- 기존 templates 하단 파일명 소문자로 수정 - 기존 pages 하단 파일명 -Page로 수정
- 헤더 토글 user 기준 -> isLoggedIn 기준
- 기존: 하드코딩 - 변경: menus로 리스트 생성 후 map으로 관리
- 기존: input required에 의존
soyoonJ
changed the title
|
전반적으로 코드가 명확하고 구조적으로 잘 짜여있는 것 같습니다. |
| const Header = (currentPath) => { | ||
| const isLoggedIn = store.getState('isLoggedIn'); | ||
|
|
||
| const menus = [ |
There was a problem hiding this comment.
active 속성을 추가해서 렌더링 여부 결정하도록 로직 작성하신거 너무 좋은 것 같아요
전 로그인 여부에 따라 배열을 필터링하는 방법만 떠올랐는데..
덕분에 새로운 아이디어 얻어갑니다 😁😁
| </footer>`; | ||
| }; | ||
|
|
||
| export default Footer(); |
There was a problem hiding this comment.
소윤님 Footer처럼 함수를 실행해서 export하는 케이스와 Header처럼 실행하지 않고 export하는 두 가지 케이스가 있더라구요!
혹시 내부에서 상태 관리를 하지 않는 컴포넌트는 바로 실행해서 export 하도록 의도하신 걸까요?!
제 생각에는 나중에 Footer 함수에 상태 관리 로직이 추가될 경우 내부에 render 함수를 구현해야 될 것 같은데,
"컴포넌트의 구조가 통일되는 것이 나중을 위해 더 좋지 않을까..?"하고 생각하는데 소윤님 생각은 어떤지 궁금합니다!
함수형 컴포넌트라 이 부분이 더 어려웠을 것 같네요 😥
There was a problem hiding this comment.
승우님! 리뷰 감사합니다 :)
말씀주신 것처럼 내부에서 상태 관리를 하지 않는 컴포넌트는 바로 실행해서 export 하려는 의도가 맞습니다!
하지만 승우님 말씀 들어보니 컴포넌트는 현재 렌더링을 하든 하지 않든 통일성을 맞추는 게 좋을 것 같네요!
좋은 의견 감사합니다 👍👍
| </div> | ||
| </div>`; | ||
|
|
||
| document.getElementById('logout')?.addEventListener('click', () => { |
There was a problem hiding this comment.
오... renderPage 하는 부분에 이벤트를 등록하는 방법을 생각을 못했었는데, 페이지별 효율적인 이벤트 관리 좋은것 같습니다
| const rootElement = document.getElementById('root'); | ||
|
|
||
| if (currentPath === '/' || currentPath === '/profile') { | ||
| rootElement.innerHTML = /* HTML */ `<div class="bg-gray-100 min-h-screen flex justify-center"> |
There was a problem hiding this comment.
페이지 단위의 템플릿 리터럴처럼 js로 관리하는 것도 좋아보입니다.
| rootElement.innerHTML = '<main id="main"></main>'; | ||
| } | ||
|
|
||
| document.querySelectorAll('li').forEach((anchor) => { |
There was a problem hiding this comment.
저도 비슷하게 a태그에다가 모든 이벤트를 걸었는데,
추후 사용성 까지 생각하면 class나, id로 이벤트 거는 것도 고려해보면 좋을 것 같습니다!
There was a problem hiding this comment.
useNavigate.js 구조
이번 과제를 하며 실제 라이브러리/프레임워크를 사용하는 것과 최대한 비슷하게 짜보자는 목표를 두고 진행했는데요, useNavigate.js가 유독 계속 아쉬움이 남는데 어떤 부분이 정확히 아쉽고 개선이 필요한 지에 대해 확인하기가 어려웠습니다. 어느 부분에서 함수 분리를 하면 좋겠다거나 변수를 다르게 했으면 좋겠다 등의 내용도 좋고 라우트 가드 구현을 어떤 방식으로 구현해보면 좋겠다 등의 리뷰를 받고 싶습니다.
useNavigate가 어플리케이션의 전반적인 로직을 담당하고 있고
이게 utils와 component를 가져와서 사용하고 있어요.
즉, "재사용"과는 거리가 굉장히 먼 친구인거죠.
그래서 각각의 페이지에서 재활용해야 하는 기능만 따로 떼어내서 불리해서 관리해주면 어떨까 싶어요 ㅎㅎ
이외에도 Page가 routes를 의존하고, routes가 Page를 가져다 사용하고 있는데
이렇게 상호 의존하는 관계는 나중에 문제가 될 수 있답니다!
routes에 수정이 발생되면 Page에도 영향이 가고
Page에 수정이 발생되면 routes에도 영향이 가는 그런 구조랄까...
routes를 다른 프로젝트에서 재활용한다고 생각하고 작성해보세요!
함수형 < class ?
바닐라 자바스크립트로 spa를 만드는 방법에 대해 찾다보면 대부분의 글들이 class로 구조를 만드는 것을 볼 수 있었습니다. 저도 동일하게 class로 구현하는 것을 고민하다가 함수형으로도 구현해보고자 함수형을 선택했는데요, 현업에서 class를 사용하는 것이 더 일반적인지 궁금합니다. 만약 상황에 따라 다르다면 어떤 상황에서 class를 보통 선호하는 지가 궁금합니다.
class는 구조를 표현할 때 많이 사용되곤 하는데요, 컴포넌트의 경우 라이프사이클을 표현해야 해서 class를 사용하는 경우가 많답니다 ㅎㅎ
하지만 이게 정답은 아니에요!
제가 추천드리는 방법은 class로도 구현해보고 function으로도 구현해보고 두 개의 장점을 적절히 섞어서 사용하도록 숙련하면 좋을 것 같아요!
"이게 정답이야!" 라고 할만한 내용은 없어요 ㅎㅎ 프레임워크마다 다르고 어떤 철학을 추구하냐 따라 무척 다르니까요!
| import Header from '../components/Header'; | ||
| import Footer from '../components/Footer'; | ||
|
|
||
| export const useNavigate = (routes, protectedRoutes) => { |
There was a problem hiding this comment.
utils라는 폴더에 이걸 구현해주셨는데요,
store도 의존하고 있고
component도 의존하고 있어요!
그러다보니 재활용을 할 수 있는 형태의 모습이라기보단
어플리케이션의 전체 로직이 여기에 총집합 되어있는 모습이 되었답니다 ㅎㅎ
재활용할 수 있으려면 이런 의존관계를 명확하게 관리해줘야 한답니다!
| <footer class="bg-gray-200 p-4 text-center"> | ||
| <p>© 2024 항해플러스. All rights reserved.</p> | ||
| </footer> | ||
| window.addEventListener('popstate', renderPage); |
There was a problem hiding this comment.
navigtation이 popstate에 대한 책임도 가져갔으면 어땠을까 싶어요!
| const HomePage = () => { | ||
| const render = () => { | ||
| const rootElement = document.getElementById('main'); | ||
| rootElement.innerHTML = HomeTemplate; | ||
| document.title = '항해플러스 - 홈'; | ||
| }; | ||
|
|
||
| render(); | ||
| }; |
There was a problem hiding this comment.
const HomePage = () => {
const rootElement = document.getElementById('main');
rootElement.innerHTML = HomeTemplate;
document.title = '항해플러스 - 홈';
};이렇게 표현해도 무방했을 것 같아요!
There was a problem hiding this comment.
const HomePage = () => { const rootElement = document.getElementById('main'); rootElement.innerHTML = HomeTemplate; document.title = '항해플러스 - 홈'; };이렇게 표현해도 무방했을 것 같아요!
추가로 궁금한 점이 있습니다!
사실 render밖에 없기 때문에 굳이 render를 적지 않아도 무방했지만 render를 적었던 이유는 다른 컴포넌트들에서도 동일한 구조를 가지고 있기 때문이었는데요, 다른 컴포넌트 구조를 잡아둔 것이 있어도 우선 해당 컴포넌트에서 깔끔하게 처리하는 것을 우선적으로 두는 것이 좋을까요?
그리고 혹시 다른 이유에서 render 밖으로 빼는 것을 말씀주신 거라면 어떤 이유들에서 빼는 게 좋은 지도 궁금합니다 :) (가령 타이틀 설정하는 것이 render라는 이름과는 어울리지 않는다거나...!)
There was a problem hiding this comment.
구조를 맞추고 싶다면 차라리 클래스를 통해서 표현하는게 좋아보여요..!
| import LoginTemplate from '../../templates/login'; | ||
|
|
||
| const LoginPage = () => { | ||
| const { navigate } = useNavigate(routes, protectedRoutes); |
There was a problem hiding this comment.
routes가 LoginPage를 가져다 사용하고 있는데,
다시 LoginPage가 routes를 가져와서 사용하고 있네요 ㅎㅎ
이렇게 상호 의존적인 모습은 좋은 설계가 아니랍니다..!
| const addEvent = () => { | ||
| document.getElementById('login-form').addEventListener('submit', (event) => { | ||
| login(event); | ||
| }); | ||
| }; | ||
|
|
||
| const login = (event) => { | ||
| event.preventDefault(); | ||
|
|
||
| const username = document.getElementById('username').value; | ||
|
|
||
| if (username.trim() === '') { | ||
| throw new Error('사용자 이름을 입력해주세요.'); | ||
| } | ||
|
|
||
| store.setState('user', { username, email: '', bio: '' }); | ||
| store.setState('isLoggedIn', true); | ||
|
|
||
| navigate('/profile'); | ||
| }; |
There was a problem hiding this comment.
const addEvent = () => {
document.getElementById('login-form').addEventListener('submit', handleSubmit);
};
const handleSubimt = (event) => {
event.preventDefault();
login(document.getElementById('username').value.trim());
}
const login = (username) => {
if (username === '') {
throw new Error('사용자 이름을 입력해주세요.');
}
store.setState('user', { username, email: '', bio: '' });
store.setState('isLoggedIn', true);
navigate('/profile');
};이런식으로
- 이벤트를 등록해주는 함수
- 이벤트를 처리해주는 함수
- 이벤트와 무관하게, 실제로 필요한 기능을 수행하는 함수
이렇게 구분해서 관리해주면 코드를 파악하기가 더 수월하답니다!
| const setState = (targetState, newState) => { | ||
| state = { | ||
| ...state, | ||
| [targetState]: newState, | ||
| }; | ||
| listeners.forEach((listener) => listener(targetState)); | ||
| localStorage.setItem(targetState, JSON.stringify(newState)); | ||
| }; |
There was a problem hiding this comment.
targetState를 지정하기보단 객체로 받아서 처리하도록 해주면 어떨까요?
setState를 할 때 마다 listener를 다 가져와서 실행하게 되는데..
한 번에 여러 개의 상태를 변경해야하는 경우에 문제가 될 수 있답니다!
| ${menus | ||
| .map((menu) => { | ||
| if (!menu.active) return ''; | ||
| return /* HTML */ `<li> | ||
| <a href=${menu.path} class="${currentPath === menu.path ? 'text-blue-600 font-bold' : 'text-gray-600'}" | ||
| >${menu.name}</a | ||
| > | ||
| </li>`; | ||
| }) | ||
| .join('')} |
There was a problem hiding this comment.
menus.filter(v => v.active).map()이렇게 미리 filter로 걸러주면 어떨까요!?
There was a problem hiding this comment.
menus.filter(v => v.active).map()이렇게 미리 filter로 걸러주면 어떨까요!?
안 그래도 이 부분이 고민이 되던 부분인데요! menues.filter().map()을 할 경우 loop가 두 번 돌아야 한다는 점이 걸려, map으로 한 바퀴만 돌면서 처리하고자 했습니다. 사실 메뉴 수가 많지 않아 크게 영향을 받을 부분은 아니지만 loop를 최소화 해야겠다는 생각이 마음 한 자리에 자리잡고 있어서요!🤣
이런 이유에도 처음에 filter랑 같이 쓰는 것을 고민했던 이유는 가독성 때문이었는데요, 코치님께서 filter로 처리하는 것을 제안주신 이유는 무엇일지 궁금합니다 :)
There was a problem hiding this comment.
ㅎㅎ 이정도의 차이는 성능에 대한 이점을 가져간다거나 그런게 딱히 없어서요!
차라리 가독성을 챙기는게 좋다고 생각합니다.
무엇보다 어차피 연산하는 횟수는 비슷할꺼에요!
There was a problem hiding this comment.
상황에 따라 판단하는 게 좋겠군요! 자세한 공유 감사합니다 :)
와! 꼼꼼한 리뷰 감사합니다!! |
과제 체크포인트
기본과제
1) 라우팅 구현:
2) 사용자 관리 기능:
3) 프로필 페이지 구현:
4) 컴포넌트 기반 구조 설계:
5) 상태 관리 초기 구현:
6) 이벤트 처리 및 DOM 조작:
7) 기본적인 에러 처리:
심화과제
1) 고급 라우팅
2) 고급 이벤트 처리
리뷰 받고 싶은 내용
이번 과제를 하며 실제 라이브러리/프레임워크를 사용하는 것과 최대한 비슷하게 짜보자는 목표를 두고 진행했는데요, useNavigate.js가 유독 계속 아쉬움이 남는데 어떤 부분이 정확히 아쉽고 개선이 필요한 지에 대해 확인하기가 어려웠습니다.
어느 부분에서 함수 분리를 하면 좋겠다거나 변수를 다르게 했으면 좋겠다 등의 내용도 좋고 라우트 가드 구현을 어떤 방식으로 구현해보면 좋겠다 등의 리뷰를 받고 싶습니다.
궁금한 점
바닐라 자바스크립트로 spa를 만드는 방법에 대해 찾다보면 대부분의 글들이 class로 구조를 만드는 것을 볼 수 있었습니다. 저도 동일하게 class로 구현하는 것을 고민하다가 함수형으로도 구현해보고자 함수형을 선택했는데요, 현업에서 class를 사용하는 것이 더 일반적인지 궁금합니다. 만약 상황에 따라 다르다면 어떤 상황에서 class를 보통 선호하는 지가 궁금합니다.
과제를 수행하면서 느낀점
과제 시작 전 생각
과제 제출 후 생각
기타
과제 난이도
4