이번에는 리뷰어님의 피드백만 가져와 봤어요~
누구보다 빠른 월터님의 피드백.. 너무 너무 감사했습니다 월터님🙇♂️🙇♂️🙇♂️
여태까지 단 한번도 상수를 여러 파일로 분리해본 적이 없어요. 그런데 처음으로 constants.ts
파일을 여러 개로 분리했어요.
constants.ts에 너무 많은 상수들이 있었거든요. (에러메세지, Action, Auth, ...)
상수도 복잡해지면 분리해줘야합니다!
보통 validate--
함수를 보면 어떤 역할과 반환 값을 기대하시나요?
맞습니다. 들어온 인자값들을 검증하고 유효하다면 true 아니면 false를 반환할 것 같죠!
근데 저는 반환값을 반대로 작성하고 있었어요.
// 에러 발생 시
new ValidationResult(true, '에러메세지');
// 에러가 발생하지 않을 시
new ValidationResult(false);
👆 ValidationResult의 첫번째 인자가 hasError
이다보니 true일 경우 에러가 발생하도록 설계되었어요.
하지만, 일반적으로 validate--
에서는 true면 값이 유효하다고 생각하겠죠.
월터님의 피드백을 토대로 hasError
를 pass
로 바꾸고 boolean값을 반전시켜줬습니다.
private page는 보통 router에서 제어해줍니다.
👇 컴포넌트를 마운팅하고 변수에 따라 template을 수정하지 말고
// in login-component.ts
if (!isLogin) {
return `<h2>🚫이 페이지는 관리자만 접근할 수 있습니다🚫 </h2>`;
}
👇 애초에 권한이 없음을 라우터에서 판단
하고 private page를 마운트해주는 방식으로 수정해줍시다!
// in router.ts
const accessiblePage = isLogin
? PAGE.PRODUCT_MANAGE_PAGE
:PAGE.ONLY_MANAGER_ACCESSIBLE_PAGE;
const [$email, $password] = this.querySelectorAll('input');
👆 삐용삐용~ 위험하죠. 어떤 이유에서 template이 변경되면 (예를 들어 input이 맨 앞에 추가된다면) 원하는 동작을 할 수 있을까요?
const $email = this.querySelector('#login-email-input') as HTMLInputElement;
const $password = this.querySelector('#login-password-input') as HTMLInputElement;
👆 id를 하나씩 부여해서 가져옵시다. 훨씬 안정적입니다. (DOM 구조 의존 회피)
😢 기존 코드에서는 API요청을 통해 유저정보를 가져오는 getUserInfo()
함수를 굉장히 많이 호출하고 있었어요.
월터님께서 피드백을 주셔서 getUserInfo를 딱 한번만 호출하도록 바꾸었는데요! (딱 한번 호출하면서 state에 user-info를 저장했어요.)
😀 바꾸고나니 웹 페이지 내 nav tab 이동하는 시간도 단축되고 엄청난 효과를 보았습니다..!
사실 지난 번 피드백에서도 localStorage.getItem()
처럼 비용이 비싼 작업을 최소화하라는 피드백을 받았었는데요, 이번에도 고치지 못하고 비슷한 피드백을 들었네요ㅜㅜ
명심합시다. API 요청, localStorage 접근 같이 비싼 작업들은 최소화합시다.
월터님 피드백 : 비동기 요청 시에는 반드시 에러 핸들링을 해줍시다!
전반적으로 비동기 요청에 대한 에러 핸들링이 많이 부족한 거 같아요.
동작하지 않는 부분도 있고, 에러가 발생해서 진행이 불가능한 케이스도 있는 것 같습니다!
API를 fetch해오는 과정에서는 정말 다양한 http response error들이 존재하죠.
그런데 try catch 문도 사용하지 않고 fetch하고 있었던 저.. 반성합니다🙇♂️🙇♂️
외부 API를 직접 요청하면 테스트는 독립적이지 못하게 된 거예요.
테스트를 독립적으로 만들기 위해 API를 처리합시다.
Cypress에서 API intercept하기
cy.intercept('POST', `${API}/login`, { fixture: 'response-ok' }).as('login');
someLogic() // 로그인을 위한 input 작성 및 로그인 form 제출 로직
cy.wait('@login');
이렇게 API를 intercept해버리면 그 API를 cy.wait()
처리를 해줄 수 있어요. cy.wait를 썼는데 정말 적절하게 썼죠?
😀 무지성 cy.wait(1000)
을 없앨 수 있어서 굉장히 유용했어요.
Cypress는 계속 로그인이 풀려서 beforeEach롤 매번 로그인을 해주고 테스트를 진행해야하는데 왜 풀리는 걸까요?
cypress 에서는 여러 테스트들이 동시에 진행될 때 사이드이펙트를 막기 위해서 cookie, localStorage를 매 테스트마다 초기화한다고 합니다.
👉 앗 저의 로그인 구현은 localStorage를 사용하고 있었고, 그래서 localStorage가 초기화되면서 기존 로그인 정보가 지워졌던 거죠!
해결방법을 찾아봤는데요.
npm i --save-dev cypress-localstorage-commands
위처럼 패키지를 사용하거나 commands.js에서 직접 localStorage를 보존하는 함수를 만들어 주면 될 것 같아요.
🤔 하지만 꼭 필요한 경우가 아니라면 기존 cypress의 의도대로 localStorage를 매번 초기화하는 것이 더 안전하다는 생각이 드네요.