Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[이현승] week 15 #469

Conversation

waterkail
Copy link
Collaborator

요구사항

기본

  • 링크 공유 페이지의 url path를 ‘/shared’에서 ‘/shared/{folderId}’로 변경해 주세요.
  • 폴더의 정보는 ‘/api/folders/{folderId}’, 폴더 소유자의 정보는 ‘/api/users/{userId}’를 활용해 주세요.
  • 링크 공유 페이지에서 폴더의 링크 데이터는 ‘/api/users/{userId}/links?folderId={folderId}’를 사용해 주세요.
  • 폴더 페이지에서 유저가 access token이 없는 경우 ‘/signin’페이지로 이동하게 해주세요.
  • 테스트 유저는 id: “[email protected]”, pw: “sprint101” 를 활용해 보세요.
  • 폴더 페이지의 url path가 ‘/folder’일 경우 폴더 목록에서 “전체” 가 선택되어 있고, ‘/folder/{folderId}’일 경우 폴더 목록에서 {folderId} 에 해당하는 폴더가 선택되어 있고 폴더에 있는 링크들을 볼 수 있게 해주세요.
  • 폴더 페이지에서 현재 유저의 폴더 목록 데이터를 받아올 때 ‘/api/folders’를 활용해 주세요.
  • 폴더 페이지에서 전체 링크 데이터를 받아올 때 ‘/api/links’, 특정 폴더의 링크를 받아올 때 ‘/api/links?folderId={folderId}’를 활용해 주세요.
  • 유효한 access token이 있는 경우 ‘/api/users’로 현재 로그인한 유저 정보를 받아 상단 네비게이션 유저 프로필을 보이게 해주세요.

심화

  • 리퀘스트 헤더에 인증 토큰을 첨부할 때 axios interceptors 또는 이와 유사한 기능을 활용해 주세요.

주요 변경사항

스크린샷

멘토에게

@waterkail waterkail requested a review from o-seung-yeon May 25, 2024 09:29
@waterkail waterkail self-assigned this May 25, 2024
@waterkail waterkail added the 순한맛🐑 마음이 많이 여립니다.. label May 25, 2024
Copy link
Collaborator

@o-seung-yeon o-seung-yeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현승님은 기능이나 동작 구현을 잘 하시는 것 같습니다!

여유가 된다면 가독성을 더 신경써보면 좋을 것 같아요~
직관적인 컴포넌트 이름과, 불리언 변수명은 조동사로 시작하기와 같은 네이밍부터
pr 을 올리고 셀프 리뷰를 통해 이 props 값을 꼭 외부에서 주입 받아야 하는가, state 를 더 줄일 순 없을까, 로직이 어떤 흐름으로 동작하는지 쉽게 파악되는가 를 고민해보시면 좋을 것 같습니다.

고생하셨습니다 👏

@@ -4,6 +4,7 @@
/node_modules
/.pnp
/WeeklyMissionPart1
/.next
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

음 왜인지 .next 폴더도 업로드 되었네요
처리 후 의도대로 제거 됐는지 확인해주시면 좋을 것 같아요~

}, [isLogIn, getData]);

return (
<UserContext.Provider value={{ user, isLogin: isLogIn }}>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isLogin: isLogIn 로 넣어주셨는데 둘 중 하나로 맞춰서 사용하면 좋을 것 같습니다

useEffect(() => {
const userToken = window.localStorage.getItem(LOGIN_TOKEN);
getData(userToken);
}, [isLogIn, getData]);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드를 보니 isLogIn 이 변경 됐을 때 토큰을 가지고 데이터를 요청해서 user 값과 로그인 상태를 나타내는 isLogIn 을 업데이트 시켜주는 것 같습니다.

그런데 isLogIn 은 초기값으로 true 이고, getData 에서 요청 상태에 따라 변경은 되지만 Provider 내부에서 isLogIn 을 변경시키는 부분이 없어서 어떤 역할을 하는지 잘 이해가 되질 않습니다.

최초로 서비스에 접속 했을 땐 로그인 돼있지 않아서 토큰 값이 없을 것이고, 그렇다면 getData 내부에서 isLogIn 을 false 로 변경하고, 다시 effect 가 실행되고, 또 토큰이 없어서 false 로 설정된다면.. 로그인 상태가 어디서든 업데이트는 돼야할 것 같은데 언제, 어떻게 업데이트 되는 걸까요..?!

유추해보자면 로그인 페이지에서 로그인 했을 때 getUser 변경되며 getData 도 변경되고 effect 가 실행됐을 때 토큰값이 존재하는 상태로 있을 것 같지만 이 파일의 로직만 보고는 파악이 어렵습니다..!

스토리지에 있는 값을 state 로 관리하고 해당 값이 변경 됐을 때 데이터를 불러오거나 로그인 상태를 체크하도록 로직을 짜면 어떨까요?

import { useRouter } from "next/router";
import { useUser } from "@/contexts/UserContext";

interface Item {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

사용처를 보니 폴더 정보를 담고 있는 인터페이스 같네요!
이름에서도 그런 정보를 알 수 있게 FolderItem 이나 Folder 로 명명하면 어떨까요?

Comment on lines +63 to +66
useEffect(() => {
const userToken = window.localStorage.getItem(LOGIN_TOKEN);
checkToken(userToken);
}, []);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이부분 + checkToken 을 합쳐서 커스텀 훅으로 분리하고 자격 증명이 필요한 페이지마다 재사용하면 좋을 것 같습니다!

count: number;
}

function folder() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

컴포넌트 함수 명은 대문자로 명명해주세요~

@@ -19,6 +19,7 @@ const AddLink = ({ folder, footView }: { folder: any; footView: boolean }) => {
e.preventDefault();
setAdd(!add);
setUrl(AddLink.current?.value);
console.log(folder);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

로그는 지워주세요~

return (
<App headerFixed={false} foot={foot}>
<SubHeader headerfixed={false}>
<AddLink folder={folderInfo?.data.folder} footView={footView} />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AddLink 컴포넌트의 folder props 사용처를 보니 모달을 열 때 필요한 것 같아요.
AddLink 에 직접적으로 사용되는 값은 �아닐것 같단 생각에 왜 필요한지를 찾아보게 됩니다.
내부를 보니 컴포넌트 내부에서 특정 상황에 필요한 값이고, 충분히 외부에서 다른 형태의 값을 받아서 folder 값에 대한 의존을 분리할 수 있을 것 같아요.
특정 상황(추가하기 버튼 클릭)에 실행시킬 함수를 콜백 Props 형태로 넘겨주면 어떨까요?

// AddLink.tsx
interface AddLinkProps {
  onAdd: () => void;
  footView: boolean;
}

// [[...folderid]].tsx
return (<AddLink onAdd={() => openFolderModal(folderId)} footView={footView} />);

Comment on lines +78 to +81
linkData={linkData}
selectFolder={selectFolder}
folderInfo={folderInfo?.data.folder}
selectedFolder={selectedFolder}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Folder 에 넘겨주는 네개의 값을 넘겨주고 있는데 folderInfo 말고는 Folder 컴포넌트 내부에서만 사용되는 값이라 이 페이지 컴포넌트 파일에서 넘겨주지 않고 내부에서 불러와서 사용할 수 있을 것 같습니다.

부모 컴포넌트에서 데이터를 관리해야하는 경우는 크게 두가지가 될 것 같습니다.

  1. 여러 자식 컴포넌트가 같은 값을 필요로 할 때
  2. 어떤 자식 컴포넌트에선 상태를 받아서 표시해주고, 또 다른 자식 컴포넌트에선 그 상태를 변경할 때 (ex. 검색 기능이 있는 페이지에서 검색창과 검색 결과 컴포넌트를 분리해서 사용하는 경우, 검색어와 검색어 변경을 부모에서 들고 있어야 상태를 공유할 수 있음)

근데 지금 상황에선 1, 2에 해당하지 않고 Folder 컴포넌트 자체도 어디서든 재사용되는 컴포넌트가 아니라서 내부에서 처리하는 게 좋을 것 같습니다. 그렇게 되면 부모의 역할이 줄게되고, 딱 필요한 역할만 보여 코드를 파악하기 더 좋을 것 같습니다.
(데이터가 필요할 때까지 작업을 미룬다고도 표현합니다.)

const { folderid } = router.query;

useEffect(() => {
if (typeof folderid !== "string") return;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

query의 프로퍼티 타입이 string | string[] | undefined 인걸로 알고 있습니다. 그래서 이렇게 타입 체크를 해주신 것 같아요 👍

요걸 더 깔끔하게 처리할 수 있는 방법이 있습니다. 타입 가드라고 하는건데요.

// util
const isStringQuery = (value: unknown): value is string => {
  return typeof value === 'string';
}

// hook
const useRouterQuery = (segmentName: string) => {
  const { query } = useRouter();

  const segment =  query[segmentName];

  if (!isStringQuery(segment)) {
    return;
  }
  return segment;
}

이런식으로 타입 가드 함수와 커스텀 훅 작성해서 세그먼트 값과 타입 관련된 관심사를 분리할 수 있을 것 같습니다.
그냥 코멘트로 남기는 코드라 동작이 되는지는 모르겠네요.. ㅎㅎ 한번 확인해보시고 수정해서 적용해보세요~

@o-seung-yeon o-seung-yeon merged commit cf6a7a5 into codeit-bootcamp-frontend:part3-이현승 May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
순한맛🐑 마음이 많이 여립니다..
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants