Skip to content

Commit

Permalink
[wook] Step19 리뷰 부탁드립니다. (#34)
Browse files Browse the repository at this point in the history
* chore: 배경 css 변경

* refactor: state를 todos로 변경

- isLoading도 state이므로 명시적으로 todos로 변경하였다.
이에따라 todosReducer도 state객체안의 todos 키의 배열이 아닌 todos배열로 변경하여 return을 객체가 아닌 배열로 변경하였음.

* refactor: calcStatusCnt 함수 삭제

- 결과는 todoCnt , doneCnt로 state로 관리할필요가없음
어짜피 출력만 하는것이기때문

* chore: 함수명 변경

* refactor: router에 따른 폴더구조 변경

* feat: Home , Header 컴포넌트구현및 스타일링

* style: Home 컴포넌트 문장 배치수정

* chore: 배포, 개발용 변경기능 구현

* feat: PropTypes로 TodoItem 구현

1. props를 {...todo}로 넘기도록 변경
2. propTypes에 뎁스가 있는경우 shape를 사용할 수 있음.

* feat: StateContext.Provider에 proptypes 구현

궁금증 - state는 타입체크 안하나?

* refactor: 최적화를 위한 Provider별 value분리

하나의 Provider에서 value객체를 받아서 관리하는것이 아닌,
각각 Provider별 value를 분리하고 커스텀 훅을 만들어서
InputBar , ResultBar 리랜더링을 막음.

이렇게 구현하면 Home , ResultBar에서 useMemo를 사용할 필요가 없음(본질적인 해결책이 아님)

* chore: TodoItem propTypes id change

* refactor: TodoItem을 react.memo로 최적화함

* chore: 주석 삭제

* docs: 최적화관련 진행사항 기록

* refactor: react-router 초기화면 home 컴포넌트로 변경

how to redirect path "/" to "/home" react-router  루트를 /가 아니라 /home으로 만들수는 없나? 그렇게 하는게 안좋은가? 라는 고민이 있음.
- redirect로 /로 들어온걸 /home으로 보내는 방법은 있지 않을까?

* bugfix: 탭간 이동시 status 남아있도록 구현

IsClicked 변수와 setIsClicked 대신
props에서 status를 받아서 status가 "todo" 인지 "done" 인지에 따라 다른 style 적용

* refactor: client 하위로 폴더구조 변경

서버와 분리하기 위해 폴더구조 변경

* feat: dev 용 서버코드 작성

* refactor: change port number

하드코딩된 포트번호를 port 변수이용하여 변경

* refactor: express.Router 이용해 분리

 - send와 json 차이는 뭐지?
- router 변수의 역할은 뭐지?
- const 왜 안쓰지?

-- json파일 어떻게 읽어오지?

* refactor: json파일 따로 분리후 읽어오도록 변경

1. fs가 아닌 require로 읽도록함
2. 굳이 router에서 /list라는 하위 url을 두어 구성 ( 시험하고싶어서 해봄)

- fs로 읽는것의 2가지 장점. require은 동기적으로 읽어옴, 이렇게 지정한 파일은 Json밖에 못읽음 , 근데 귀찮음
- https://stackabuse.com/reading-and-writing-json-files-with-node-js/

* chore: 불필요한 주석 제거

* bugfix: webpack 개발, 배포용 변경

1. webpack.config.js 파일 enviroment 변수 수정
2. DefingPlugin에 ACCESS_PATH 추가 ( json.stringfy로 감싸주어야함)
3. TodoApp 컴포넌트에서 Route, Link 태그 수정

path={ACCESS_PATH+ "about"}
path={`${ACCESS_PATH}`+"about"} 굳이 한번더 감싸지 않아도됨
  • Loading branch information
pocokim authored and crongro committed Sep 4, 2019
1 parent 7deb379 commit 5394766
Show file tree
Hide file tree
Showing 35 changed files with 1,071 additions and 327 deletions.
18 changes: 0 additions & 18 deletions README.md

This file was deleted.

48 changes: 48 additions & 0 deletions client/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@

## 1. 설계

### 1.1 컴포넌트
1. 전체를 감쌀 컴포넌트 :TodoApp
2. 결과를 출력하는 컴포넌트 : ResultBar
3. 할일 입력하는 컴포넌트 : inputBar
4. 할일 목록을 감싸는 컴포넌트 : TodoList ( 해야할일 , 접기는 짜잘해서 포함시켜버림)
5. 할일 목록 컴포넌트 : TodoItem

### 1.2 구조
- TodoApp
- ResultBar
-RadiusDisplayer (todos)
-RadiusDisplayer (done)
- InputBar
- TodoList
- TodoItem

## 2. 코드개선
### 2.1 Proptypes 를 어디에 사용할것인가
- Todoitem
- StateProvider

### 2.2 최적화
#### Profiler 탭 분석
- [1->2] todos fetch해올때 inputBar 리랜더 막기
- [2->3] isLoading 변경될때 resultBar , InputBar 리랜더 막기
- [6->7] InputBar의 "ADD_TODO"로 인해 todos 추가될때 TodoItem 리랜더 막기, InputBar 리랜더 막기(불가능)

#### contextAPI 쪽 코드변경
- TodoState.js의 Provider의 value 3개 한꺼번에 넣지말고 나눠서 넣어
isLoading, todos, dispatch 사용하면 리랜더링 방지.
- Provider를 2개로 todos,dispatch묶어서 넘겼을땐 todos가 바뀌어도 dispatch만
사용하는 resultBar가 리랜더가 일어남.
- context 쓰면서 리랜더 막는 방법
https://github.com/facebook/react/issues/15156 컨텍스트 쪼개기
(useMemo , useCallback 안써도 이걸로 리랜더 막을 수 있음 )

#### useCallback, useMemo
- StateProvider 는 isLoading , todos를 가져올때 값이 바뀌지만 InputBar은 리랜더 막을 수 있고 dispatch에 useCallback , todos 에 useMemo를 사용하면된다.-> disaptch에 useCallback,useMemo사용할 수 없음. (hook안에 hook사용하게 됨)
- https://github.com/facebook/react/issues/15351 props에만 적용되는게 아님


### 2.3 아쉬운 점
- Profiler 탭에서 무엇에 의해 render 되는지 알면 더 좋을듯.. 몰라서 console찍어서 확인함
- 과제가 useCallback과 useMemo를 사용하기에 적절한 사례가 아니었던것같음.
- Profiler 탭에서 reload하고 재실행할때마다 시간이 다르게 측정되어 최적화가 됬는지 확인하기 어려웠음.
File renamed without changes.
39 changes: 39 additions & 0 deletions client/dist/bundle.3a707650d37cf08ce718.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/index.html → client/dist/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@
</head>
<body>
<div id="root"></div>
<script type="text/javascript" src="bundle.820c29be6ed22002e773.js"></script></body>
<script type="text/javascript" src="bundle.3a707650d37cf08ce718.js"></script></body>
</html>
6 changes: 4 additions & 2 deletions package.json → client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,17 @@
"author": "sunwook kim <[email protected]>",
"license": "MIT",
"scripts": {
"start": "webpack-dev-server",
"start": "NODE_ENV=development webpack-dev-server",
"build": "webpack --mode production"
},
"dependencies": {
"@babel/runtime": "^7.5.5",
"prop-types": "^15.7.2",
"react": "^16.9.0",
"react-dom": "^16.9.0",
"react-id-generator": "^2.0.0",
"react-prop-types": "^0.4.0"
"react-prop-types": "^0.4.0",
"react-router-dom": "^5.0.1"
},
"devDependencies": {
"@babel/cli": "^7.5.5",
Expand Down
File renamed without changes.
9 changes: 9 additions & 0 deletions client/src/About.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import React from 'react'

export default function About() {
return (
<div>
어바웃
</div>
)
}
4 changes: 2 additions & 2 deletions src/App.js → client/src/App.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { hot } from "react-hot-loader/root";
// import { hot } from "react-hot-loader/root";
import React from "react";
import TodoApp from "./TodoApp";

Expand All @@ -11,4 +11,4 @@ const App = props => {
);
};

export default hot(App);
export default App;
36 changes: 36 additions & 0 deletions client/src/Home.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import React from "react";
import { useTodosStateValue } from "./StateHelper/TodoState";
import styled from "styled-components";

const HomeWrapper = styled.div`
font-size: 1.2rem;
margin: 0;
text-align:center;
p {
margin: 0;
span {
color: hotpink;
}
}
div{
margin:2rem;
}
`;

export default function Home() {
const todos = useTodosStateValue();
const todoCount = todos.filter(todo => todo.status === "todo").length;
const doneCount = todos.filter(todo => todo.status === "done").length;

return (
<HomeWrapper>
<div>반갑습니다.할일관리 애플리케이션입니다.</div>
<p>
현재 해야 할일이 <span>{todoCount}</span>개,
</p>
<p>
완료된 일이 <span>{doneCount}</span>개 있습니다.
</p>
</HomeWrapper>
);
}
52 changes: 52 additions & 0 deletions client/src/StateHelper/TodoState.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import React, { createContext, useContext, useReducer, useState } from "react";
import todosReducer from "./todoReducer";
import useFetch from "../customHooks/useFetch";
import PropTypes from "prop-types";

const TodosStateContext = createContext();
const TodosDispatchContext = createContext();
const IsLoadingContext = createContext();

export const StateProvider = ({ children }) => {
const [todos, dispatch] = useReducer(todosReducer, []);

const initTodoData = todos => {
dispatch({ type: "INIT_TODO", todos });
};

const isLoading = useFetch(initTodoData);

return (
<TodosStateContext.Provider value={todos}>
<TodosDispatchContext.Provider value={dispatch}>
<IsLoadingContext.Provider value={isLoading}>
{children}
</IsLoadingContext.Provider>
</TodosDispatchContext.Provider>
</TodosStateContext.Provider>
);
};

export const useTodosStateValue = () => {
return useContext(TodosStateContext);
};

export const useTodosDispatchValue = () => {
return useContext(TodosDispatchContext);
};

export const useIsLoadingValue = () => {
return useContext(IsLoadingContext);
};

TodosStateContext.Provider.propTypes = {
value: PropTypes.array
};

TodosDispatchContext.Provider.propTypes = {
value: PropTypes.func
};

IsLoadingContext.Provider.propTypes = {
value: PropTypes.bool
};
39 changes: 39 additions & 0 deletions client/src/StateHelper/todoReducer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
const todosReducer = (todos, action) => {
switch (action.type) {
case "INIT_TODO":
return action.todos;

case "ADD_TODO":
const newTodoObj = {
title: action.title,
id: action.id,
status: "todo"
};
return [...todos, newTodoObj];

case "DELETE_TODO":
const remaindedTodos = todos.filter(todo => todo.id !== action.id);
return remaindedTodos;

case "CHANGE_TODO_STATUS":
const index = todos.findIndex(todo => todo.id === action.id);
const selected = todos[index];
const nextTodos = [...todos];

const statusToggle = status => {
return status === "todo" ? "done" : "todo";
};

nextTodos[index] = {
...selected,
status: statusToggle(selected.status)
};

return nextTodos;

default:
return todos;
}
};

export default todosReducer;
9 changes: 4 additions & 5 deletions src/Header/InputBar.js → client/src/Todo/Header/InputBar.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import React, { useState } from "react";
import styled from "styled-components";
import nextId from "react-id-generator";
import { useStateValue } from "../StateHelper/TodoState";
import { useTodosDispatchValue } from "../../StateHelper/TodoState";

const Form = styled.form`
background: #a6d0d1;
height: 4em;
display: flex;
justify-content: center;
Expand All @@ -15,7 +14,7 @@ const Form = styled.form`

export default function InputBar() {
const id = nextId();
const { dispatch } = useStateValue();
const dispatch = useTodosDispatchValue();
const [newTodo, setNewTodo] = useState("");

const handleChangeInput = ({ target: { value } }) => {
Expand All @@ -34,9 +33,9 @@ export default function InputBar() {

return (
<Form>
<label>할일 입력 : </label>
<label>enter todo : </label>
<input value={newTodo} onChange={handleChangeInput} />
<button onClick={handleAddtodo}>등록</button>
<button onClick={handleAddtodo}>Submit</button>
</Form>
);
}
31 changes: 31 additions & 0 deletions client/src/Todo/Header/ResultBar.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import React, { useMemo } from "react";
import { useTodosStateValue } from "../../StateHelper/TodoState";
import Displayer from "../../components/Displayer";
import styled from "styled-components";

const ResultWrapper = styled.div`
display: flex;
justify-content: flex-end;
`;

export default function ResultBar() {
const todos = useTodosStateValue();

const countStatus = status => {
return todos.filter(todo => todo.status === status).length;
};

const todoCount = countStatus("todo");
const doneCount = countStatus("done");

return (
<ResultWrapper>
<Displayer
color={"rgb(210, 209, 221);"}
status={"todos"}
display={todoCount}
/>
<Displayer status={"done"} display={doneCount} />
</ResultWrapper>
);
}
29 changes: 19 additions & 10 deletions src/Todo/TodoItem.js → client/src/Todo/TodoItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,17 @@ import React, { useState } from "react";
import styled from "styled-components";
import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
import { faTimes } from "@fortawesome/free-solid-svg-icons";
import { useStateValue } from "../StateHelper/TodoState";
import { useTodosDispatchValue } from "../StateHelper/TodoState";
import PropTypes from "prop-types";

const Title = styled.span`
height: 2rem;
line-height: 2rem;
&:hover {
background: rgba(255, 255, 255, 0.9);
}
${({ isClicked }) =>
isClicked &&
${({ status }) =>
status === 'done' &&
`
text-decoration: line-through;
color: #adb5bd;
Expand All @@ -22,28 +23,36 @@ const StyledFontAwesomeIcon = styled(FontAwesomeIcon)`
margin-left: 0.5rem;
`;

export default function TodoItem(props) {
const { dispatch } = useStateValue();
const { title, id } = props.todo;
const [isClicked, setIsClicked] = useState(false);

function TodoItem({ title, id , status }) {
const dispatch = useTodosDispatchValue();

const handleDeleteTodo = () => {
dispatch({ type: "DELETE_TODO", id });
};

const handleClick = () => {
setIsClicked(!isClicked);
dispatch({ type: "CHANGE_TODO_STATUS", id });
};

return (
<>
<li>
<Title onClick={handleClick} isClicked={isClicked}>
<Title
onClick={handleClick}
// isClicked={isClicked}
status={status}
>
{title}
</Title>
<StyledFontAwesomeIcon icon={faTimes} onClick={handleDeleteTodo} />
</li>
</>
);
}

TodoItem.propTypes = {
title: PropTypes.string,
id: PropTypes.oneOfType([PropTypes.number, PropTypes.string])
};

export default React.memo(TodoItem);
Loading

0 comments on commit 5394766

Please sign in to comment.