[3주차] 주효정 미션 제출합니다. #15

jhj2713 · 2022-03-31T19:20:22Z

안녕하세요. 프론트 15기 주효정입니다! 지난주차랑 이어지는 미션이라 시간이 오래 안 걸릴 줄 알았는데 typescript로 변환하면서 생각보다 시간이 많이 걸리더라구요. Context api도 너무 오랜만에 사용해서 굉장히 배울 점이 많은 주차였습니다:joy:

배포 링크

https://react-todo-15th-jhj2713.vercel.app/

신경 쓴 부분

any type 없애기
자꾸 오류가 떠서 any를 쓸까말까 한참을 고민했는데 최대한 안 쓰려고 노력했습니다.

감사합니다!

sebastianrcnt

효정님 안녕하세요, 프론트엔드 멘토 정시원입니다.

꼼꼼하게 모든 객체를 타이핑해주신 점 정말 좋은 것 같아요. reducer 패턴과 custom hook를 이용해서 state의 안정성과 관심사의 분리를 달성하려고 하신 점이 특히 인상깊습니다.

말씀드린 제안 사항 한번 살펴보시고, 코멘트 부탁드립니다 :)

sebastianrcnt · 2022-04-01T15:08:47Z

package.json

+    "@types/jest": "^27.4.1",
+    "@types/node": "^17.0.23",
+    "@types/react": "^17.0.43",
+    "@types/react-dom": "^17.0.14",
+    "@types/styled-components": "^5.1.24",


타입 라이브러리의 경우 dev-dependencies에 추가해주면 배포 때 번들 사이즈를 줄일 수 있을 것 같아요!

[https://ingorae.tistory.com/1754](참고 링크)

오 처음 안 사실이네요! 앞으로 참고해서 사용하겠습니다!

sebastianrcnt · 2022-04-01T15:10:01Z

src/App.tsx

+  const _saveLocalStorage = useCallback(
+    (type: string, list: Array<ITodoItem>): void => {
+      localStorage.setItem(type, JSON.stringify(list));
+    },
+    [],
+  );


이 부분은 Hooks 내부에 정의할 필요는 따로 없을 것 같네요 ㅎㅎ

그렇네요! 밖으로 뺄 생각을 못했네요. 감사합니다!!

sebastianrcnt · 2022-04-01T15:10:54Z

src/App.tsx

+const Container = styled.div`
+  height: 100vh;
+  display: flex;
+  justify-content: center;
+  align-items: center;
+`;


사람마다 다른데, 최상위 컴포넌트는 Wrapper로 이름 짓는 경우가 많더라구요 ㅎㅎ

어쩐지 네이밍이 마음에 안 들더라구요.. 알려주셔서 감사합니다!

sebastianrcnt · 2022-04-01T15:16:03Z

src/contexts/TodoContext.tsx

+const TodoProvider = ({ children }: IContextProps) => {
+  const [state, dispatch] = useReducer(reducer, initialState);
+
+  return (
+    <TodoContext.Provider
+      value={{
+        todoList: state.todoList,
+        doneList: state.doneList,
+        dispatch,
+      }}
+    >
+      {children}
+    </TodoContext.Provider>
+  );
+};
+
+export { TodoContext, TodoProvider };


custom hook을 잘 �활용하셨네요!

useTodo()라는 커스텀 훅 내부에 로직을 숨겨 조금 더 추상화해보면 어떨까요?

const { todoList, doneList, dispatch } = useContext(TodoContext); // 로 쓰는 것보다 const { todoList, doneList, addTodo, deleteTodo, addDone, deleteDone } = useTodo() // context을 사용하는 것도 useTodo 속에 숨겨봅시다

이렇게 관심사의 분리를 적용해봅시다!

오 관심사의 분리 적용하면 코드 이해하기도 훨씬 편할 것 같네요. 꼭 적용해보겠습니다:thumbsup:

sebastianrcnt · 2022-04-01T15:16:45Z

src/contexts/index.tsx

+import { TodoContext, TodoProvider } from "./TodoContext";
+
+export { TodoContext, TodoProvider };


import 할 때 깔끔하고 좋군요!
다만, 파일을 추가하고 바꿀때마다 수정해줘야 하니 유지 보수 비용이 늘어날 수 있겠네요

아하 그렇네요.. 조금 더 고민하고 효율적으로 사용해봐야 할 것 같네요!

sebastianrcnt · 2022-04-01T15:20:56Z

src/interface.tsx

+}
+
+export interface ITodoItemProps {
+  type: "todo" | "done";


typescript는 enum 타입을 제공합니다. enum을 활용해보시는 것도 좋을 것 같네요

typescript 너무 어렵네요.. enum 사용해보겠습니다!

sebastianrcnt · 2022-04-01T15:22:29Z

src/interface.tsx

+  todoList: Array<ITodoItem>;
+  doneList: Array<ITodoItem>;
+  dispatch: React.Dispatch<IAction>;


하나도 놓치지 않는 꼼꼼한 타이핑이 인상적이네요.

Array<T> 대신 T[]를 사용할 수 있답니다

Suggested change

todoList: Array<ITodoItem>;

doneList: Array<ITodoItem>;

dispatch: React.Dispatch<IAction>;

todoList: ITodoItem[];

doneList: ITodoItem[];

dispatch: React.Dispatch<IAction>;

넵 감사합니다!

sebastianrcnt · 2022-04-01T15:28:06Z

src/components/TodoInputForm.tsx

+      if (text.trim()) {
+        // list에 todo item 추가
+        dispatch({
+          type: "ADD_TODO",
+          todo: { text },
+        });
+      }
+      _resetText();


이 부분은 앞서 말씀드린 useTodo 훅에 넣어도 될 것 같네요!

const { addTodo } = useTodo() const handleTodoInputChange = (event: React.FormEvent) => { addTodo({text: event.target.value}) } // ... addTodo는 useTodo내에서 구현

이렇게 하면 todoInputForm의 이벤트 처리 로직과 todo 상태 변경 로직의 분리를 달성할 수 있습니다.

넵 커스텀 훅에 적용해보도록 하겠습니다~!

sebastianrcnt · 2022-04-01T15:29:13Z

src/components/TodoItem.tsx

+const TodoItem = ({
+  type,
+  todo,
+  idx,
+  deleteCurrentList,
+  addToggleList,
+}: ITodoItemProps) => {


Suggested change

const TodoItem = ({

type,

todo,

idx,

deleteCurrentList,

addToggleList,

}: ITodoItemProps) => {

const TodoItem = ({

type,

todo,

idx,

deleteCurrentList,

addToggleList,

}: ITodoItemProps): React.FC<ITodoItemProps> => {

보통은 이런 식으로 직접 컴포넌트를 타이핑해준답니다.

아하 넵 저걸 빼먹었네요!

sebastianrcnt · 2022-04-01T15:30:01Z

src/components/TodoItem.tsx

+    ${TodoDeleteBtn} {
+      opacity: 1;
+      transition: 0.1s;
+    }


styled-components의 고급 기능을 잘 활용하셨네요!

S-J-Kim

효정님 안녕하세요

프론트엔드 멘토 김선종입니다.

코드 정말 깔끔하네요... flux가 왜 좋은지는 이런 코드를 볼 때 크게 와닿는 것 같습니다. 타입스크립트가 어렵지는 않으셨나요? 개인적으로 저도 타입스크립트가 걸음마 단계라서 크게 코멘트를 드리지는 못했습니다. 다만 몇가지 참고하시면 좋을거 같은 내용을 적어봤어요.

과제 하느라 고생하셨습니다

S-J-Kim · 2022-04-01T15:22:30Z

src/interface.tsx

+export interface IAction {
+  type: "ADD_TODO" | "DELETE_TODO" | "ADD_DONE" | "DELETE_DONE";
+  todo?: { text: string };
+  idx?: number;
+}


action은 명확하게 나눠줘야 합니다. 현재 코드에서는 ADD_DONE 타입의 action에 payload로 idx를 넘겨준다고 해도 컴파일타임에 에러가 없습니다. (하지만 명백히 잘못된 payload 전달이죠) 타입스크립트의 목적중 하나가 런타임에 에러를 방지하려는 것임을 고려해보면 꼭 지켜져야 하는 부분입니다.

Suggested change

export interface IAction {

type: "ADD_TODO" | "DELETE_TODO" | "ADD_DONE" | "DELETE_DONE";

todo?: { text: string };

idx?: number;

}

export interface IAction {

type: "ADD_TODO";

todo: { text: string };

} |

{

type: "DELETE_TODO";

idx: number;

} |

{

type: "ADD_DONE";

todo: { text: string };

} |

{

type: "DELETE_DONE";

idx: number;

}

action에 따라 다른 payload를 받는 것을 어떻게 선언해줘야 할 지 몰라서 저런 애매한 코드를 작성해버렸네요.. 말씀해주신대로 명확하게 작성하도록 하겠습니다. 감사합니다!

S-J-Kim · 2022-04-01T15:27:37Z

src/contexts/TodoContext.tsx

+    case "ADD_TODO":
+      return {
+        ...state,
+        todoList: todo ? [...state.todoList, todo] : state.todoList,


이 액션은 "해야할 일 목록에 TO-DO 아이템 추가"라는 명시적인 목적을 수행할 수 있어야 합니다. todo 상태에 따라 추가가 될지 안될지를 리듀서에서 결정하는 것은 좋지 못한 패턴입니다.

넵 수정하도록 하겠습니다!

S-J-Kim · 2022-04-01T15:30:27Z

src/contexts/TodoContext.tsx

+const initialState = {
+  todoList: loadTodo,
+  doneList: loadDone,
+  dispatch: () => {},
+};


초기값에 dispatch 함수가 포함될 필요는 없어보이네요

dispatch를 초기값에 설정해주지 않으니까 type에 맞지 않다고 오류가 나더라구요.. 다른 방법을 생각해보도록 하겠습니다!

S-J-Kim · 2022-04-01T15:31:33Z

src/contexts/TodoContext.tsx

+const initialState = {
+  todoList: loadTodo,
+  doneList: loadDone,
+  dispatch: () => {},
+};


Suggested change

const initialState = {

todoList: loadTodo,

doneList: loadDone,

dispatch: () => {},

};

const initialState: IState = {

todoList: loadTodo,

doneList: loadDone,

dispatch: () => {},

};

상태는 타입을 꼭 명시해주세요...^^

아이코 저걸 빼먹었네요.. 감사합니다!!

9yujin

안녕하세요, 3주차 리뷰파트너 한규진입니다!!

1주차 과제부터 효정님 코드보고 배워가는게 참 많습니다. useReducer를 사용하니 확실히 데이터 흐름을 보기가 쉬워지네요. 제가 이번에 타입스크립트를 완전히 처음 접해서 무언가 조언을 드릴 수 있는 게 없어 아쉽습니다. 궁금한점과 배워가는부분에 감사인사 정도 드리고 가겠습니다!!

9yujin · 2022-04-02T14:41:20Z

src/components/TodoList.tsx

+  const { todoList, addDone, deleteTodo } = useTodo();
+
+  // done item 추가
+  const _addDoneList = useCallback(


함수 네이밍에 _(언더바)가 어떤 의미를 갖는지 궁금합니다!!

처음에는 변수명과 함수를 구분하기 위해 사용했던게 습관이 되었던 것 같습니다. 그런데 앞에 정시원 멘토님이 조언해주신 말씀을 보고 찾아보니 private 함수 앞에 붙여주는 일종의 자바스크립트 컨벤션이더라구요!

9yujin · 2022-04-02T14:42:26Z

src/App.tsx

+import { useEffect, useContext } from "react";
+import styled from "styled-components";
+import { TodoInputForm, TodoList, DoneList } from "components";
+import { TodoContext } from "contexts";


contexts에서 index.tsx에서 따로 export 해온게 이부분에서 더 깔끔하게 보이기 위해서라고 이해하면 될까요?? 혹시 다른 이유가 또 있을까요??

특별한 이유는 없고 한 폴더에서 여러 컴포넌트를 import 할 때 한 줄로 깔끔하게 import하기 위해서 자주 사용했습니다..

9yujin · 2022-04-02T14:42:42Z

src/contexts/TodoContext.tsx

+
+const TodoContext = createContext<IState>(initialState);
+
+const reducer = (state: IState, action: IAction): IState => {


전 useReducer 훅을 사용해본 적이 없어서 고려해보지 못했는데, 코드가 굉장히 정리된 느낌을 주는것 같습니다. 종종 사용해봐야겠네요.

9yujin · 2022-04-02T14:43:22Z

src/components/TodoItem.tsx

+const ListItem = styled.li`
+  margin-bottom: 13px;
+  :hover {
+    ${TodoDeleteBtn} {
+      opacity: 1;
+      transition: 0.1s;
+    }


hover 안에 다른 요소를 넣을 수 도 있군요..! 앞으로 저도 자주 쓰게 될 것 같습니다.

jhj2713 added 30 commits March 21, 2022 08:46

style: add container box

a96be62

feat: add addTodoItem function

13dce4a

feat: add todo to done toggle

3426754

feat: add done to todo toggle

730aa49

feat: add delete function

1dfcca7

feat: add saveLocalStorage function

72e52c3

style: change ui

2fd507f

fix: change to functional component

2508a13

style: change UI and add font style

c8638ea

refactor: add list component

66843a2

refactor: add input component

c0530ec

refactor: add styled components

4742cf7

refactor: delete doneList component

e78f1ab

refactor: add styled components in TodoList

73a2035

style: change UI

57c76d1

refactor: add useCallback function

c6f5f52

style: change title and icon

f795be3

style: change UI

5690951

style: change UI

f30b495

refactor: usage of props in styled-components

6e6256c

refactor: change InputText location

22d1f3d

fix: dependency warning

8f38c5e

refactor: add globalStyle

a971121

refactor: change localStorage load location

3ba7e60

refactor: change Styled-components location

6edb893

refactor: add todoItem

9b3b9e9

refactor: refactor typescript

3d0c45a

refactor: add inputType

f813526

refactor: add custom hooks

ed12088

refactor: interface refactoring

5404fc6

jhj2713 added 7 commits March 29, 2022 17:11

refacor: add context

a1514c9

refactor: remove memo

8eb76cc

refactor: change title location

6c10362

refactor: add customInput hook

0aaf39f

refactor: add reducer

4024ce2

chore: add type

90f7462

chore: add comment

c950615

sebastianrcnt reviewed Apr 1, 2022

View reviewed changes

S-J-Kim reviewed Apr 1, 2022

View reviewed changes

jhj2713 added 3 commits April 2, 2022 18:13

refactor: change some types and rename tag

69ad840

refactor: add useTodo hook

dbec7e7

refactor: change function name

69f52da

9yujin reviewed Apr 2, 2022

View reviewed changes

		import { TodoContext, TodoProvider } from "./TodoContext";

		export { TodoContext, TodoProvider };

-export interface IAction {
-  type: "ADD_TODO" | "DELETE_TODO" | "ADD_DONE" | "DELETE_DONE";
-  todo?: { text: string };
-  idx?: number;
-}
+export interface IAction {
+  type: "ADD_TODO";
+  todo: { text: string };
+} |
+{
+  type: "DELETE_TODO";
+  idx: number;
+} |
+{
+  type: "ADD_DONE";
+  todo: { text: string };
+} |
+{
+  type: "DELETE_DONE";
+  idx: number;
+}


		const TodoContext = createContext<IState>(initialState);

		const reducer = (state: IState, action: IAction): IState => {

[3주차] 주효정 미션 제출합니다. #15

Are you sure you want to change the base?

[3주차] 주효정 미션 제출합니다. #15

Conversation

jhj2713 commented Mar 31, 2022

배포 링크

신경 쓴 부분

sebastianrcnt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

S-J-Kim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

9yujin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment