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

[wook] Step19 리뷰 부탁드립니다. #34

Merged
merged 24 commits into from
Sep 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
a1b35a2
chore: 배경 css 변경
pocokim Aug 26, 2019
b41231b
refactor: state를 todos로 변경
pocokim Aug 26, 2019
f2ff25d
refactor: calcStatusCnt 함수 삭제
pocokim Aug 26, 2019
c8683c2
chore: 함수명 변경
pocokim Aug 27, 2019
3ab3bf8
refactor: router에 따른 폴더구조 변경
pocokim Aug 27, 2019
8b60830
feat: Home , Header 컴포넌트구현및 스타일링
pocokim Aug 27, 2019
6456f78
style: Home 컴포넌트 문장 배치수정
pocokim Aug 27, 2019
ba4cd7b
chore: 배포, 개발용 변경기능 구현
pocokim Aug 28, 2019
a11bdff
feat: PropTypes로 TodoItem 구현
pocokim Aug 30, 2019
74db1b2
feat: StateContext.Provider에 proptypes 구현
pocokim Aug 30, 2019
bc842b4
refactor: 최적화를 위한 Provider별 value분리
pocokim Sep 1, 2019
c890547
chore: TodoItem propTypes id change
pocokim Sep 1, 2019
9baad4a
refactor: TodoItem을 react.memo로 최적화함
pocokim Sep 1, 2019
9d81800
chore: 주석 삭제
pocokim Sep 1, 2019
22c267a
docs: 최적화관련 진행사항 기록
pocokim Sep 1, 2019
bbc9672
refactor: react-router 초기화면 home 컴포넌트로 변경
pocokim Sep 2, 2019
d40f88a
bugfix: 탭간 이동시 status 남아있도록 구현
pocokim Sep 2, 2019
d653db8
refactor: client 하위로 폴더구조 변경
pocokim Sep 3, 2019
e9dd4c2
feat: dev 용 서버코드 작성
pocokim Sep 3, 2019
2e81604
refactor: change port number
pocokim Sep 3, 2019
4f3152d
refactor: express.Router 이용해 분리
pocokim Sep 3, 2019
1cbd786
refactor: json파일 따로 분리후 읽어오도록 변경
pocokim Sep 3, 2019
36d66d6
chore: 불필요한 주석 제거
pocokim Sep 3, 2019
ad2da91
bugfix: webpack 개발, 배포용 변경
pocokim Sep 3, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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를 사용하기에 적절한 사례가 아니었던것같음.
Copy link
Contributor

Choose a reason for hiding this comment

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

추가할만한 기능하나만 아이디어 좀 주시면 감사..!

Copy link
Author

Choose a reason for hiding this comment

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

넵 생각해보겠습니다!

- 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 <koownusmik@gmail.com>",
"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