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

[박상준] Week15 #476

Conversation

sj0724
Copy link
Collaborator

@sj0724 sj0724 commented May 26, 2024

요구사항

기본

  • 링크 공유 페이지의 url path를 ‘/shared/{folderId}’로 변경하고, {folderId}에 해당하는 폴더 데이터가 화면에 보이게 해주세요.
  • 폴더 페이지의 url path를 전체 링크를 보는 경우 ‘/folder’로, 특정 폴더를 보는 경우 ‘/folder/{folderId}’로 변경하고, {folderId}에 해당하는 폴더 데이터가 화면에 보이게 해주세요.

심화

  • https://bootcamp-api.codeit.kr/docs 에서 인증이 필요한(자물쇠 아이콘이 있음) api의 경우 Authorization 리퀘스트 헤더에 “Bearer {accessToken}”을 함께 보내야 합니다.

주요 변경사항

  • 링크 공유 페이지 경로에 폴더 아이디, 유저 아이디 포함에서 전달하도록 구현했습니다.
  • 환경변수로 url, 카카오톡 키값 저장하고 있습니다.
  • 주요 기능(폴더 추가, 링크 추가, 수정) 구현 했습니다.
  • 이미지 태그로 이미지 최적화 했습니다.
  • 반응형 웹 수정중입니다.

스크린샷

멘토에게

  • https://sj-linkbrary.vercel.app/
  • 유저 기능은 현재 수업을 듣고있어서 구현못했고 대부분의 기능은 구현했습니다. 전역변수 라이브러리를 도입해보고 싶은데 recoil을 사용해보려고 합니다. recoil도 많이 사용되는지 궁금합니다!
  • 전역변수 라이브러리로 관리할만한 데이터를 고민중인데 어떤 값들이 통상적으로 전역으로 관리되나요?

sj0724 added 27 commits May 20, 2024 11:07
@sj0724 sj0724 requested a review from o-seung-yeon May 26, 2024 10:58
@sj0724 sj0724 self-assigned this May 26, 2024
@sj0724 sj0724 added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label May 26, 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.

상태 관리 라이브러리

몇가지를 선택해 패키지 다운로드 수로 비교해보면 redux 가 가장 많고 recoil 이 비교군에선 가장 낮네요. 국내 채용 공고에는 redux 가 많이 보였는데 요새는 zustand 나 다른 라이브러리도 좀씩 보이는 것 같더라고요 (제 뇌피셜이니 신뢰도가 아주 떨어집니다 ^.ㅜ)
recoil 은 사용 방법이 간단했던 걸로 기억합니다. 라이브러리 간에 장단점을 조건별로 비교해보시고, 사용법도 예시 코드로 보고 결정해보시면 좋을 것 같습니다.
https://npmtrends.com/jotai-vs-mobx-vs-recoil-vs-redux-vs-zustand

전역으로 관리하는 데이터

절대적인건 없지만, 페이지 전반에서 사용되는 인증 상태나 유저 정보, 스타일 테마, 언어, 모달 등의 Ui 상태가 있을 것 같습니다.


effect 하나에 여러 처리를 하기보다 실행돼야하는 시점에 맞게 분리해주시면 좋을 것 같고, 상태 변수나 props 네이밍 신경써주시면 더 파악하기 좋은 코드가 될 것 같습니다.

고생하셨습니다 👏

@@ -26,6 +26,7 @@ yarn-error.log*

# local env files
.env*.local
.env
Copy link
Collaborator

Choose a reason for hiding this comment

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

.vscode 폴더도 추가해주세요~!

Comment on lines +3 to +14
axios.interceptors.request.use(
(config) => {
const accessToken = localStorage.getItem('token');
config.headers['Authorization'] = accessToken;

return config;
},
(error) => {
console.log(error);
return Promise.reject(error);
}
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

헤더 값을 일괄적으로 적용해주셨네요 👍

}: {
item: LinkData;
setUrl: Dispatch<SetStateAction<string>>;
setUrl?: Dispatch<SetStateAction<string>>;
onSelect?: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

onSelect 가 나타내는 값이 어떤건지 모호합니다.
on~ 은 보통 어떤 액션에 대한 콜백 함수 네이밍할 때 붙여서 혼동이 있을 것 같아요~
폴더 정보인지, 링크 정보인지 명시적으로 네이밍해주시면 좋을 것 같습니다!

id: string;
name: string;
};
setLinkId?: Dispatch<SetStateAction<number>>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

컴포넌트 내부에서 선언한 상태를 업데이트 하는 것과 구분하기 위해 이 속성명을 onSelect: (id: number) => void; 로 해도 좋을 것 같습니다.

Comment on lines +35 to +37
if (setLinkId) {
setLinkId(id);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

함수가 옵셔널이라면 setLinkId?.(id) 로 축약할 수 있으니 참고해주세요!

Comment on lines +28 to +49
useEffect(() => {
const loadOwnerFolderData = async () => {
const folder = await getFolderData(folderId);
if (folder) {
setFolderName(folder[0].name);
setUserId(folder[0].user_id);
}
};

const loadOwnerData = async () => {
const user = await getUserData(userId);
if (user) {
setOwner(user[0]);
}
};
if (folderId) {
loadOwnerFolderData();
}
if (userId) {
loadOwnerData();
}
}, [folderId, userId]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

deps 에 따라 서로 영향을 미치는게 없어서 두개의 effect 로 분리해서 처리해주세요.
둘 중 하나만 변경되고 내부 로직이 실행될 때, 변경되지 않은 조건의 평가값이 true 면 필요 이상으로 호출될 수 있을 것 같습니다.

userId 가 deps 에 있고, 내부에 setUserId 로 업데이트 해주는 부분이 있어 무한 업데이트에 빠질 수도 있습니다..!

Comment on lines +52 to +65
useEffect(() => {
const access = localStorage.getItem('token');
if (!access) {
router.replace('/');
return;
}
const observer = new IntersectionObserver(handleObserver);
if (!loading && obsRef.current) {
observer.observe(obsRef.current);
}
return () => {
observer.disconnect();
};
}, [loading, folderId, router]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

하나의 Effect 에서 인증 확인과 옵저버 등록을 처리하고 있어서 기능별로 effect 를 분리해주시면 좋을 것 같고,
옵저버 쪽은 handleObserver 을 포함해서 커스텀훅으로 분리해 호출해오면 관심사 분리를 통해 더 파악하기 쉬운 코드가 될 것 같습니다.

const [wrongFolder, setWrongFolder] = useState(false);
const [linkId, setLinkId] = useState(0);
const router = useRouter();
const folderId = router.query.folderId as string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

query segment 값이 유니언이라 타입 단언으로 처리해주셨네요~

타입 가드를 통해 더 명시적으로 값을 가져올 수 있을 것 같습니다.

// 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;
}

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

const router = useRouter();
const folderId = router.query.folderId as string;
const { linkList, loading } = useGetFolder(id, searchKeyword, folderId);
const { link, linkLoading } = useGetFolderList(id, folderId);
Copy link
Collaborator

Choose a reason for hiding this comment

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

요 link 값이 전체 폴더 리스트에 대한 값일까요..??

Comment on lines +67 to +76
useEffect(() => {
for (let i = 0; i < link.length; i++) {
setWrongFolder(true);
if (link[i].id == folderId) {
setOnSelect({ id: folderId, name: link[i].name });
setWrongFolder(false);
break;
}
}
}, [link, folderId]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

link 배열을 순회하며 쿼리로 받아온 folderId 와 일치하는 걸 onSelect 상태에 넣어주는 것 같아요.
맥락상 link 배열이 전체 폴더 리스트에 대한 값일 것 같은데 맞을까요..?
파악이 조금 어려워서 변수명 한번 검토해주세요~

onSelect 상태를 아래처럼 처리하고 wrongFolder 는 selectedFolder 가 undefined 인 상태를 체크하면 제거할 수 있을 것 같습니당

const selectedFolder = useMemo(() => {
  // link 가 폴더 목록인 것 같아서 folders 로 명명했습니다.
  return folders.find((folder) => folder.id === folderId);
}, [folderId, folders]);

@o-seung-yeon o-seung-yeon merged commit f791c80 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