[1단계 - 지하철 노선도 미션] 하루(김하루) 미션 제출합니다. #12
Conversation
| }, | ||
| "prettier.useEditorConfig": true, | ||
| "liveServer.settings.port": 5500 | ||
| } |
There was a problem hiding this comment.
학습과정 초반에 '관리할 파일이 아니면 push하지 않는다'라는 피드백을 받은 적이 있습니다...! 👩🏻🏫 그래서 페어와 함께 이 파일을 push할지 말지 즉, 이 파일 공유하고 관리해야 하는 파일인지 고민했었습니다,,,👉👈
저희는 페어프로그래밍을 진행하면서 vscode/settings.json 파일은 공유되어야 하고, 변경사항이 생기면 함께 프로젝트에 반영되고 관리가 되어야 한다고 생각했습니다. 코드에 영향을 줄 수 있는 설정은 공유하는 것이 좋다고 생각했기 때문입니다. 예를 들어files.trimTrailingWhitespace의 경우 VS Code 로컬 설정이 false라면 .editorconfig에서 true로 해도 적용되지 않는데, vscode/settings.json에서 설정해주면 해결되는 경우도 있었습니다.
고런 연유로 push를 하게 되었는데,,, 혹시 에디터의 setting 파일은 공유하지 않는 것이 일반적인지 궁금합니다...!
There was a problem hiding this comment.
에디터의 설정내용은 개인 파일이라, 코드 컨벤션같은 경우는 eslintrc 나 prettierrc 로도 충분히 맞출 수 있지 않나 싶긴해요!
그치만 페어와의 규칙이라면 이해했습니다!
| return; | ||
| } | ||
|
|
||
| event.preventDefault(); |
There was a problem hiding this comment.
// 변경 전
export function handleLinkClick(event) {
const { target: $target } = event;
const $anchor = $target.closest('a');
if (!$anchor) {
return;
}
const { origin, pathname } = $anchor;
if (!isSameOrigin(origin)) {
return;
}
event.preventDefault();
...
}// 변경 후
export function handleLinkClick(event) {
const $anchor = event.target.closest('a');
if ($anchor === null || isDifferentOrigin($anchor.origin)) {
return;
}
event.preventDefault();
...
}핸들러를 handleClickLink 작성할 때,www.naver.com과 같이 외부 페이지로 접근하는 경우를 함께 고려했습니다. (현재 기능상 외부페이지로 접근하는 경우는 없지만, 이벤트 핸들러를 최상위 태그인 $app에 달고 있다보니, 이런 경우까지 고려하게 되었습니다. 👉👈 ) 이렇게 외부 페이지로 접근하는 경우에는, <a> 태그의 기본동작을 막지 않아야하고, 외부 페이지가 아니라 내부 경로가 맞는 것으로 판단되면 그때는 기본동작을 막아주어야 했습니다. 따라서 함수의 맨 첫부분이 아닌 중간 부분에서 preventDefault()를 호출을 했었습니다.
그런데 리뷰어님의 피드백을 듣고나니 기본동작을 막는 preventDefault()가 핸들러 후반부에 등장하는 것이 어색하게 느껴져서 앞쪽을 최대한 간결화 하는 방향으로 수정해보았습니다...! 괜찮은지 검토해주시면 감사하겠습니다 🙏
또, 수정을 하다가 궁금해진 사항이 있어 추가 질문드립니다...!🙋🏻♀️ target의 closest로 가져온 $anchor가 <a>태그가 없을 경우(null일 경우)에는 기능상 preventDefault()를 해주지 않아도 문제가 없어 보입니다. 그렇다면 <a>인지 검사한 후에 preventDefault()를 실행해도 되는 것인지, 아니면 일종의 컨벤션(?)으로 preventDefault를 항상 핸들러 맨 첫줄에 작성하는 것이 좋을지(?) 궁금합니다...!
There was a problem hiding this comment.
와아 이렇게 정성스럽게 설명해주시다니ㅋㅋㅋㅋㅋㅋㅋㅋ 😂
저는 그런거라면 차라리 a 태그일 때 preventDefault() 해주는게 나을 것 같아요. 맨 뒤에서 하니까 무슨 이유에서 맨 뒤에서 하는지 코드를 짜지 않은 사람의 입장으로 보면 어색해보입니다!
src/js/router/index.js
Outdated
| // TODO: API요청으로 데이터 가져온 후 변경된 데이터인지 확인하는 함수로 수정 | ||
| function isChangedData() { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
router 가 너무 많은 일을 하고 있는 느낌이에요! path 변경과 그에 따른 component 선택만해도 충분할 것 같은데, 렌더링 관련 일까지 하니 router 가 너무 커진 느낌입니다
There was a problem hiding this comment.
request changes 에 남겨주신 코멘트(디렉토리 정리, views에 view와 관련되지 않은 로직이 있음)와 요 코멘트(라우터 내 라우터의 역할이 아닌 로직) 참고해서 전체 구조와 라우터의 역할을 수정해봤습니다!
처음 디렉토리 구조잡을 때만 해도 만족스럽게 짰다고 생각했는데,,,, 리뷰어님의 피드백을 듣고나니 왜 이렇게 했지,,,? 싶네요,,,, 👉👈 개인적으로 전체적인 구조를 잡고 그 틀을 지켜나가는 훈련이 아직 많이 필요한 것 같습니다...! 성장에 도움이 되는 피드백 감사드립니다 💙💙💙
주어진 과제 템플릿에서 사용하지 않는 ui/modal.js과 같은 파일 및 디렉토리는 삭제하고, views 디렉토리를 componenets 디렉토리로 이름변경하였습니다. components내에서 view 역할을 수행하고 있던 index.js를 모두 view.js 이름변경하였습니다.
또 router에서 수행하고 있던 render와 관련된 로직을 모두 ./components로 분리했습니다!
수정 후 디렉토리 구조와 역할이 보시기에 괜찮은지 다시 한 번 봐주시면 감사하겠습니다,,,,🙇🏻♀️
src/js/views/auths/login/validate.js
Outdated
| }; | ||
|
|
||
| export const validateEmail = generateInputValidator(getEmailValidityMessage); | ||
| export const validatePassword = generateInputValidator(getPasswordValidityMessage); |
There was a problem hiding this comment.
아무도 안합니다... 🥲 아래와 같은 에러핸들러를 만들고 try...catch문을 이용해서 catch (error) { handleError(error) }와 같은 식으로 일관성있게 에러를 처리할 수 있도록 코드를 수정해보았습니다!
function handleError(error) {
reportError({
messageToUser: AUTH_MESSAGES.INPUT_VALIDATION_HAS_BEEN_FAILED,
messageToLog: error.message,
});
}
src/js/utils/DOM.js
Outdated
| return; | ||
| } | ||
|
|
||
| const customValidityMessage = await getValidityMessage($input); |
There was a problem hiding this comment.
파라미터로 넘기는 함수(getValidityMessage) 가 promise 가 아닌데 await 를 사용한 이유가 있을까요?
There was a problem hiding this comment.
해당 함수에서 인자로 받아 실행되는 getValidityMessage 함수는 다음의 세 가지 값을 가질 수 있습니다.
- get
EmailValidityMessage : 메세지 결정할 때 fetch 요청 있음 => await 필요함 🙆🏻♀️ 🙆🏻♀️ 🙆🏻♀️ - get
NameValidityMessage : 메세지 결정할 때 fetch 요청 없음 => await 필요 없음 - get
PasswordValidityMessage : 메세지 결정할 때 fetch 요청 없음 => await 필요 없음
이메일의 경우, 사용자에게 띄워줄 메세지를 결정하기 위해 중복검사 API요청을 보내야하지만, 사용자이름, 비밀번호는 JS코드만으로 사용자에게 띄워줄 메세지를 결정할 수 있습니다. 이렇게 세 가지 경우 중 단 한가지 경우에 비동기 처리가 있어서 await를 붙여주게 되었습니다. 그래서,,, 나머지 name과 password의 경우에는 await를 하지 않아도 되는데 await를 붙여주고 있는게 맞습니다,,, 이런 코드가 나온 이유를 설명드려보자면,,, 사용자에게 보여줄 메세지를 결정하는 부분을 이메일이든, 사용자이름이든, 비밀번호든 일관성 있게 getValidityMessage라는 함수를 인자로 받아서 message를 결정하고 다시 함수를 return해주는 방식을 가져가려고 한 것이었습니다,,,, 요기에 await를 작성할 당시에 요렇게 비대칭적으로 사용해도 되나 고민은 되었지만👉👈 getEmailValidityMessage 의 경우 반드시 await가 필요하니까 이렇게 작성했었습니다...! 혹시 이렇게 작성하면 안되는 것일지 궁금합니다...!! 🙏
There was a problem hiding this comment.
그러면 추상화가 잘못된 부분이 아닐까요~?
validate 는 단순 유효성 체크를 하는 함수라 기대할텐데, email 의 경우 api 호출까지 하고 있다면 다른 곳과는 추상화레벨이 다르다고 생각합니당!
물론 동작은 기대하는대로 하겠지만, 나중에 다시 한 번 생각해보면 좋을 것 같아요!
| } | ||
|
|
||
| return Object.values(obj).includes(value); | ||
| }; |
There was a problem hiding this comment.
여기서 throw 하는 TypeError 도 처리해주는 곳이 없네요!
There was a problem hiding this comment.
🥲🥲,,, 꼼꼼한 피드백 감사드립니다,,,!! 덕분에 페어와 함께 에러가 무엇인지 에러 핸들링은 어떻게 해야 잘하는 것인지 진득하게 고민해 볼 수 있었습니다...!
질문드립니다! 🙋🏻♀️ - 에러핸들링
1. 얼리리턴 vs try...catch
에러 핸들링을 보완하면서 가장 먼저 직면한 질문은 어떤 경우에 early return을 하고, 어떤 경우에 throw error를 해서 catch하면 좋을까 였습니다. (무엇을 에러로 볼 것이냐 라는 질문과도 연결되는 것 같습니다.)
에러 핸들링 방식에 대해 고민하던 중, 런타임 에러가 아니라 CustomError를 직접 throw해서 에러를 능동적으로 발생시키고 catch하는 예제코드를 접했습니다. HTTP통신에서 200번대 이상의 상태코드를 반환받으면(예를 들어 이메일이 중복되어 422코드를 받으면) CustomError를 throw하는 식이었습니다. 사실 이 경우에 CustomError를 사용하지 않고도 얼리리턴으로 충분히 처리가 가능하다고 생각했습니다. 이렇게 CustomError를 사용하는 방식에 저희가 미처 생각못한 이점이 있는 것일지 궁금했습니다.
결론적으로 저희는 코드 전체에서 일관적으로 에러를 처리하고, 적절하게 사용자에게 보여줄 수만 있다면 얼리리턴을 해도 되고 try...catch를 해도 된다고 생각했습니다. (정해진 답은 없다고 생각했습니다.) 따라서 예측가능하고 사용자에게 적절하게 보여줄 수 있는 경우라면, 처리 후 즉시리턴 해주고, 그게 아니라면 catch문에서 처리한다.는 원칙을 가져가기로 했습니다.
저희가 고민한 내용에 틀린 점이 있을지, 그리고 이 두 방식에 대해 리뷰어님께서는 어떻게 생각하시는지 궁금합니다,,,! 🙇🏻♀️
2. 에러 vs 예외 vs 버그
에외이란 의도치 않게, 예기치 않게 앱을 중지시키는 것이고, 그런 경우에도 앱이 동작하도록 만들어주는 것이 에러 핸들링의 목적이라고 생각했습니다. 그런데 런타임에러가 발생하지 않을 상황에서 능동적으로 throw Error를 해주는 방식을 보고 제가 에러와 예외를 조금 잘못 이해하고 있다고 느꼈습니다...! 혹시 실무에서는 에러(Error), 예외(Exception), 버그(Bug) 라는 용어를 구분해서 사용하시는지 조심스레 질문드려 봅니당,,,👉👈
3. 예측불가능한 에러 다루기
에러를 예측할 수 있는 에러와 예측할 수 없는 에러로 구분할 때, 예측할 수 없는 에러로 인해 앱이 정지하는 것을 예방하기 위해서는 어떤 장치를 두면 좋을까요...? 꼼꼼한 에러핸들링을 하고 싶어서 이런 생각을 해보았는데요,,, 극단적으로 모든 모듈스코프에 try...catch를 두는 방법을 생각해봤는데,,, 작성 비용과 전체 코드의 가독성을 생각하면 좋은 방법은 아닌 것 같습니다,,,, 🥺,,,, 예측 불가능한 에러의 핸들링에 대해서 관련해서 간단하게 조언해주시면 감사하겠습니다,,,💙💙💙
4. 에러 리포트 방법?!
저희는 개발자에게는 에러 그대로를, 사용자에게는 에러 그대로가 아니라 '적당히' 사용자가 이해하기 쉬운 언어로 보여주어야 한다고 생각했습니다. 따라서 catch에서 실행하는 에러핸들러 함수에서 개발자를 위해 남기는 로그에는 error.message를 그대로, 사용자에게는 커스텀 메세지를 보여주고 있습니다,,, 이렇게 에러핸들러에서 개발자를 위한 메세지, 사용자를 위한 메세지를 함께 처리하는 것이 괜찮은 방법인지 궁금합니다,,, 👉👈
function handleError(error) {
reportError({
messageToUser: AUTH_MESSAGES.INPUT_VALIDATION_HAS_BEEN_FAILED,
messageToLog: error.message,
});
}There was a problem hiding this comment.
훠우 질문이 기네요! ㅋㅋㅋㅋㅋㅋㅋ
-
저는 이렇게 생각하는 것 같아요. 처리 후 early return 의 경우에는 보통 사용자에게 피드백을 준 후 return 을 하는 경우일텐데요! 만약 api 를 모아둔 파일에서 이런 일을 한다면, api 가 너무 많은 일을 하고 있는 경우겠죠? 사용자 피드백을 화면에 노출하는 렌더링 역할까지 하는 셈이니까요! 이런 경우에는 Error 를 던지거나 에러메세지를 리턴해 api 를 사용하는 사용처에서 에러 핸들링을 하게 하겠죠!
다른 측면으로는 컴포넌트 단에서 예외처리를 하는 경우에는 early return 을 해도 되겠죠? 컴포넌트가 다 처리를 해도 되니까요!
이런 방식으로 생각하면 될 것 같아요~! 그리고 어디는 early return, 어디는 에러 던지기 이렇게 중구난방으로 하지 않고 통일성을 갖추면 좋을 것 같아요! -
보통 버그는 개발자가 아닌 다른 직군의 사람들과 이야기할 때 주로 얘기하게 되는 것 같구요, 저희는 예외 처리가 되어있지않다~ 로 이야기하게 되는 것 같습니다!
-
보통의 최상위 컴포넌트에서 예외처리를 추가해 한번 감싸는 구조로 할텐데요, 곧 배우게 될 리액트에서는 에러바운더리 라는 것도 이용하게 됩니당! 한번 구경만 해보세요! ㅋㅋㅋㅋㅋ
-
아주 좋죠! 배민도 사용자에게는 좀더 정제된 메세지를, 저희 개발자가 보는 로그는 있는 그대로의 메세지를 남깁니당!
| } | ||
|
|
||
| return ''; | ||
| }; |
There was a problem hiding this comment.
getEmailValidityMessage 함수 이름과는 달리 안에서 너무 많은 일을 하고 있네요! api 호출까지 하고 있으니까요!
validation 만 하는 함수로 수정해보는건 어떨까요?
There was a problem hiding this comment.
getEmailValidityMessage에서 메세지를 결정하기 위해서는 이메일 중복여부 검사도 담당해야 한다고 생각했습니다...! 중복이메일인지 검사하는 API요청이 validation의 일환이라고 생각했고, 이 로직을 다른 곳으로 분리하기 어렵다고 생각해서 그대로 두었는데 혹시 제가 방향을 잘못 잡았다면 다시 한 번 피드백 부탁드리겠습니다,,,!! 🙏
|
|
||
| export const validateName = generateInputValidator(getNameValidityMessage); | ||
| export const validateEmail = generateInputValidator(getEmailValidityMessage); | ||
| export const validatePassword = generateInputValidator(getPasswordValidityMessage); |
src/js/views/index.js
Outdated
| import { renderSignUp } from './auths/signUp/index.js'; | ||
| import { ROUTES } from '../constants/index.js'; | ||
|
|
||
| const render = { |
There was a problem hiding this comment.
앗 이 부분은 key값을 ROUTES로 시작하는 경로명으로 사용하고 있지만, 결국 경로(key값)에 따라 그에 알맞는 render함수를 반환하는 객체이기 때문에 render로 이름지어 보았습니다...! 처음에는 renderByPathname과 같은 함수명을 생각해봤는데 인자로 pathname을 받고 있기 때문에 pathname을 두 번 써주지 않아도 의미전달이 되지 않을까 생각했습니다...! value값에 있는 render-*로 시작하는 함수들은 components 디렉토리 내에 view.js에서 화면을 그려주는 함수들이고, 이 함수들은 아래와 같이 render라는 이름으로 호출됩니다,,, 저희가 의도한 기능은 render에 가까운 것 같은데 혹시 저희가 놓친 부분이 있다면 다시 한 번 피드백 주시면 감사하겠습니다,,,!!! 🙇🏻♀️
// router/index.js
export function goTo(pathname) {
addPathnameToBrowserHistory(pathname);
render(pathname); // ***** 호출!!
}// render-* 예시
// components/auths/login/view.js
export const renderLogin = ($parent) => {
$parent.innerHTML = TEMPLATE;
const $form = $parent.querySelector('form');
const $email = $parent.querySelector('#email');
const $password = $parent.querySelector('#password');
$email.addEventListener('input', validateEmail);
$email.addEventListener('blur', validateEmail);
...
};
webpack.config.js
Outdated
| ignored: /node_modules/, | ||
| }, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
devserver 가 있는데 package.json 의 script 에서는 활용하고 있지 않아보이네요!
There was a problem hiding this comment.
webpack-dev-server를 serve로 줄여서 사용할 수 있는 것으로 알고 있습니다...! package.json에서 script를 아래와 같이 적어두고 실행 시에는 start, 빌드 시에는 build 커맨드로 써보고 있습니다,,,!
// package.json
"scripts": {
"start": "webpack serve --mode=production",
"start:dev": "webpack serve --mode=development",
"build": "webpack --mode=production",
"build:dev": "webpack --mode=production",
"test": "yarn cypress open"
},
|
혹시 데모페이지는 없을까요~? |
|
아직 데모페이지를 준비하지 못했습니다,,,죄송합니다,,, 🙇♀️🙇♀️ + 👇 데모페이지 추가했습니다! 데모페이지 바로가기관련커밋: e2b1c73, 024f8b4 |
Co-authored-by: 365kim <365kim@users.noreply.github.com>
Co-authored-by: 365kim <365kim@users.noreply.github.com>
Co-authored-by: 365kim <365kim@users.noreply.github.com>
Co-authored-by: 365kim <365kim@users.noreply.github.com>
Co-authored-by: 365kim <365kim@users.noreply.github.com>
Co-authored-by: 365kim <365kim@users.noreply.github.com>
src/js/router/validate.js
Outdated
| // eslint-disable-next-line no-console | ||
| console.error(error); | ||
|
|
||
| return ROUTES.HOME; |
|
크 이렇게 정리도 해주시다니.. 감사합니당 👍 |
|
앗 넵 맞습니다...!! 😁 |
Hyeona님, 안녕하세요!
처음 인사드리는 🙋🏻♀️ 하루(@365kim) 입니다. 앞으로 3주간 잘 부탁드리겠습니당!!! 👍👍👍
저는 동동(@bigsaigon333)과 함께 페어프로그래밍을 진행했습니다.
요약
지하철 노선도 미션 - 1단계
Webpack을 통한 번들링
라우팅 기능
회원 기능
name,email,password이다./루트 페이지로 돌아온다.바쁘신 와중에 시간내서 리뷰해주셔서 정말 감사드립니다!!! 🙇🏻♀️
크고 작은 개선사항 코멘트로 전해주시면 열심히 개선해보겠습니다!! 🙏
감사합니다 😆