[레벨1 - 미션4] 2단계 자판기 미션 피드백

레벨1 미션4 자판기 미션 2단계 PR

이번에는 리뷰어님의 피드백만 가져와 봤어요~

누구보다 빠른 월터님의 피드백.. 너무 너무 감사했습니다 월터님🙇‍♂️🙇‍♂️🙇‍♂️

💪 JS

상수도 복잡하면 나눠라

여태까지 단 한번도 상수를 여러 파일로 분리해본 적이 없어요. 그런데 처음으로 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 routing

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;

DOM은 id로 직접 찝어주자

 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 요청 줄이기

😢 기존 코드에서는 API요청을 통해 유저정보를 가져오는 getUserInfo() 함수를 굉장히 많이 호출하고 있었어요.

월터님께서 피드백을 주셔서 getUserInfo를 딱 한번만 호출하도록 바꾸었는데요! (딱 한번 호출하면서 state에 user-info를 저장했어요.)

😀 바꾸고나니 웹 페이지 내 nav tab 이동하는 시간도 단축되고 엄청난 효과를 보았습니다..!

사실 지난 번 피드백에서도 localStorage.getItem()처럼 비용이 비싼 작업을 최소화하라는 피드백을 받았었는데요, 이번에도 고치지 못하고 비슷한 피드백을 들었네요ㅜㅜ

명심합시다. API 요청, localStorage 접근 같이 비싼 작업들은 최소화합시다.

API 에러 핸들링

월터님 피드백 : 비동기 요청 시에는 반드시 에러 핸들링을 해줍시다!

1. 전반적으로 비동기 요청에 대한 에러 핸들링이 많이 부족한 거 같아요.

2. 동작하지 않는 부분도 있고, 에러가 발생해서 진행이 불가능한 케이스도 있는 것 같습니다!

• API를 fetch해오는 과정에서는 정말 다양한 http response error들이 존재하죠.

• 그런데 try catch 문도 사용하지 않고 fetch하고 있었던 저.. 반성합니다🙇‍♂️🙇‍♂️

💪 Cypress

API test

외부 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에서 로그인이 풀리는 에러 (feat. LocalStorage)

Question

Cypress는 계속 로그인이 풀려서 beforeEach롤 매번 로그인을 해주고 테스트를 진행해야하는데 왜 풀리는 걸까요?

Answer

cypress 에서는 여러 테스트들이 동시에 진행될 때 사이드이펙트를 막기 위해서 cookie, localStorage를 매 테스트마다 초기화한다고 합니다.

👉 앗 저의 로그인 구현은 localStorage를 사용하고 있었고, 그래서 localStorage가 초기화되면서 기존 로그인 정보가 지워졌던 거죠!

해결방법을 찾아봤는데요.

npm i --save-dev cypress-localstorage-commands

위처럼 패키지를 사용하거나 commands.js에서 직접 localStorage를 보존하는 함수를 만들어 주면 될 것 같아요.

🤔 하지만 꼭 필요한 경우가 아니라면 기존 cypress의 의도대로 localStorage를 매번 초기화하는 것이 더 안전하다는 생각이 드네요.