6조 과제제출 (이은비, 주하림, 박현준) #3
Conversation
Choi yong jun/mypage list
| export const loginRequest = async (params: ILoginData): Promise<IDataResponse<ILoginUser>> => { | ||
| const response = await client.post('/login', params) | ||
| return response.data | ||
| } | ||
|
|
There was a problem hiding this comment.
params를 사용하여 서버로부터 받은 응답 데이터를 반환하고, 응답 데이터 객체의 프로미스를 사용한 부분이 인상깊었습니다!
| export const client = axios.create({ | ||
| baseURL: host, | ||
| headers: { | ||
| 'Content-Type': 'application/json' | ||
| } | ||
| }) |
There was a problem hiding this comment.
apis 폴더 안에 api.ts를 생성하지 않고 axios.ts를 따로 만드신 특별한 이유가 궁금합니다!
There was a problem hiding this comment.
axios.ts는 axios config를 설정하는 코드가 있는 부분이라 파일명을 axios라고 지었습니다!
| @@ -0,0 +1,47 @@ | |||
| import { Skeleton, SkeletonProps, Table } from 'antd' | |||
There was a problem hiding this comment.
'antd' 라이브러리를 사용하여 스켈레톤 로딩 상태에 따라 스켈레톤 테이블 또는 실제 데이터를 표시한 부분이 인상깊었습니다. 'antd' 라이브러리를 사용하면서 편리했던 점과 불편했던 점이 궁금합니다.
There was a problem hiding this comment.
아무래도 기본적인 컴포넌트의 디자인과 기능이 전부 구현이 되어 있어서 컴포넌트에 대한 기능 구현이 최소화 된 부분이 가장 편리했던 것 같습니다! 불편했던 점은 기본적인 스타일링이 되어 있기 때문에 프로젝트에 맞게 커스텀 하는 과정들과 style 코드를 추가적으로 작성해줘야 하는 부분들이 조금 불편했던 것 같습니다!
| type NotificationPopupProps = { | ||
| datas: INotificationData[] | ||
| } | ||
| export const NotificationPopup = React.memo(({ datas }: NotificationPopupProps) => { |
There was a problem hiding this comment.
최적화를 위해 React.memo를 사용한 부분이 인상깊었습니다.
| export * from 'components/common/NavLayout' | ||
| export * from 'components/common/AppNav' | ||
| export * from 'components/common/NotificationPopup' | ||
| export * from 'components/common/SkeletonTable' |
There was a problem hiding this comment.
calendar, common, dayoff 등 index.ts 파일을 생성하여 export * from 구문을 사용하여 해당 모듈의 모든 내용을 내보낸 특별한 이유가 궁금합니다!
There was a problem hiding this comment.
Named exports를 사용하면 한 파일에서 여러 값을 export하는데 좋습니다! 또한 같은 계층에 있는 모듈을 import 할 때 한줄로 import가 가능합니다. Export default의 경우 import할 모든 모듈을 명시해야하기 때문에 import 구문이 길어지기도 하구요.. 그래서 각 폴더의 루트에 있는 index.ts를 사용하면 하위 파일에서 내보내고 있는 모듈들을 한번에 내보낼 수 있어서 좋다고 생각해서 이런 방식으로 진행하게 되었습니다!
| const [loginData, setLoginData] = useState<ILoginData>({ email: '', password: '' }) | ||
| const [emailError, setEmailError] = useState<string>('') | ||
| const navigate = useNavigate() | ||
| // 로그인한 유저 정보 전역 State | ||
| const { setUserInfo } = useUserStore() | ||
| const { openModal } = modalStore() | ||
|
|
||
| const handleChange = (event: React.ChangeEvent<HTMLInputElement>) => { | ||
| const { name, value } = event.target | ||
| setLoginData({ | ||
| ...loginData, | ||
| [name]: value | ||
| }) | ||
| } |
There was a problem hiding this comment.
전역 상태 관리를 위한 useUserSotore 훅과 modalStore 훅을 가져오고, 로그인 데이터의 해당 필드만 업데이트하여 기존 데이터와 병함한 부분이 인상깊었습니다!
| useEffect(() => { | ||
| console.log(signUpData) | ||
| }, [signUpData]) | ||
|
|
||
| const handleChange = (event: React.ChangeEvent<HTMLInputElement>) => { | ||
| const { name, value } = event.target | ||
| setSignUpData({ | ||
| ...signUpData, | ||
| [name]: value | ||
| }) | ||
| } | ||
|
|
||
| const handleDatePickerChange = (date: Dayjs | null, dateString: string) => { | ||
| setDate(date) | ||
| setSignUpData({ | ||
| ...signUpData, | ||
| joinDate: date ? dateString : '' | ||
| }) | ||
| } | ||
|
|
||
| const handlePositionChange = (value: string) => { | ||
| setSignUpData({ | ||
| ...signUpData, | ||
| position: value | ||
| }) | ||
| } | ||
|
|
||
| const handlePasswordConfirmChange = (event: React.ChangeEvent<HTMLInputElement>) => { | ||
| const { value } = event.target | ||
| setPasswordConfirm(value) | ||
| } | ||
|
|
||
| const handleSubmit = async () => { | ||
| //이메일 유효성검사 | ||
| const emailRegex = /^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}$/ | ||
| if (!emailRegex.test(signUpData.email)) { | ||
| setEmailError('유효한 이메일 주소를 입력하세요.') | ||
| return | ||
| } | ||
| //비밀번호확인 유효성검사 | ||
| if (signUpData.password === passwordConfirm) { | ||
| console.log(signUpData) | ||
| signUpRequest(signUpData).then( | ||
| res => { | ||
| console.log('API 호출 성공!') | ||
| console.log(res) | ||
| openModal({ | ||
| ...resultModalDatas.SIGNUP_SUCCESS, | ||
| okCallback: () => { | ||
| navigate('/login') | ||
| } | ||
| }) | ||
| }, | ||
| error => { | ||
| console.log(error) | ||
| openModal({ | ||
| ...resultModalDatas.SIGNUP_FAILURE, | ||
| content: error.message || resultModalDatas.SIGNUP_FAILURE.content | ||
| }) | ||
| console.error('API 호출 실패!') | ||
| console.error(error) | ||
| } | ||
| ) | ||
| } else { | ||
| console.log('비밀번호 확인 실패!') | ||
| setPasswordMismatch(true) | ||
| } | ||
| } |
There was a problem hiding this comment.
각 부분의 변경 시 핸들러 함수를 정의한 부분이 가장 인상깊었고, useEffect를 사용하여 signupData가 변경될 때마다 로그를 출력하도록 하셨는데 이 부분 잘하셨습니다.
There was a problem hiding this comment.
총평
프로젝트의 폴더 구조가 아주 좋습니다.
많은 수강생 분들이 협업시에 자신의 스코프에만 집중해서 코드가 제각각이고 불필요한 재사용이 많은데 6조 분들의 과제는 그런것이 전혀 없습니다.
따라서 특별히 아쉬운 점은 없습니다.
6조 분들의 리뷰는 크게 아쉬운 점이 없어서 괜히 딴지 걸었다 생각하고 맘 편히 보세요.
마무리...
이번 과정에서 제가 여러분께 드리는 마지막 코드리뷰고, 여러분이 받는 마지막 코드리뷰겠네요.
제가 한 모든 코드리뷰는 한 사람의 의견으로 듣되, 왜 이런 의견이 나왔는지 고심해 보시길 바랍니다.
짧은 시간 동안 제가 여러분들의 사수 역할을 조금이나마 해냈다면 좋겠네요.
마지막 프로젝트는 아니지만 그 동안 고생하셨고, 여러분이 노력하신 만큼 결과가 뒤따라오길 기원하겠습니다.
다음에 뵐때는 멘토와 수강생이 아니라 개발자와 개발자로 뵙겠습니다.
수고하셨습니다.
| <Option value="0">사원</Option> | ||
| <Option value="1">주임</Option> | ||
| <Option value="2">대리</Option> | ||
| <Option value="3">과장</Option> | ||
| <Option value="4">차장</Option> | ||
| <Option value="5">부장</Option> |
There was a problem hiding this comment.
하드 코딩하지 않고 반복문을 사용하는게 더 좋을 것 같습니다.
| switch (position) { | ||
| case 0: | ||
| return '사원' | ||
| case 1: | ||
| return '주임' | ||
| case 2: | ||
| return '대리' | ||
| case 3: | ||
| return '과장' | ||
| case 4: | ||
| return '차장' | ||
| case 5: | ||
| return '부장' | ||
| default: | ||
| return '' | ||
| } |
There was a problem hiding this comment.
- object로 처리하면 굳이 switch문을 사용하지 않아도 됩니다.
| switch (position) { | |
| case 0: | |
| return '사원' | |
| case 1: | |
| return '주임' | |
| case 2: | |
| return '대리' | |
| case 3: | |
| return '과장' | |
| case 4: | |
| return '차장' | |
| case 5: | |
| return '부장' | |
| default: | |
| return '' | |
| } | |
| { | |
| '0': '사원', | |
| '1': '주임', | |
| '2': '대리', | |
| '3': '과장', | |
| '4': '차장', | |
| '5': '부장', | |
| } |
There was a problem hiding this comment.
그리고 position의 존재 자체가 애매합니다.
굳이 position을 숫자로 처리하지 않고 그냥 '사원' 같이 표현하면 이렇게 parsing할 필요도 없고 바로 사용할 수 있는데다가 position이 3이라고 하면 무슨 직급인지 이해가 가지 않지만 position이 '과장'이라고 하면 이해도 쉽습니다.
| const dateCellRender = (value: Dayjs) => { | ||
| const listData = | ||
| filteredSchedules.find( | ||
| schedule => dayjs(schedule.date).format('YYYY-MM-DD') === value.format('YYYY-MM-DD') | ||
| )?.schedules ?? [] | ||
| return ( | ||
| <EventUl> | ||
| {listData.map((item, index) => ( | ||
| <li key={index}> | ||
| <Tag color={item.color as TagProps['color']}> | ||
| {CALENDER_TYPE[item.type]} {item.username} {USER_POSITION[item.position]} | ||
| </Tag> | ||
| </li> | ||
| ))} | ||
| </EventUl> | ||
| ) | ||
| } |
There was a problem hiding this comment.
컴포넌트 안에서는 컴포넌트를 선언하면 안됩니다.
참고: https://react.dev/learn/your-first-component#nesting-and-organizing-components
| return ( | ||
| <StyledCalendar | ||
| style={{ padding: 20, borderRadius: 6 }} | ||
| mode={'month'} |
| import { Card, Badge } from 'antd' | ||
| import { styled } from 'styled-components' | ||
|
|
||
| export const ScheduleCard = React.memo(({ schedule }: { schedule: ICalendarSchedule }) => { |
There was a problem hiding this comment.
React.memo, useCallback, useMemo 모두 좋은 메모이제이션입니다.
하지만 메모이제이션 기법을 사용할 때도 생각해야 할 부분이 있습니다.
메모이제이션의 장점은 불필요한 렌더링을 줄이고 이를 통해 리렌더링의 횟수를 줄이는 것입니다.
이때 모든 메모이제이션은 추가적인 코드를 사용하기 때문에 실제 코드가 증가해 번들의 사이즈가 증가합니다.
또한, 메모이제이션 된 결과는 메모리상에 위치하므로 추가적인 메모리도 할당받습니다.
따라서 메모이제이션의 결과값이 작고 리렌더링이 어차피 자주 일어나는 경우 메모이제이션의 이점은 없고 단점만 부각될 수 있습니다.
따라서, 메모이제이션은 습관적으로 사용하지 않고 리렌더링이 불필요하게 많이 일어나거나 리렌더링시 과도한 메모리가 사용되는 경우에만 적용하는 것이 좋습니다.
| export const DayOffRequestModal = React.memo( | ||
| ({ availableDays, isModalOpen, onClickOk, onClickCancel }: TDayOffRequestModalProps) => { |
There was a problem hiding this comment.
지금 이 컴포넌트에서 React.memo에 대한 이야기를 좀 더 이어나가 보겠습니다.
DayOffRequestModal 사용되는 컴포넌트는 DayOff 컴포넌트입니다.
해당 컴포넌트에서 state는 아래와 같이 사용합니다.(4개)
const [isModalOpen, setIsModalOpen] = useState(false)
const [dayOffList, setDayOffList] = useState<IDayOffResponse[]>([])
const [isLoading, setIsLoading] = useState(false)
const [numOfDayOff, setNumOfDayOff] = useState<number>(0)
또, DayOffRequestModal이 받는 props는 아래와 같습니다.
availableDays={numOfAvailableRequestDays}
isModalOpen={isModalOpen}
onClickOk={handleOk}
onClickCancel={handleCancel}
단순 state를 isModalOpen이 사용하고 있고 numOfAvailableRequestDays라는 함수는 아래와 같습니다.
const numOfAvailableRequestDays = useMemo(
() => numOfDayOff - calcNumOfRequest(requestList) - numOfUsedDays,
[numOfDayOff, requestList, numOfUsedDays]
)
위 함수는 numOfDayOff라는 state와 requestList, numOfUsedDays에 따라 바뀝니다.
requestList, numOfUsedDays는 아래와 같습니다.
const requestList = useMemo(() => getFilteredDayOffRequestList(dayOffList), [dayOffList])
const historyList = useMemo(() => getFilteredDayOffHistoryList(dayOffList), [dayOffList])
const numOfUsedDays = useMemo(() => calcNumOfUsedDayOff(historyList), [historyList])
requestList는 dayOffList라는 스테이트를 받고 numOfUsedDays -> dayOffList를 받습니다.
따라서 DayOffRequestModal은 isModalOpen, numOfDayOff, dayOffList에 따라 리렌더링 됩니다.
그리고 결정적으로...handleOk 같은 일반 함수는 부모 컴포넌트의 리렌더링 시마다 새로운 함수 참조를 가지므로 부모 컴포넌트의 리렌더링시에 무조건 DayOffRequestModal이 리렌더링 되게 됩니다.
There was a problem hiding this comment.
메모이제이션이 잘못 되었고 지양해야 한다는 것은 아닙니다!
하지만, 메모이제이션을 습관처럼 사용하는 것은 좋지 않습니다.
메모이제이션은 꼭 필요한 상황에서만 사용하는 것을 목표로 사용해보세요!
| alert('로그인 세션이 만료되었습니다. 다시 로그인해주세요.') | ||
| location.replace('/login') | ||
| logout() |
There was a problem hiding this comment.
alert 이전에 logout 처리를 해야 할 것 같습니다.
alert는 script를 block 합니다.
| export const colorOfType = (type: number) => { | ||
| switch (type) { | ||
| case 0: | ||
| return 'geekblue' | ||
| case 1: | ||
| return 'cyan' | ||
| case 2: | ||
| return 'green' | ||
| default: | ||
| return 'orange' | ||
| } | ||
| } |
There was a problem hiding this comment.
object로 처리할 수도 있겠습니다.(default는 else로...)
| export const colorOfType = (type: number) => { | |
| switch (type) { | |
| case 0: | |
| return 'geekblue' | |
| case 1: | |
| return 'cyan' | |
| case 2: | |
| return 'green' | |
| default: | |
| return 'orange' | |
| } | |
| } | |
| export const colorOfType = { | |
| '0': 'geekblue', | |
| '1': 'cyan', | |
| '2': 'green', | |
| } |
There was a problem hiding this comment.
혹은 그냥 type을 색으로 지정해두면 굳이 parsing하지 않을 수 있습니다.
| case 0: | ||
| if (dayjs().startOf('date').diff(data.startDate) < 0) { | ||
| filteredList.push(data) | ||
| } | ||
| break | ||
| case 1: | ||
| if ( | ||
| dayjs().startOf('date').diff(data.startDate) < 0 || | ||
| (dayjs().startOf('date').diff(data.startDate) === 0 && dayjs().get('hour') < 9) | ||
| ) { | ||
| filteredList.push(data) | ||
| } | ||
| break | ||
| case 2: | ||
| if ( | ||
| dayjs().startOf('date').diff(data.startDate) < 0 || | ||
| (dayjs().startOf('date').diff(data.startDate) === 0 && dayjs().get('hour') < 14) | ||
| ) { | ||
| filteredList.push(data) | ||
| } |
There was a problem hiding this comment.
if 문 안의 조건문이 반복되는군요.
변수로 처리하면 반복을 줄일 수 있겠습니다.
| dayoffList.map(dayoff => { | ||
| if (dayoff.startDate !== dayoff.endDate) { | ||
| let date = dayjs(dayoff.startDate) | ||
| const diffDays = dayjs(dayoff.endDate).diff(date, 'day') | ||
| for (let i = 0; i <= diffDays; i++) { | ||
| if (date.get('day') !== 0 && date.get('day') !== 6) { | ||
| dayOffSchedules.push({ | ||
| ...dayoff, | ||
| color: colorOfType(dayoff.type), | ||
| startDate: date.format('YYYY-MM-DD') | ||
| }) | ||
| } | ||
| date = date.add(1, 'day') | ||
| } | ||
| } else { | ||
| dayOffSchedules.push({ ...dayoff, color: colorOfType(dayoff.type) }) | ||
| } | ||
| }) |
There was a problem hiding this comment.
map을 돌리면서 변형된 데이터를 dayOffSchedules에 넣어주고 있습니다.
다음과 같은 문제가 있습니다.
- map은 배열을 리턴하는 고차함수입니다. 하지만 해당 map의 리턴값을 전혀 사용하지 않고 있습니다. 이 경우엔 map이 아니라 forEach를 사용하는 것이 더 적절한 선택입니다.
- map은 새로운 배열을 리턴하므로 map의 리턴을 바로 사용할 수 있습니다. 즉, dayOffSchedules을 빈 배열로 생성 -> dayoffList 순회 로직 -> dayOffSchedules 리턴의 과정을 dayoffList 순회 로직 및 리턴 으로 줄일 수 있습니다.
There was a problem hiding this comment.
감사합니다. 멘토님!
코멘트로 달아주신 부분들 꼼꼼하게 다시 보면서 리팩토링 해보도록 하겠습니다.
자세한 코드리뷰 감사드립니다. 비록 짧은 시간이지만 그동안 많은 조언과 격려 해주셔서 감사합니다 :)
온앤오프 (OnOff)
📋 프로젝트 소개
개발 기간 : 2023. 07. 24 ~ 2023. 08. 10
배포 링크 : 온앤오프
Github : 프론트엔드, 백엔드
👥 개발 팀원 및 역할
관리자 휴가/당직 관리 페이지
관리자 사원 관리 페이지
Navigation 메뉴바
회원가입 페이지
마이 페이지
일정 페이지
휴가 페이지
당직 페이지
엑셀 다운로드
휴가/당직 처리 알림 기능
⚒️ 기술 스택 및 개발 환경
Development
Config
Deployment
Environment
Cowork Tools
📌 프로젝트 기능 소개
API 설계
알림 기능
로그인 / 회원가입 Page
로그인 Page
회원가입 Page
내 정보 Page
홈 - 일정 Page
휴가/당직 신청 조회
휴가 Page
휴가 조회
휴가 신청
당직 Page
당직 조회
당직 신청
관리자
휴가/당직 관리 Page
휴가/당직 신청현황 조회
휴가/당직 승인/반려
사원 관리 Page
사원 조회
사원 정보 수정
📂 프로젝트 구조
접기/펼치기
📦src
┣ 📂apis
┃ ┣ 📜axios.ts
┃ ┗ 📜index.ts
┃
┣ 📂components
┃ ┣ 📂admin
┃ ┃ ┗ 📜index.ts
┃ ┣ 📂calendar
┃ ┃ ┗ 📜index.ts
┃ ┣ 📂common
┃ ┃ ┗ 📜index.ts
┃ ┣ 📂dayoff
┃ ┃ ┗ 📜index.ts
┃ ┣ 📂duty
┃ ┃ ┗ 📜index.ts
┃ ┣ 📂login
┃ ┃ ┗ 📜index.ts
┃ ┣ 📂mypage
┃ ┃ ┗ 📜index.ts
┃ ┣ 📂signup
┃ ┃ ┗ 📜index.ts
┃ ┗ 📜index.ts
┣ 📂constants
┃ ┗ 📜index.ts
┣ 📂hooks
┃ ┗ 📜index.ts
┣ 📂pages
┃ ┣ 📜AdminEmployee.tsx
┃ ┣ 📜AdminSchedule.tsx
┃ ┣ 📜App.tsx
┃ ┣ 📜DayOff.tsx
┃ ┣ 📜Duty.tsx
┃ ┣ 📜HomeCalendar.tsx
┃ ┣ 📜Login.tsx
┃ ┣ 📜MyPage.tsx
┃ ┣ 📜SignUp.tsx
┃ ┗ 📜index.ts
┣ 📂stores
┃ ┗ 📜index.ts
┣ 📂types
┃ ┣ 📂admin
┃ ┃ ┗ 📜index.ts
┃ ┣ 📂calendar
┃ ┃ ┗ 📜index.ts
┃ ┣ 📂common
┃ ┃ ┗ 📜index.ts
┃ ┣ 📂dayoff
┃ ┃ ┗ 📜index.ts
┃ ┣ 📂duty
┃ ┃ ┗ 📜index.ts
┃ ┣ 📜IUser.ts
┃ ┗ 📜index.ts
┣ 📂utils
┃ ┗ 📜index.ts
┣ 📜GlobalStyle.ts
┣ 📜GlobalThemeConfig.ts
┣ 📜firebase.ts
┣ 📜main.tsx
┗ 📜vite-env.d.ts
🖼️ 결과물
|