[김선혜] week6

[HaydenDevK]

요구사항

기본

로그인 페이지

• https://bootcamp-api.codeit.kr/docs 에 명세된 “/api/sign-in”으로 { “email”: “[email protected]”, “password”: “sprint101” } POST 요청해서 성공 응답을 받고 “/folder”로 이동하나요?

회원가입 페이지

• 이메일 input에서 “[email protected]”으로 “/api/check-email” 이메일 중복 확인 POST 요청하면 이메일 중복 에러를 확인할 수 있나요?
• 유효한 회원가입 형식의 경우 “/api/sign-up” POST 요청하고 성공 응답을 받으면 “/folder”로 이동하나요?
  심화

심화

• 로그인/회원가입 페이지에 접근시 로컬 스토리지에 accessToken이 있는 경우 “/folder” 페이지로 이동하나요?

주요 변경사항

리팩토링

• 콘솔 제거
• 상수만 따로 모듈 분리
• goToFolderPage
• named parameter
• 더 작은 기능으로 함수 분리

멘토에게

• 노션 링크

[sujin-park]

6주차 미션도 고생 많으셨어요 선혜님! 👍
좀 더 큰 맥락이 수정되면 추가로 코멘트 드릴 수 있는 부분이 있을 것 같아요.
일단 비동기 에러 핸들링에 대해 개선해보시면 좋을 것 같아요!
6주간 너무 고생많으셨습니다!

1. 코멘트로 드린 부분이면 될 것 같아요.
2. 저는 지금처럼 argument 로 전달하는 것도 좋은 것 같아요!
3. 음.. 일단 제가 드린 코멘트 한번 더 개선하고 다시 확인해봐도 괜찮을까요! 👀

[sujin-park]

저라면 fetch 함수 호출부가 중복이 되어 아래와 같이 함수를 분리해서 재사용해볼 것 같아요!
그러면 굳이 노션에서 질문 주신 1번에 대한 질문이 필요 없을 것 같아요.
이미 fetchClient 라는 함수에서 response.status === 200 에 대한 핸들링은 되었기 때문이죠!

const fetchClient = async (url, method, body) => {
  try {
    const response = await fetch(`${BASE_URL}/${url}`, {
      method,
      headers: {
        'Content-Type': 'application/json',
      },
      body: JSON.stringify(body),
    });

    if (response.status !== 200) {
      // HTTP 오류 처리
      throw new Error(response.status);
    }

    const data = await response.json();
    return data;
  } catch (error) {
    throw error; // fetchClient 를 호출하는 함수에서 에러 처리하기 위해
  }
};

[sujin-park]

추가로 위 fetchClient 는 apiClient 라는 모듈로 분리해서 사용할 것 같아요.
signUp, signIn, getIsNewEmail 은 동일하게 auth/authroize 라는 모듈에 있어도 괜찮을 것 같아서요!
fetchClient 와 같이 API Request 관련 로직이 분리될 수 있다면 저는 위 3개의 함수는 분리되어도 괜찮을 것 같아요!

[HaydenDevK]

1. response.ok로 처리하면 200대 코드는 모두 성공으로 판단하고, response.status !== 200은 200 코드만 성공으로 판단한다.
2. 200만 성공으로 판단하는 게 맞다.
3. 현업에서도 주로 이렇게 구현한다.
   이렇게 이해해도 괜찮을지 의견 부탁드립니다!🙂🙏

[HaydenDevK]

다른 질문이 있습니다!

1. getIsNewEmail는 통신이 실패할 경우에 실패했다는 것에 대한 리턴 값을 반환해야하는 함수인데용, catch문에서 throw error 외에 어떻게 처리를 해주는 게 좋을까요?
2. fetchClient에서 response.json()를 공통적으로 실행할 경우, getIsNewEmail에서는 필요없는 로직이라, fetchClient에서 -> signIn, signUp으로 빼려는데 괜찮을까용?

[sujin-park]

1. API 명세에 따라 달라질 것 같은데 회원가입/로그인 상관없이 200대 코드를 모두 성공으로 판단하고자한다면 response.ok 를 사용할 것 같아요! 만일 로그인과 같이 status 가 200만 있을 것으로 추정되는 경우는 200 으로도 체크를 합니다.
2. 데이터가 정상적으로 생성되었을 때는 201 status가 반환되는 경우도 있으니 200대를 성공으로 보는게 맞을 것 같아요!
3. 현업에서도 주로 response.ok / response.status 로 체크 많이 합니다ㅎㅎ

[sujin-park]

유저에게 보여줄 에러메세지나 에러 UI 가 없는 경우에는 개발팀에서 모니터링하기위해 모니터링 서비스 관련 로직을 추가하거나 로깅을 하기 위해 로깅 메서드를 호출하곤 합니다. 예를 들어 아래와 같이 될 수 있을 것 같아요!

try {
  fetchClient({
    url: "check-email",
    method: "POST",
    body: { email },
  });
} catch {
 // 에러 핸들링
 logger.debug(‘xxxxx’);
}

지금 선혜님이 리팩토링해주신 코드 다시 확인하였는데 fetchClient 함수를 사용하는 곳에서 에러핸들링이 안 되어있는 것 같아서 지금은 console.log 또는 alert 으로 에러 핸들링을 해두면 좋을 것 같아요!
그리고 signin, signup 은 에러가 발생하면 인풋 하단에 에러 메세지가 노출되어야하는게 맞을까요?!

[sujin-park]

그리고 fetchClient 에서 signin, signup 을 따로 뺀다는 의미가 어떤건지 조금 더 자세히 공유해주실 수 있을까요?
예시 코드같은게 있으면 좋을 것 같아요!

[HaydenDevK]

• 앗! 우선 어제 올린 코드는 잘못된 부분이 많아서 리셋하고 다시 작업중이라 아직 안 봐주셔도 될 것 같습니다!
• 그리고 2번 질문에 대해 설명드리자면, fetchClient 내부 로직에 response.json() 부분이 있는데요! 이게 getIsNewEmail에서는 필요없는 로직이라서용! fetchClient에서는 제거를 하고 signIn, signUp에서 실행하려는데 괜찮을지 궁금했습니다!
• 그리고 getIsNewEmail은 사용 가능한 새로운 이메일 주소가 인지/아닌지를 true/false로 리턴하려는 함수인데요! 여기서 사용하는 api에서 새로운 이메일 주소가 아닌 경우 409에러가 나도록 되어있어서요! 질문은 제가 이 에러를 어떻게 캐치해서 true/false 처리를 할 수 있을지였습니다! 제가 지금 생각하는 해결방법은 getIsNewEmail 함수에서 fetchClient를 호출하는 부분을 알려주신대로 try catch를 사용하는 아래 방식입니다!

export const fetchClient = async ({ url, method, body }) => {
	try {
		const response = await fetch(`${BASE_URL}/${url}`, {
			method,
			headers: {
				"Content-Type": "application/json",
			},
			body: JSON.stringify(body),
		});

		if (response.status !== 200) {
			throw new Error(response.status);
		} else {
			return response;
		}

		// const data = await response.json();
		// return data;
	} catch (error) {
		throw error;
	}
};

const getIsNewEmail = async (email) => {
try {
await fetchClient({
	url: "check-email",
	method: "POST",
	body: { email },
});
return true;
} catch {
return false;
}
	```

[sujin-park]

아 확인했습니다! 공유해주신 코드면 저는 좋은 것 같습니다~! 👍

[sujin-park]

정말! 한번 더 생각해보면 좋을 것 같아요. 저는 이런 주석도 좋아해요.
다만 현업에서는 이런 주석에 대해 의미가 없을 수 있어요. 이 주석을 나중에 어떻게 관리할 수 있을까요? 👀
고민해볼 필요가 있을 것 같아요!

[HaydenDevK]

auth.js 안에 크게 1) 비밀번호 토글 관련 함수 2) 로그인/회원가입 유효성 검사 관련 함수 이렇게 있어서, 구분지어 그룹화를 하고 싶었던 의도인데용!
혹, 다른 방법으로 모듈화를 하거나 그룹화를 해볼 수 있을지 의견 여쭙습니다~!☺️

[sujin-park]

만일 주석으로 그룹화를 해야하는 함수가 있다면 그게 바로 모듈화가 필요한 시점이 아닐까 생각됩니다!
비밀번호 토글 관련 함수 (passwordVisibility.js ? / 로그인/회원가입 유효성 검사 관련 함수 (validation/validator.js) 와 같이 모듈화를 해볼 수 있을 것 같아요.

때로는 선혜님이 하신 것처럼 주석으로 그룹화하는 경우도 있고 해당 주석이 좋은 정보를 제공해줄 수 있어요. 다만 주석을 최소한으로 사용해보자는 의미로 드리는 리뷰이니 참고 부탁드릴게요!

[sujin-park]

accessToken 은 상수로 관리하거나 getAccessToken/setAccessToken 과 같이 쌍으로 사용하는 accessToken 관련 함수가 있어도 좋을 것 같아요!

[HaydenDevK]

accessToken.js 모듈을 만들어 getAccessToken/setAccessToken 함수를 만드는 방법으로 개선하려고 합니다!
혹시 상수로 관리하는 방법은 어떤 방법인지 더 자세히 알려주실 수 있을까용?!🥺

[sujin-park]

보통 현업에서는 localStorage 에 저장하는 값들이 accessToken 외에 다른 값들도 있을 수 있기 때문에 localStorage key 를 상수로 관리하고 있어요.

예를 들어 아래와 같이 상수로 관리하는 방법을 사용해볼 것 같습니다! 다만 지금 말씀해주신 것처럼 set/get accessToken 함수를 만든다면 상수로 관리하지않아도 충분할 것 같아요 👍

const ACCESS_TOKEN_KEY = ‘accessToken’;

const STORAGE_KEYS = {
  ACCESS_TOKEN: ‘accessToken’
}