[1단계 - 장바구니 협업] 유세지(김용래) 미션 제출합니다. #20

usageness · 2022-06-02T08:45:01Z

📢 프로젝트 결과물은 아래를 확인해주세요!

📕 스토리북 페이지

🚫 데모 페이지

실제 서버와 연동할 수 있도록 구성하여 기존에 배포 된 데모 페이지에서는 정상적인 테스트가 불가능합니다.
아래의 명령어로 로컬 환경에서 실행해주세요!

npm install

npm run start

처음 뵙겠습니다, 헤인티! 이번 미션의 리뷰이가 된 유세지입니다. 만나서 반갑습니다 🙌
기존에 어느정도 진행되었던 프로젝트라 쉽게 할 수 있을 줄 알았는데
생각보다 개발 외적인 부분에서 노력이 들어갔던 미션이었네요 😅
이번 협업 미션을 진행하며 신경썼던 부분과 아쉬웠던 부분들을 아래에 정리해 보았습니다.
감사합니다 😊

아쉬웠던 점

1. 백엔드 팀원과의 활발한 의사소...통?

처음 회의를 진행하며 API 명세를 정할때, 프론트엔드 크루와 백엔드 크루간의 배경지식 차이가 있어
URI를 정하는 방식 등 논의하기 다소 난감한 주제들이 있었습니다.
매일 이슈나 궁금한 사항이 생길때마다 자주 이야기를 나누며 어느정도 간극을 줄이고는 있지만
좀 더 효율적인 논의 과정이나 꼭 알아야 할 배경 등이 있을 것 같은 생각이 드네요 😅
혹시 현업에서는 어떻게 의견을 나누는지 헤인티님의 경험을 공유해주시면 감사하겠습니다 🙏

2. 보안 이슈

현재 로그인 시 accessToken만을 응답으로 받아 세션 스토리지에 저장하고 있습니다.
refreshToken이 별도로 없는 상태에서 토큰 하나에 로그인 상태를 의지하고 있는 구조가
보안상으로 그렇게 좋아보이지는 않는데, 프론트에서 자체적으로 탈취를 방지할 수 있는
방법이 없을까요? 프론트만으로는 한계가 명확하겠지만... 그렇다고 가만히 있기에는 불안하네요 😭

3. 인증의 위치

회원정보 수정과 같은 로그인 상태에서만 접근 가능한 페이지를 비로그인 상태일때 접근하지 못하도록하는
기능을 구현하려고 했는데, PR 시간에 맞추지 못해서 To do list로 남겨두었습니다...ㅠㅠ
당장 떠오르는 방법은 두 가지인데, 이러한 잘못된 접근을 라우터 단에서 차단해주는 방법과
해당 권한이 필요한 페이지 컴포넌트 (ex. EditUserInfo.jsx) 에서 차단하는 방법입니다.

접근 자체를 막아야하니 라우터에서 차단해주는게 맞을 것 같은데,
그렇게 한다면 Auth 함수를 만들어서 아래처럼 구현해주는게 맞을까요?

const Auth = (component) => {
  if (isLogin) return component;
  return <defaultComponent />
}

// 라우터
<Route ... element={Auth(<Component />)} />

🚩 구현된 요구사항

컴포넌트 구현

Input 컴포넌트 구현
로그인 페이지 컴포넌트 구현
Icon 컴포넌트 구현
ErrorMessage 컴포넌트 구현
회원가입 페이지 컴포넌트 구현
본인확인 페이지 컴포넌트 구현
회원 정보 수정 페이지 컴포넌트 구현
비밀번호 변경 페이지 컴포넌트 구현
스피너 컴포넌트 구현

API

기능

Co-authored-by: JASUN LEE <[email protected]>

pocojang

@HyeonaKwon
빠른 미션 수행을 위해 먼저 머지합니다
하지만 6/4일까지는 추가적으로 리뷰 부탁드립니다 :)

HyeonaKwon

안녕하세요 세지님! 처음 뵙네요 :)

다른 리뷰어분께 리뷰 받았던 코드일거라 style 과 storybook 코드는 따로 보지 않았는데 혹시 필요하시면 말씀해주세요~

질문답변

백엔드 팀원과의 활발한 의사소...통?
저는 크게 이해되지 않는 부분이 아니라면 백엔드에 맞추는 편입니다! 그리고 정 타협이 안된다면 api 코드 사이에 dto 를 하나 둬도 되구요! (~~뭐 근데 이렇게 할 필요가 있을만큼 의견이 안맞는 일은 거의 없..~~)
보안 이슈
사실 어떻게 해도 프론트에서는 노출될 수밖에 없어요~ 그래서 token 으로 주고받기를 하는 것이구요. 보통은 token 의 유효기간이 끝나면 서버에서 끝났다고 알려주고 프론트에서는 로그인 페이지로 이동시키죠?!
인증의 위치
네네 세지님 말씀대로 해도 될 것 같아요. Hoc 에 대해서 공부해보실래요?!

HyeonaKwon · 2022-06-04T13:21:22Z

src/ShoppingCartApp.jsx

+        <Route path="/editUserInfo" element={<EditUserInfo />} />
+        <Route path="/editUserPassword" element={<EditUserPassword />} />


요런것들은 path 를 공통으로 가져가볼 수 있지 않을까요?

/edit/info, /edit/password

뭐 이런식으로요! 그러면 중첩 라우터를 이용해볼 수도 있을 것 같아요! 보통 정보수정 페이지는 레이아웃이 정해져있으니!

공통 부분을 가지면 path로도 묶어줄 수 있겠네요!
outlet을 이용하여 중첩 라우팅을 적용해보았습니다 😊

refactor : 정보 수정 페이지 중첩 라우팅으로 변경

HyeonaKwon · 2022-06-04T13:23:27Z

src/actions/cart.js

+import { 장바구니_액션 } from './types';
+
+const addCartList = (product, cartList) => {
+  const foundExistProduct = cartList.find((item) => item.id === product.id);


변수명 너무 어렵네요.. targetProduct 뭐 이런건 어떠신가요?

며칠뒤에 다시 보니까 저도 읽기 힘든 변수 이름이네요... 😥

refactor : 장바구니 액션의 기존 상품 변수 이름 변경

HyeonaKwon · 2022-06-04T13:35:47Z

src/actions/products.js

+  if (response.status === 비동기_요청.FAILURE) {
+    dispatch({
+      type: 상품리스트_불러오기_액션.FAILURE,
+      payload: response.content,
+    });
+
+    return;
+  }
+
+  dispatch({
+    type: 상품리스트_불러오기_액션.SUCCESS,
+    payload: response.content,
+  });
+};


액션들 전부 마찬가지이지만 if 문을 추가하지 않고 삼항 연산자로 나누어야할 부분만 나눠보는게 어떨까요?

Suggested change

if (response.status === 비동기_요청.FAILURE) {

dispatch({

type: 상품리스트_불러오기_액션.FAILURE,

payload: response.content,

});

return;

}

dispatch({

type: 상품리스트_불러오기_액션.SUCCESS,

payload: response.content,

});

};

const actionType = response.status === 비동기_요청.FAILURE

? 상품리스트_불러오기_액션.FAILURE

: 상품리스트_불러오기_액션.SUCCESS;

dispatch({

type: actionType,

payload: response.content,

});

헉.. early return 형태만이 능사가 아니었네요!
삼항 연산자를 사용해서 나눠주고, 중복이 발생해서 유틸 함수로 분리해주었습니다!

refactor : action dispatch 로직 유틸 함수로 분리

// utils/asyncDispatchAction.js const asyncDispatchAction = (dispatch, response, action) => { const actionType = response.status === 비동기_요청.FAILURE ? action.FAILURE : action.SUCCESS; dispatch({ type: actionType, payload: response.content, }); };

HyeonaKwon · 2022-06-04T13:38:32Z

src/actions/types.js

+  장바구니_액션,
+  스낵바_액션,
+  스피너_액션,
+};


한글 변수명을 사용하셨군요ㅋㅋㅋㅋ 저는 익숙하지 않아서 뭔가 더 헷갈리는 것 같네요 😂

길고 복잡한 변수명은 한글 변수가 더 편리하더라구요! ㅋㅋㅋ
이 프로젝트에는 그 정도로 어려운 변수명은 딱히 없지만...
~~이번 기회에 Hip 한 코드를 써보고 싶었습니다 😎~~
혼란을 드려서 죄송합니다!

HyeonaKwon · 2022-06-04T13:57:14Z

src/hooks/useCheckBox.js

+    });
+  };
+
+  const isChecked = (productId) => checkboxItems.findIndex((id) => id === productId) !== -1;


이런거는 some 을 사용해보는게 어땠을까 싶네요!

some이 더 깔끔하네요!
고차함수 활용 넘나 어려운 것 같아요... 😂

refactor : checked 확인 로직 findIndex -> some으로 변경

HyeonaKwon · 2022-06-04T14:18:27Z

src/pages/EditUserPassword.jsx

+    if (response.status === 비동기_요청.SUCCESS) {
+      dispatch(snackbar.pushMessageSnackbar('비밀번호를 수정 하였습니다!'));
+      navigate('/');
+      return;
+    }
+    dispatch(snackbar.pushMessageSnackbar('비밀번호를 수정이 실패하였습니다!'));
+    navigate('/');


위의 리뷰와 마찬가지로 if 문 대신 삼항 연산자로 가독성 좋게 해보는게 어떨까.. 싶습니다! (취향 차이일 수도 있어요 ㅋㅋㅋ)

const message = response.status === 비동기_요청.SUCCESS ? '비밀번호를 수정 하였습니다!' : '비밀번호를 수정이 실패하였습니다!'; dispatch(snackbar.pushMessageSnackbar(message)); navigate('/');

저도 이렇게 하는쪽이 가독성 측면에서 좋다고 생각하는데,
되도록이면 삼항 연산자 사용을 자제하라는 이야기를 많이 들어서 고민되더라구요.
상황에 따라 다르겠지만 그 후로 early return을 의식적으로 사용해왔는데...
삼항 연산자는 어떤 점에서 가독성이 안좋다는 이야기가 나오게 된걸까요?
? 에 대한 헤인티님의 견해가 궁금해요 😂

refactor : early return 문을 삼항 연산자로 수정

세지님 피드백 남기면서 다시 PR 돌아보니 이 코멘트를 제가 놓쳤군요! 😂
if 와 삼항연산자는 사람의 취향차이이겠지만, 저는 그 때 그 때 가독성이 좋은 쪽을 선택합니다!

HyeonaKwon · 2022-06-04T14:19:05Z

src/pages/EditUserPassword.jsx

+              />
+            </label>
+
+            <label html-for="input-user-newpassword">


label 안에 input 을 넣으면 html-for 를 작성하지 않아도 돼요!

앗.. 몰랐던 사실을 배워갑니다.. 😂

refactor : 의미없는 html-for 속성 제거

HyeonaKwon · 2022-06-04T14:20:46Z

src/pages/ProductList.jsx

+
+  useEffect(() => {
+    if (productList?.length === 0) dispatch(getProductList());
+  }, [dispatch, productList?.length]);


deps 에 undefined 가 들어갈 수도 있다라..

ㅠㅠㅠㅠㅠㅠㅠㅠㅠㅠㅠ
이전에 reducer에서 productList 값을 건드리다가
기존 state의 요소를 spread로 받아와서 기존 값을 유지하며 갱신 시켜주어야 했는데
리덕스 사용이 익숙하지 않아서 깜빡하고 누락했었습니다...

그래서 product 내부의 items 배열(여기서는 productList) 이 사라지는 현상이 발생했고,
없는 프로퍼티를 참조하려니 오류가 발생해서... 급한대로 옵셔널 체이닝을 주었습니다.
나중에 빼먹고 있었다는 사실을 깨닫고 부랴부랴 spread 연산자를 추가해주었는데
이번에는 옵셔널 체이닝을 빼주지 않았네요...

죄송합니다 😭😭😭 ~~(이 spread의 여파로 밑에서 사과 한 번 더 드릴 예정입니다)~~

refactor : 불필요한 옵셔널 체이닝 연산자 제거

HyeonaKwon · 2022-06-04T14:21:34Z

src/pages/SignUp.jsx

+  const isValidForm = useMemo(
+    () =>
+      checkUserId &&
+      checkDuplicatedId &&
+      checkUserPassword &&
+      checkUserPasswordConfirm &&
+      checkUserAge &&
+      checkUserNickName,
+    [
+      checkUserId,
+      checkDuplicatedId,
+      checkUserPassword,
+      checkUserPasswordConfirm,
+      checkUserAge,
+      checkUserNickName,
+    ],
+  );


다른 코드들도 그렇지만 이렇게 다 deps 에 넣어줄바엔 그냥 useMemo 없이 로컬변수로 하는것은 어떨까.. 싶네요!

조금 오버스러운 느낌이긴... 하죠...? ㅎㅎ...
쓰면서도 이렇게 다 넣어도 되는건가.. 싶었어요 😅

refactor : 큰 의미없는 useMemo 제거

HyeonaKwon · 2022-06-04T14:23:22Z

src/reducers/cart.js

+
+    case 장바구니_액션.ADD_EXIST_PRODUCT:
+      return {
+        items: [...state.items].map((item) => {


어차피 map 이 새로운 배열을 반환해서 state.items 를 spreading 안해도 될거예요!

죄송합니다 😭😭😭 (이 spread의 여파로 밑에서 사과 한 번 더 드릴 예정입니다)

같은 맥락에서, spread 연산자를 부랴부랴 추가하다가... 여기에도 넣어버렸습니다...
죄송합니다 ㅠㅠㅠㅠㅠㅠㅠㅠㅠㅠㅠ

refactor : 불필요한 spread 연산자 제거

usageness · 2022-06-06T14:55:19Z

늦어서 죄송합니다 😥
연휴동안 개인 사정이 있어서 코드를 볼 시간이 없었네요...
피드백 주셨던 부분들 모두 확인하고, 반영하였습니다!
말씀해주신 Hoc에 대해서도 공부해볼게요 😊

한 가지 걸리는건, 코멘트로도 남겼지만 삼항 연산자의 가독성에 대해 살짝 의문이 남네요...
사람마다 상황마다 다른거긴 하겠지만, 아직은 언제 사용해야 할지에 대한 기준을 잘 모르겠어요 😅
요거 제외하고는 모두 이해했습니다!!!

자세하게 리뷰해주셔서 정말 감사합니다! 늦었지만 좋은 하루 되세요 🙏👍👍

usageness and others added 30 commits May 31, 2022 15:32

chore: project init

6743eac

docs : 요구사항 문서 작성

11c7a78

Co-authored-by: JASUN LEE <[email protected]>

feat: Input 컴포넌트 구현

64c6629

Co-authored-by: JASUN LEE <[email protected]>

style : 새 Input 컴포넌트 적용

5028bcb

Co-authored-by: JASUN LEE <[email protected]>

feat: 로그인 페이지 컴포넌트 구현

836d188

Co-authored-by: JASUN LEE <[email protected]>

feat: Icon 컴포넌트 구현

d5b0947

Co-authored-by: JASUN LEE <[email protected]>

feat: 에러 메시지 컴포넌트 추가

ac16179

Co-authored-by: JASUN LEE <[email protected]>

feat: 회원가입 페이지 컴포넌트 구현

5914a00

Co-authored-by: JASUN LEE <[email protected]>

feat : 본인확인 페이지 컴포넌트 구현

c48b97f

Co-authored-by: JASUN LEE <[email protected]>

feat : 회원 정보 수정 페이지 컴포넌트 구현

da1e712

Co-authored-by: JASUN LEE <[email protected]>

feat : 비밀번호 변경 페이지 컴포넌트 구현

f56ed6e

Co-authored-by: JASUN LEE <[email protected]>

feat : MSW를 활용한 API 모킹 구성

f37a603

Co-authored-by: JASUN LEE <[email protected]>

fix : 헤더 메뉴 구성 및 스타일 변경

881cfda

Co-authored-by: JASUN LEE <[email protected]>

feat : Token 기반 로그인 기능 구현

b95f882

Co-authored-by: JASUN LEE <[email protected]>

feat : 회원가입 기능 구현

3f4545c

Co-authored-by: JASUN LEE <[email protected]>

fix : 로그인 상태에 따른 헤더 적용

5198d9e

Co-authored-by: JASUN LEE <[email protected]>

feat : 회원 정보 수정 전 본인 확인 기능 구현

814a565

Co-authored-by: JASUN LEE <[email protected]>

feat : 회원 정보 수정 기능 구현

0762958

Co-authored-by: JASUN LEE <[email protected]>

refactor : 누락된 스토리북 추가 및 버그 픽스

18431a9

Co-authored-by: JASUN LEE <[email protected]>

fix : Authorization header 추가

67d9bf4

Co-authored-by: JASUN LEE <[email protected]>

feat : 회원 탈퇴 기능 구현

b15ea62

Co-authored-by: JASUN LEE <[email protected]>

feat : logout 기능 구현

bb47437

Co-authored-by: JASUN LEE <[email protected]>

feat : 로그인 시 유저 아이디를 받아와서 redux에서 관리

b7c1584

Co-authored-by: JASUN LEE <[email protected]>

feat : 로그아웃 기능 구현

1c7c298

Co-authored-by: JASUN LEE <[email protected]>

feat : 비밀번호 수정 기능 구현

c9378b6

Co-authored-by: JASUN LEE <[email protected]>

feat : Spinner 컴포넌트 구현

fcd53dd

Co-authored-by: JASUN LEE <[email protected]>

feat : 서버 응답 대기시 Spinner 애니메이션 띄워주기

bf73b3a

Co-authored-by: JASUN LEE <[email protected]>

fix : key 값으로 uuid 전달하여 오류 해결

75a6dd1

Co-authored-by: JASUN LEE <[email protected]>

refactor: user action과 api 로직 분리

a91ed95

Co-authored-by: JASUN LEE <[email protected]>

refactor: alert을 snackbar 메시지로 변경

d1b357a

Co-authored-by: JASUN LEE <[email protected]>

pocojang requested a review from HyeonaKwon June 3, 2022 01:25

pocojang assigned HyeonaKwon Jun 3, 2022

pocojang approved these changes Jun 3, 2022

View reviewed changes

pocojang merged commit 8873ce8 into woowacourse:usageness Jun 3, 2022

HyeonaKwon reviewed Jun 4, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1단계 - 장바구니 협업] 유세지(김용래) 미션 제출합니다. #20

[1단계 - 장바구니 협업] 유세지(김용래) 미션 제출합니다. #20

usageness commented Jun 2, 2022 •

edited

Loading

pocojang left a comment

HyeonaKwon left a comment

HyeonaKwon Jun 4, 2022

usageness Jun 6, 2022

HyeonaKwon Jun 4, 2022

usageness Jun 6, 2022

HyeonaKwon Jun 4, 2022

usageness Jun 6, 2022

HyeonaKwon Jun 4, 2022

usageness Jun 6, 2022

HyeonaKwon Jun 4, 2022

usageness Jun 6, 2022

HyeonaKwon Jun 4, 2022

usageness Jun 6, 2022

HyeonaKwon Jun 20, 2022

HyeonaKwon Jun 4, 2022

usageness Jun 6, 2022

HyeonaKwon Jun 4, 2022

usageness Jun 6, 2022

HyeonaKwon Jun 4, 2022

usageness Jun 6, 2022

HyeonaKwon Jun 4, 2022

usageness Jun 6, 2022

usageness commented Jun 6, 2022

		<Route path="/editUserInfo" element={<EditUserInfo />} />
		<Route path="/editUserPassword" element={<EditUserPassword />} />

[1단계 - 장바구니 협업] 유세지(김용래) 미션 제출합니다. #20

[1단계 - 장바구니 협업] 유세지(김용래) 미션 제출합니다. #20

Conversation

usageness commented Jun 2, 2022 • edited Loading

📢 프로젝트 결과물은 아래를 확인해주세요!

아쉬웠던 점

1. 백엔드 팀원과의 활발한 의사소...통?

2. 보안 이슈

3. 인증의 위치

🚩 구현된 요구사항

컴포넌트 구현

API

기능

pocojang left a comment

Choose a reason for hiding this comment

HyeonaKwon left a comment

Choose a reason for hiding this comment

질문답변

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

usageness commented Jun 6, 2022

usageness commented Jun 2, 2022 •

edited

Loading