✨[Code Review] moorad code review #33
Conversation
There was a problem hiding this comment.
안녕하세요, UMC 7th 중앙 웹 파트장 매튜/김용민입니다. 먼저, 부족한 웹 워크북이지만, 성실하게 수행해주신 점 정말 감사드립니다!
전체적인 피드백 남깁니다!!
- widgets 폴더에 대한 개인적인 궁금증!
기존에 components 폴더를, widgets으로 네이밍을 변경해서 관리하는 것 같습니다!!
FSD 폴더 구조를 예시로 들면, app, pages, widgets, features, entities, shared 이러한 레이어를 만들어, 프로젝트의 폴더를 관리하기는 합니다만 이때의 widgets은 아래와 같은 느낌으로 사용되어집니다!
- widgets: layout의 내부 내용 변경될 때 (전체적인 틀 같은 것을 만드는 경우)
- entities : 컴포넌트 (데이터 그 자체)
- features : 데이터를 제거한 단순한, 모양의 컴포넌트
FSD 구조는 이런식으로, 구분해서 앱을 제작하는데 기존에 제가 알고 있는 widgets 이라는 기준과, moorad가 생각하시는 widgets이 조금 다른 것 같습니다!!
widgets이란 폴더는, 어떠한 것들을 담고 있는지 (기존, components 폴더랑은 어떻게 다른지) 알 수 있는 방법이 추가적으로 있으면 좋겠다고 생각했습니다!!!
하지만, 폴더 구조에는 정답이 없기에, 만약 확고한, 폴더 규칙에 대한 팀 컨벤션이 있다면, 이렇게 진행하셔도 좋아보입니다!! 정말 단순 저의 궁금증이기에, 이렇게 사용하는, 폴더구조에 관한 아티클이나, 문서같은게 있으시면 공유해주시면 감사하겠습니다!!
- 로그인 처리! (토큰 관리, 리프레시 토큰 관련 처리)
다른 분들과 다르게, 전역적으로 로그인을 관리하셨다는 점에서 정말 인상깊었습니다!! accessToken과 refreshToken 같은 경우는 개인정보가 담긴 만큼, 잘 관리를 해주어야 하는데요!
지금 제가 드린 API는, 정말 간략하게 제공을 했지만, 보통은 외부에서 접근할 수 없게 Http Only 쿠키로 제공을 한다거나, 리프레시 토큰 자체를, 프론트에 넘기지 않고, 서버에서 관리하고, accessToken 또한, 로컬스토리지나, 쿠키에 따로 저장하지 않고, 정말 리액트의 state -> in-memory 방식으로 처리하는 경우가 많습니다!!
in-memory 방식(state)으로, 관리할 떄, 사용자가, 새로고침을 하거나, 그런 경우 당연히, 토큰에 대한 정보가 날라가니, 다음 요청시에는 토큰 값이 undefined가 되어, 토큰에 대한 만료나, undefined인 경우, 로그인이 풀리면 유저 경험이 좋지 않아, refreshToken 관련 로직을 axiosInterceptor를 활용하여, 요청 전후에, 재발급 하는 api를 호출하여 처리하곤 합니다!! 충분히 이러한 부분에 대해 고민하시고 해결하실 수 있을 것이라고 생각하니 한번 조금 고민해보시고 처리해보시는 것도 좋아보입니다!!
위에 대한 고민을 해보시면서, useState의 콜백함수를 넘기는 경우에 대해서도 한번 고민해보셔도 좋아보입니다!! 추가적으로 useLayoutEffect 또한, 해당 고민 과정에서 잘 활용할 수 있기에 알아보셔도 좋을 것 같습니다!!
정말 고생 많으셨습니다!
There was a problem hiding this comment.
환경 변수, 같은 경우는 깃허브 상에 노출되지 않는 것이 바람직합니다! 혹시라도 실수로 올리셨다면, 올린 .env 파일 또한 git log에서, 완전히 제거하는 방법도 있다고 하니, 완전히 제거해주심이 바람직해보입니다!
| ]); | ||
|
|
||
| const Routing = () => { | ||
| return <RouterProvider router={router}></RouterProvider>; |
There was a problem hiding this comment.
내부에 Children이 없는 경우 아래와 같이 대체해서 사용하시기를 추천드립니다.
<RouterProvider router={router} />| ); | ||
| }; | ||
|
|
||
| const SignInContainer = styled.form` |
There was a problem hiding this comment.
/src/pages/SignIn.tsx를 만드시고, 해당 파일에, 페이지에 관한 내용과, 스타일링에 관한 모든 코드를, 하나의 파일에 다 담으셨는데 분리해보는 것도 좋아보입니다.
| {errors.email && ( | ||
| <ErrorMessage>{errors.email.message}</ErrorMessage> | ||
| )} | ||
| <input |
There was a problem hiding this comment.
내부에 children이 없는 경우 이런식으로 작성하는게 좋아보입니다!
|
|
||
| const SignUpContainer = styled.form` | ||
| width: 100%; | ||
| height: calc(100vh - 12vh); |
There was a problem hiding this comment.
vh의 경우, 모바일 주소 표시줄 영역에 의해서 100vh로 높이 값을 설정했다고 하더라도 화면이 잘리는 경우가 발생합니다.
주소 표시줄의 유무에 따라 뷰포트 높이가 가변적으로 변할 때를 고려해서 dvh를 사용하는 것도 고려해보면 좋을 것 같습니다.
| import { movieInstance } from "../instance/apiInstance"; | ||
| const lang = "ko-KR"; | ||
|
|
||
| const getMoviesData = async (apiAddress: string, pageParam: number) => { |
There was a problem hiding this comment.
아래와 같이, 작성할 경우, 무조건 파라미터를 전달 할 경우, 정확한, 위치에 값을 전달해야합니다.
const getMoviesData = async (apiAddress: string, pageParam: number) => {} 파라미터가 많아질 경우, 자리를 헷갈릴 수 도 있기에 저는 보통, 객체 안에다 값을 넣어줍니다. 이 경우에는, apiAddress, pageParam 이름만 동일하다면, 순서는 상관없어져서, 조금 더 실수를 방지할 수 있습니다!
const getMoviesData = async ({apiAddress, pageParam}: {apiAddress: string, pageParam: number}) => {} |
|
||
| const getMoviesData = async (apiAddress: string, pageParam: number) => { | ||
| const res = await movieInstance.get( | ||
| `${apiAddress}?language=${lang}&page=${pageParam}` |
There was a problem hiding this comment.
쿼리 파라미터 같은 경우 위와 같이 표현해주셔도 좋고, 아래와 같이 관리해보는 것도 좋아보입니다!!
export const getMovies = async ({
category,
language,
page,
region,
}: GetMovieRequest): Promise<GetMovieResponse> => {
const { data } = await axiosInstance.get<GetMovieResponse>(
`/movie/${category}`,
{
params: {
language,
page,
region,
},
}
);
return data;
};| ]; | ||
|
|
||
| // 카테고리 컴포넌트 최적화 | ||
| const Category = React.memo( |
There was a problem hiding this comment.
부모 컴포넌트가, 자주 리렌더링 되는 경우, React.memo를 통해, 최적화를 할 수 있을 수 도 있지만, 만약 그렇지 않고, 비교적 가벼운 역할을 한다면, 해당 코드를 제거해도 좋지 않을까 싶습니다!!
| import { z } from "zod"; | ||
| export const signInSchema = z | ||
| .object({ | ||
| email: z.coerce |
There was a problem hiding this comment.
z.coerce를 통해서, 문자열 값이 전달되지 않을 경우, 강제로 string 형태로, 형변환을 시키는 의도로 작성하신 것 같습니다!!
만약, 들어오는 email input 값이 string이 확정인 경우에는, 해당하는 코드를 제거해도 괜찮지 않을까 생각합니다!!
만약 입력 값이, string도 들어올 수 있고, number도 들어갈 수 있지만, 무조건 서버에 데이터를 전송할 시에, string으로 무조건 전달해야 한다면은, 이런식으로 형변환을 강제로 시키는 것도 좋아보입니다!!
|
FSD 구조에 대해 완벽히 이해하고 사용하지 않고 우선 도입해서 사용하다 보니 기존과는 다르게 사용한 것 같습니다 😂 어떤 의도를 갖고 한 점은 아닙니다 ㅎㅎ.. in - memory 방식으로 token 관리하는 레퍼런스를 혹시 볼 수 있을까요 ? 아직 감이 잘 안잡히네요 ㅠ.. |
No description provided.