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

[2주차] 윤영준 미션 제출합니다. #2

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

yyj0917
Copy link

@yyj0917 yyj0917 commented Sep 19, 2024

https://reacttodoyyj0917.vercel.app/
(github에서 바로 이동이 안 되어 주소 복사해서 이동해주시면 감사하겠습니다. 빠른 시일 내에 알아오겠습니다..)

😸 구현 기능

  • react-calendar hook custom
  • hashMap 구조의 Task 관리
  • ToolTip, Alert Custom

📝 느낀점
이번엔 과제를 함에 있어 어떤 점에 초점을 맞춰 구현할지 고민하였고, 이전과제와 같은 ToDoList이기 때문에 기능적인 부분보다 내가 만약에 ToDoList 웹/앱을 쓰면 어떤 형태의 투두리스트를 사용할지를 중심으로 구현하였습니다. Calendar형태로 하여 이번 달의 나의 기록을 확인하고, 간단하면서도 기능을 수행할 수 있는 UI에 더 심혈을 기울였습니다. 저의 UX적인 부분을 고려한 과제였기에 제가 추구하는 경험이 담겨있고, React를 사용하면서 코드적인 부분에서는 나의 코드가 다른 분들에게 어떻게 비춰질지를 고민했고, 추후 코드리뷰에 따른 수정을 계속 해나가는 것을 계획했습니다. 앞으로 사용가능한 hook, tool들이 늘어날텐데 그것에 맞춰 내가 구현할 것에 어떤 이점이 있을지, 왜 사용해야 할지 등 여러 생각을 동반하여 구현해나가야 더 의미있는 스터디, 과제가 될 수 있겠다고 생각하였습니다. 확실히 리액트가 편하긴 한 것 같습니다. 다음번 과제에는 여유가 있다면 더 많은 것을 끌여다 써보고 싶다는 생각을 했습니다.

✏️ Key Questions

  • Virtual-DOM은 무엇이고, 이를 사용함으로서 얻는 이점은 무엇인가요?

Virtual-Dom은 가상의 DOM 구조를 메모리 상에 저장하고, 이를 실제 DOM과 비교하여 모든 부분이 아닌 실제로 변경된 부분만 실제 DOM 상에 업데이트하여 렌더링 성능을 최적화하는 기술이다.

실제 DOM 조작은 비용이 많이 드는 작업으로 부분적인 변경사항으로 실제 DOM 전부를 업데이트하는 것은 비효율적이다. 그렇기 때문에 메모리에 (JavaScript 엔진으로) 가상의 DOM 트리를 생성하고, Diff 알고리즘으로 실제 DOM과 비교한 뒤 변경된 부분만 실제 DOM에 업데이트하여 효율을 높이는 데 이점이 있다.

예시로 React 라이브러리가 있다. 라이브러리 내부적으로 가상 DOM 트리 생성 로직, Diff 알고리즘 로직이 있다.
첫 렌더링 => React 컴포넌트 렌더링 -> 가상 DOM을 실제 DOM에 반영(처음엔 전부 업데이트) -> 브라우저 렌더링 엔진이 실제 DOM을 렌더링
이후 state, props 변경 시 => React 컴포넌트 리렌더링 -> 리렌더링된 컴포넌트를 가상 DOM에 반영 -> Diff 알고리즘으로 실제 DOM과 가상 DOM을 비교 -> 변경된 부분만 실제 DOM에 업데이트 후 브라우저가 다시 렌더링 --> 이 과정에서 렌더링 성능개선.

  • React.memo(), useMemo(), useCallback() 함수로 진행할 수 있는 리액트 렌더링 최적화에 대해 설명해주세요. 다른 방식이 있다면 이에 대한 소개도 좋습니다.

React.memo() -> 컴포넌트의 props가 변경되지 않으면 리렌더링이 되지 않게 방지한다.
const ChildComponent = React.memo(({ value }) => { console.log('ChildComponent re-rendered'); return <div>{value}</div>; });
value props가 변경되지 않으면 ChildComponent는 리렌더링되지 않는다.
주 사용 용도는 자식 컴포넌트가 부모 컴포넌트 상태 변화로 인해 자주 리렌더링될 때 사용. 주의할 점으로 props 비교 자체가 추가적인 비용이 들기 때문에 항상 성능 개선이 이루어지는 것은 아니며 렌더링 비용이 큰 자식 컴포넌트에 사용하면 효과적이다.

useMemo() -> 계산 비용이 큰 값의 메모제이션 역할을 한다. 연산 비용이 높은 함수의 결과값을 메모제이션하고, 특정 조건(* 의존성 배열 값 변경 *)이 충족되지 안 되었을 때 기존의 값을 재사용하여 불필요한 연산을 방지한다.

useCallback() -> 함수 재생성을 방지하여 컴포넌트가 불필요하게 리렌더링되는 것을 방지.
자바 스크립트에서 함수 = 객체. 새로운 함수가 선언될 때 새로운 객체가 생성. 그렇기 때문에 React에서도 리렌더링 때마다 새로운 함수가 생성되고, 불필요한 생성이 반복되면 비용이 커질 수 있음. useCallback() 또한, 의존성 배열로 재사용 여부를 조정함.
useCallback()에 해당하는 부모 컴포넌트의 함수는 처음 컴포넌트가 렌더링될 때 생성 -> 이후 메모리 상에 저장되고 참조되어 재사용이 가능해짐 -> 추후 컴포넌트가 업데이트될 때 의존성 배열 확인 후 값 변경이 없다면 메모리 상 참조하고 있는 함수를 재사용 -> 새로운 인스턴스 생성이 안 되므로 props로 함수를 받는 자식 컴포넌트에서 리렌더링이 발생하지 않음.
주의할 점 -> 과도한 useCallback 사용은 오히려 오버헤드를 초래할 수 있다. useCallback은 함수 인스턴스를 메모리에 캐싱해야 하기에 메모리 사용량이 증가하고, 의존성 배열 비교도 필요하기에 적절한 컴포넌트에 사용된 것이 아니라면 오히려 성능 저하를 유발할 수 있다.
주 사용 용도는 부모 컴포넌트에서 함수를 자식 컴포넌트에 props로 전달할 때, 함수의 생성비용이 클 때, 의존성 배열 관리가 용이한 함수일 때가 적절히 적합했을 때 사용하면 효율적이다.

이외의 렌더링 최적화 방안
Virtualization -> React Virtualized, React Window 등의 라이브러리같은 가상화 기법으로 화면에 보이는 데이터만 렌더링하고, 다른 데이터들은 필요할 때 추가하여 렌더링 성능을 높임
Lazy Loading -> 특정 컴포넌트를 필요로 할 때 로드하는 방식. 리액트의 단점으로 여겨지는 초기 로딩속도 개선에 효율적임. React.lazy, Suspense를 함께 사용하여 필요한 컴포넌트를 동적으로 가져옴. React.lazy()를 통해 동적으로 가져올 컴포넌트를 정하고, Suspense(*fallback)를 통해 React.lazy()에 해당하는 컴포넌트를 비동기적으로 로딩하는 동안 UI적으로 로딩화면을 비춰줌.
Code Splitting -> 모든 코드를 한번에 로드하지 않고, 필요한 부분만 나누어 로드하여 번들크기를 감소시킴
Webpack, Parcel 같은 모듈 번들러를 사용하여 코드 청크로 분리해서 필요한 부분만 로드되도록함.
상태 관리 라이브러리(Redux, useReducer, Recoil, Zustand etc...) -> 상태 관리 도구를 통해 불필요한 렌더링을 방지하고, 성능을 최적화함. 목적에 맞는 상태 관리 라이브러리를 활용하여 상태를 관리하고, 적절한 상태 업데이트로 불필요한 렌더링을 최소화함.

  • React 컴포넌트 생명주기에 대해서 설명해주세요.

크게 Mounting(생성), Updating(갱신), Unmounting(제거) 등 3가지로 나누어진다.
함수형 컴포넌트
Mounting -> 컴포넌트가 처음으로 DOM에 삽입될 때 실행되는과정
Updating -> 컴포넌트가 새로운 props를 받거나 state가 변경될 때 발생. 이때 컴포넌트는 다시 렌더링되어 그에 적합한 화면이 업데이트
Unmounting -> 컴포넌트가 DOM에서 제거될 때 발생

함수형 컴포넌트에서는 생명주기를 Hooks를 사용하여 처리
Mounting, Unmounting, Updating -> useEffect를 이용하여 처리
`// Mounting & Updating
useEffect(() => {
console.log('Component mounted or updated');

// Unmounting
return () => {
  console.log('Component will unmount');
};

}, [count]);`
useEffect(() => {...}): 두 번째 인자로 의존성 배열을 제공하지 않으면, 상태나 props가 변경될 때마다 실행됨. 여기서 컴포넌트의 상태를 갱신하거나, 외부 데이터를 가져올 수 있고, 의존성 배열을 특정 값으로 지정하면 해당 값이 변경될 때만 useEffect가 실행된다.

클래스형 컴포넌트
함수형 컴포넌트가 나오기 이전에는 클래스형 컴포넌트가 주로 사용되었고, 이는 생명주기를 관리하는 방법이 다르다. 현재는 함수형 컴포넌트와 Hook을 통해 관리하지만 클래스형 컴포넌트의 방법도 추후 다른 코드를 이해함에 있어서 중요하다.
주로 state와 생명주기 메서드(constructor, render, componentDidMount 등)을 사용한다. 클래스형 컴포넌트는 생명주기 메서드를 명시적으로 정의하기 때문에 각 생명주기 단계에서 실행해야 하는 작업이 많거나 명확하게 정의하는 것이 필요하다면 메서드 사용에 있어서 장점이 부각된다. 또한, 상속을 통해 확장이 가능한 점도 장점이 된다. 자세한 방식은 작성하지 않았지만 클래스형 컴포넌트의 생명주기 관리를 이해하고 가는 것이 이해함에 도움이 될 것이라 생각합니다.

😁 Key Question 느낀점
렌더링 최적화와 관련하여 가장 중요한 점은 목적과 방식이라는 생각이 들었습니다. React와 브라우저의 렌더링 과정을 고려했을 때 가장 비용이 큰 것은 브라우저의 렌더링. 그에 비해 React 내의 렌더링은 비용이 크지 않습니다. 이미 React 라이브러리 내에서 가상 DOM을 통한 브라우저 렌더링 최적화가 이루어지기 때문에 React 내의 컴포넌트 렌더링만 효율적으로 구현하면 됩니다. 렌더링의 최적화는 있을수록 좋지만, '단순히 최적화가 좋기 때문에'를 목적으로 과도한 사용, 불필요한 사용이 이루어진다면 역으로 코드의 복잡성 증가, 불필요한 리소스 낭비 등의 성능 저하가 유발될 수도 있다고 생각이 들었습니다. 그렇기에 복잡한 컴포넌트 트리, Props Drilling 같은 경우에는 React 내의 리렌더링 비용도 커지기 때문에 이러한 상황에서 적절한 곳에 'ex. 불필요한 함수 재생성 방지'와 같은 목적으로 어떠한 최적화 방안을 적용하여 렌더링 최적화를 하겠다라는 흐름과 목적을 이해하고 적용하는 것이 필요하다고 느꼈습니다.

Copy link

@hae2ni hae2ni left a comment

Choose a reason for hiding this comment

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

수고하셨습니다! 코드 보면서 많이 배웠어요!!

// 각 타일에 내용을 추가하는 함수
const tileContent = ({ date, view }) => {
if (view === 'month') { // 월간 뷰일 때만 표시
const { completed, total } = getTodoProgressForDate(date); // 성취도 계산
Copy link

Choose a reason for hiding this comment

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

구조분해할당 잘 쓰신 것 같습니다!

Comment on lines +43 to +44
if (date === new Date().toISOString().split('T')[0]) {
return 'today'; // 오늘 날짜를 하이라이트
Copy link

Choose a reason for hiding this comment

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

여기서 date는 date 객체이고, new Date().toISOString().split('T')[0] 는 문자열이라서, 아마 TS에서는 에러가 날 수도 있을 것 같다는 생각이 들었습니다! date도 문자열로 변경해서 타입을 맞춘 다음에, 비교를 하는 건 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

좋은 지적 감사합니다! js를 사용하더라도 ts에 대한 의식을 했어야 했는데 놓치고 있었던 것 같습니다. 깨닫게 해주셔서 감사합니다!

};

// 특정 조건에 따라 날짜에 클래스 이름을 할당하는 함수
const getTileClassName = ({ date, view }) => {
Copy link

Choose a reason for hiding this comment

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

요 함수 useCallback으로 묶어주어도 괜찮을 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

month 기준으로 타일이 세팅되는 구조라 말씀대로 useCallback으로 묶고, month를 의존성 배열값으로 주면 더 효율적일 수 있을 것 같아요! 감사합니다!

</Title>
<SelectMonth value={month} onChange={handleMonthChange}>
{Array.from({ length: 12 }, (_, i) => (
<option key={i} value={i}>
Copy link

Choose a reason for hiding this comment

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

key값을 index보다는 다른 더 유니크한 값으로 바꿔 주셔도 좋을 것 같아요!

Comment on lines 44 to 47
tasks.set(taskId, newTaskObj);
setNewTask('');
updateTasks(tasks);
};
Copy link

Choose a reason for hiding this comment

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

Suggested change
tasks.set(taskId, newTaskObj);
setNewTask('');
updateTasks(tasks);
};
//상단
const [task, setTask] =useState({})
...
setTasks(prevTasks => ({
...prevTasks,
[taskId] : newTaskObj
})));
setNewTask('')
...
};

tasks 상태를 직접 변경하는 것보다 새로운 객체나 배열을 생성해서 상태를 업데이트하는 게 오류 방지에 더 좋을 것 같습니다!

Copy link
Author

Choose a reason for hiding this comment

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

코드 가독성 측면에서 좀 줄여보자는 생각으로 구현했었는데 밑에 알려주신 링크와 조금 더 알아보니 새로 생성해서 업데이트하는 게 좋을 것 같습니다! 가독성, 에러방지, 효율 등에서 적절한 선을 찾아야겠네요 ㅎㅎ 감사합니다!

Comment on lines +14 to +15
loadedTasks.forEach(task => newTaskMap.set(task.id, task));

Copy link

Choose a reason for hiding this comment

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

Suggested change
loadedTasks.forEach(task => newTaskMap.set(task.id, task));
loadedTasks.map(task => [task.id, task]))

이렇게 하면 조금 더 간단해지지 않을까요..?

Copy link
Author

Choose a reason for hiding this comment

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

요렇게만 바꿨을 때 제 task의 타입이 Map이라서 Map에는 저장이 안 될 것 같아서
피드백을 참고하여 new Map(loadedTasks.map(task => [task.id, task])); 요렇게 바꿔보았습니다!


tasks.set(taskId, updatedTask); // 기존 Map 수정

setTasks(new Map(tasks)); // 새로운 Map을 생성하지 않고 기존 Map을 수정한 후 상태를 업데이트
Copy link

Choose a reason for hiding this comment

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

자바스크립트 객체 타입의 불변성을 유지하기에는 새로운 map을 생성하는게 더 바람직하다고 하네요,,!

Copy link

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

저도 혜인 님과 같은 생각입니다!💡 Map 자체가 참조형 데이터이기 때문에 새로운 map을 생성해서 원래 tasks의 상태를 변경하지 않도록 하는 것이 더 안전할 것 같아용!

Comment on lines +44 to +56
{/* 작업이 없을 때 메시지 출력 */}
{sortedCompletedTasks.length === 0 && (
<p style={{'margin': 'auto', 'fontSize': '1.2rem', 'color': 'gray'}}>
Complete your tasks
</p>
)}
{/* 작업 리스트 출력 */}
{sortedCompletedTasks.map((task) => (
<TaskItem
key={task.id}
className={task.completed ? 'completed' : ''}
>
<p className='task-text'>
Copy link

Choose a reason for hiding this comment

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

조건부 렌더링으로 바꾸어보았습니당

Suggested change
{/* 작업이 없을 때 메시지 출력 */}
{sortedCompletedTasks.length === 0 && (
<p style={{'margin': 'auto', 'fontSize': '1.2rem', 'color': 'gray'}}>
Complete your tasks
</p>
)}
{/* 작업 리스트 출력 */}
{sortedCompletedTasks.map((task) => (
<TaskItem
key={task.id}
className={task.completed ? 'completed' : ''}
>
<p className='task-text'>
{/* 작업이 없을 때 메시지 출력 */}
{sortedCompletedTasks.length === 0 ? (
<p style={{'margin': 'auto', 'fontSize': '1.2rem', 'color': 'gray'}}>
Complete your tasks
</p>
)}
: (
{sortedCompletedTasks.map((task) => (
<TaskItem
key={task.id}
className={task.completed ? 'completed' : ''}
>
<p className='task-text'>
Suggested change
{/* 작업이 없을 때 메시지 출력 */}
{sortedCompletedTasks.length === 0 && (
<p style={{'margin': 'auto', 'fontSize': '1.2rem', 'color': 'gray'}}>
Complete your tasks
</p>
)}
{/* 작업 리스트 출력 */}
{sortedCompletedTasks.map((task) => (
<TaskItem
key={task.id}
className={task.completed ? 'completed' : ''}
>
<p className='task-text'>
{/* 작업이 없을 때 메시지 출력 */}
{sortedCompletedTasks.length === 0 && (
<p style={{'margin': 'auto', 'fontSize': '1.2rem', 'color': 'gray'}}>
Complete your tasks
</p>
)}
{/* 작업 리스트 출력 */}
{sortedCompletedTasks.map((task) => (
<TaskItem
key={task.id}
className={task.completed ? 'completed' : ''}
>
<p className='task-text'>

Copy link
Author

Choose a reason for hiding this comment

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

감사합니다! 조건부가 더 좋은 것 같아요


return (
<TaskItemsContainer>
{sortedPendingTasks.length === 0 && <p style={{'margin': 'auto', 'fontSize': '1.2rem', 'color': 'gray'}}>Add something to do</p>}
Copy link

Choose a reason for hiding this comment

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

요 부분은 styled-component를 이용해도 좋을 것 같아요!!

< AddSthTodo $sortedPendingTasks={sortedPendingTasks.length} >Add something to do </AddSthTodo/>


...

const AddSthTodo = styled.p`
${($sortedPedingTask) => $sortedPendingTask === 0 && return ~~

{sortedPendingTasks.map((task) => (
<TaskItem
key={task.id}
className={task.completed ? 'completed' : ''}
Copy link

Choose a reason for hiding this comment

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

className 진짜 깔쌈하게 잘 쓰시느ㄴ 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

감사합니다 네이밍 연구중입니다!

Copy link

@billy0904 billy0904 left a comment

Choose a reason for hiding this comment

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

과제에서 요구하는 사항 이외에도 다양한 기능을 구현해주셨는데도 전반적으로 복잡하지 않고 기능에 따라 컴포넌트를 깔끔하고 효율적으로 분리하셔서 코드 리뷰하기 편했던 것 같습니다! 👍✨ 2주차 과제 하시느라 정말 수고 많으셨습니다! 덕분에 많이 배워갑니다🍀

(+) 저도 전에 마크다운으로 하이퍼링크 넣을 때 영준 님과 비슷한 오류가 생겼었는데 "[] (url)" 이 부분에 대괄호 부분 말고 url 부분에 링크를 넣어서 해결한 적이 있습니다...! 혹시 같은 상황이면 참고하실 수 있을 것 같아 말씀드립니다!☺️

Comment on lines +45 to +46
} else if (completed === total && completed > 0 && total > 0) {
return 'great'; // 모든 할 일이 완료된 경우

Choose a reason for hiding this comment

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

지금 조건으로도 모든 할 일이 완료된 경우에 대한 if문이 충분히 잘 기능하지만 조건을 조금 더 간결하게 completed === total && total > 0로도 작성할 수 있을 것 같아요!

Suggested change
} else if (completed === total && completed > 0 && total > 0) {
return 'great'; // 모든 할 일이 완료된 경우
} else if (completed === total && total > 0) {
return 'great'; // 모든 할 일이 완료된 경우

Comment on lines +103 to +121
{/* Pending Tasks 컨텐츠 */}
{activeIndex === 0 && (
<PendingTask
tasks={tasks}
setTasks={setTasks}
handleDeleteTask={handleDeleteTask}
selectedDate={selectedDate}
/>
)}

{/* Completed Tasks 컨텐츠 */}
{activeIndex === 1 && (
<CompletedTask
tasks={tasks}
setTasks={setTasks}
handleDeleteTask={handleDeleteTask}
selectedDate={selectedDate}
/>
)}

Choose a reason for hiding this comment

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

pending task와 completed task를 분리해서 보여주는 방식과 UI가 마음에 드네요! 제가 실사용자라면 편리하게 사용할 수 있을 것 같아요😃👍

Comment on lines +1 to +26
import React from 'react';
import styled from 'styled-components';
import PropTypes from 'prop-types';

const TooltipContainer = styled.div`
position: absolute;
background-color: rgba(12, 0, 0, 0.75);
color: #fff;
padding: 5px 10px;
border-radius: 5px;
font-size: 10px;
top: -30px; /* 조정 필요 */
left: 50%; /* 조정 필요 */
transform: translateX(-50%);
white-space: nowrap;
z-index: 1000;
`;
function Tooltip({ text }) {
return <TooltipContainer>{text}</TooltipContainer>;
}

// PropType 정의
Tooltip.propTypes = {
text: PropTypes.string.isRequired,
};
export default Tooltip;

Choose a reason for hiding this comment

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

툴팁 기능을 사용하셔서 UX도 편리해보이고 무엇보다 결과물이 훨씬 완성도있어보이는 것 같아요! 저도 다음 과제에서는 마우스 호버링 툴팁을 사용해봐야겠어요😃


tasks.set(taskId, updatedTask); // 기존 Map 수정

setTasks(new Map(tasks)); // 새로운 Map을 생성하지 않고 기존 Map을 수정한 후 상태를 업데이트

Choose a reason for hiding this comment

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

저도 혜인 님과 같은 생각입니다!💡 Map 자체가 참조형 데이터이기 때문에 새로운 map을 생성해서 원래 tasks의 상태를 변경하지 않도록 하는 것이 더 안전할 것 같아용!

Comment on lines +58 to +60
@media (max-width: 768px) {
font-size: 12px;
}

Choose a reason for hiding this comment

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

미디어 쿼리를 적용하셔서 모바일에서도 반응형으로 동작하는 점이 좋은 것 같아요!

Comment on lines +59 to +67
.today {
background-color: #FF8200;
}
.great {
background-color: #32AAFF;
}
.sorry {
background-color: #FF5A5A;
}

Choose a reason for hiding this comment

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

지금 코드도 충분히 직관적이고 간결하지만 ColorBox 내부의 클래스들의 색상 값과 텍스트를 상수로 분리하면 유지 보수성을 조금 더 높일 수 있을 것 같아요!

Suggested change
.today {
background-color: #FF8200;
}
.great {
background-color: #32AAFF;
}
.sorry {
background-color: #FF5A5A;
}
const colorState = {
today: '#FF8200',
great: '#32AAFF',
sorry: '#FF5A5A',
};

Copy link
Collaborator

@jinnyleeis jinnyleeis left a comment

Choose a reason for hiding this comment

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

영준님 코드 보면서 고민하신 흔적들이 느껴졌어요!!
성취도 확인 눌렀을 때, 오늘 할 일이 없어요! 모달보고 깜짝 놀랐어요 ㅎㅎ 모달에 애니메이션까지 적용되어있고, 여러 ux적 상황을 고려하시면서 프로젝트를 설계하신것같아 정말 디테일하다는 생각이 들었습니다!!!
전반적으로 깔끔하고 가독성이 좋은 코드를 보며 이해하기 편했던 것 같습니다.
특히 파일 이름이나, 네이밍이 명확해서 좋은 것 같아요!!
다른 분들이 코드리뷰 잘 남겨주셔서 하나 참고하시면 좋을 사항 리뷰로 남겨두었어요!!!

이번 과제도 정말 수고 많으셨습니다!!

const formattedDate = selectedDate.toISOString().split('T')[0];


const handleDeleteTask = (taskId) => {
if (window.confirm('정말로 삭제하시겠습니까?')) { // 삭제 확인
if (tasks.has(taskId)) {
tasks.delete(taskId); // 해당 task 삭제
Copy link
Collaborator

Choose a reason for hiding this comment

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

Map을 사용하니, 배열을 사용할 때처럼 find()등 배열 메서드로 하나씩 체크하지 않아도,
taskId로 바로 접근하니 코드가 간결하고 시간 복잡도 측면에서도 좋은 것 같아요!!
코드를 간결하고 효율적으로 작성하시려는 노력이 보이는 것 같아 많이 배워갑니다!!!

@@ -14,7 +14,8 @@ export default function CompletedTask({ tasks, setTasks, handleDeleteTask, selec
loadedTasks.forEach(task => newTaskMap.set(task.id, task));

setTasks(newTaskMap);
}, []);
}, [selectedDate, setTasks]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

의존성 배열에 setState 함수를 추가하셨는데, setState 함수는 컴포넌트가 리렌더링되더라도 동일한 함수가 계속해서 사용되기 때문에, 의존성 배열에 넣을 필요가 없다고 합니다!!
setState를 의존성 배열에 추가하더라도 useEffect가 불필요하게 실행되지는 않기 떄문에, 성능에 문제를 일으키지는 않지만, setState가 의존성 배열에 들어가 있지 않아도, 해당 useEffect가 올바르게 동작할 것이기 때문에, setTasks를 의존성 배열에서 제거하셔도 무방할 것 같습니다!

이와 관련해서 최근 버전의 공식문서에는
'The set function has a stable identity, so you will often see it omitted from effect dependencies, but including it will not cause the effect to fire. If the linter lets you omit a dependency without errors, it is safe to do. '라고,
https://ko.react.dev/reference/react/useState
리엑트 18버전의 공식문서에는
'React guarantees that setState function identity is stable and won’t change on re-renders. This is why it’s safe to omit from the useEffect or useCallback dependency list.'라고 언급되어있네요!! 한번 확인해보시면 좋을 것 같아요!!

https://legacy.reactjs.org/docs/hooks-reference.html#usestate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants