[wook] Step18 리뷰 부탁드립니다. #27

pocokim · 2019-08-21T11:11:31Z

구현사항

contextAPI와 useReducer를 이용한 구현
CRA를 webpack으로 변경
express 대신 github로 호스팅

느낀점

1. state를 제어하는 함수들은 줄일 수 없으며 어떻게 정리할것인가가 포인트

이전에 캐러샐 미션에서 controller가 비대해지는것과 비슷한 느낌이었습니다.
initialState에 todos 와 newTodo를 등록했기에 useReducer크기가 todos만 바꾸는것에 비해 훨씬 더 비대해졌습니다. newTodo는 input이 바뀔때마다 랜더링이 되므로 이렇게 코드를 짜면 랜더링이 훨씬 더 많이 일어난다는 문제점이 있지만..

2. 무엇을 props로 넘겨줄것인가

부모 - 자식과 같은 1단계면 props로 넘겨주는게 좋을것같습니다. 미션에서는 의도적으로 props를 안넘겨주려고 노력했습니다.
(제가 멍청해서 ㅎㅎ... 수업에서 여쭤보았던것처럼 map함수의 index도 dispatch를 사용하면 넘겨줄 필요가 없었는데...그건 넘겨버렸습니다.. )

3. 디버깅

가끔 에러가 나오는데 어디에서 틀렸는지를 안알려주는 경우가 있었습니다. 감으로 이것저것 바꿔주면서 원인은 못찾고 현상은 해결을 했는데...(문제상황을 표현하기가 어렵네요 ㅜ ) 원리를 좀 더 공부해야겠다는 생각이 들었습니다.

데모링크

https://dxvinci.github.io/react-todo/dist/
read 용 API는 github hosting으로도 가능하다는것을 구글링으로 찾아서 요걸로 배포해보았습니다 ㅎㅎ

changeRequest 받을 준비가 되어있씁니다 ㅎ..... 감사합니다! 🙇‍♂️

1. 모든 컴포넌트를 클래스형에서 함수형으로 변경 - 함수형으로 변경할때는 this를 안쓰고 함수형 컴포넌트가 props만 받으면 됨. - 클래스가 아닌 함수이므로 콘솔위치 , 변수위치, 함수위치 하나도 신경쓸거 없음 (대신 함수는 표현식으로 작성해야함) -useEffect안의 콜백함수가 componentDidMount때 실행 - setTodos 로 todos 를 변경 ( 마운트시 state가 변경)

- hooks 를 순서대로 사용하기 위한 eslint 플러그인추가 - id 생성용 react-id-genertator 추가 TodoTable 컴포넌트에 추가한것 1. input에 저장될 newTodo를 state로 등록. 2. newTodo 를 변경하기 위한 함수 changeInputData 함수도 추가 3. newTodo가 todos에 추가되도록 하는 addTodo 함수 추가 4. InputBar에 newTodo, addTodo , changeInputData props로 등록

1. 이벤트를 넘기는게 아니라 props로 전달한 값을 인자로 넘겨주는 방식으로 변경하였음. 2. 랩핑해주는 handle 함수 자식에 생성 클릭이 발생한 html태그의 이벤트객체를 넘기는것이 아니라, props나 컴포넌트 내부에서 생성한 값(id) 을 넘겨주었고, 부모 컴퍼넌트에서 인자를 받으므로 자식 컴포넌트에서 인자를 넘겨주기위해 또는 preventDefault와 같은 사전처리를 위한 랩핑해주는 함수 handleDeleteTodo , handleAddTodo 함수를 만들어 처리

todos.filter(todo=>todo.id !== id); 로 넘겨받은 id가 아닌 새로운 배열을 만들어 setTodos에 넣어주는 방식으로 구현 이게 가장 좋은 방법인지는 의문

1. OutputRow 컴포넌트에 isClicked state를 두고 handleClick 함수를 이용하여 상태를 변경 2. isClicked를 Li 컴포넌트에 props로 전달하여 props값에 따라 다른 CSS가 적용되도록 구현

isOpened의 상태에 따라 다른 css 스타일이 적용되도록 구현

기존의 fetchInitialData 함수에서 await 과 async 를 삭제할 수 없음;; 더 쉽게 짠건데 뭐하러 때야하는지 의문

리팩토링하여 재사용가능한 컴포넌트용 components 폴더를 생성하고RadiusDisplater라는 컴포넌트를 components 하위의 폴더에 구현하였음. status에 따라 css속성을 다르게 주려고 하나 어떻게 다르게 처리할지는 아직 미정 ( gren 스타일로 해보려고 함 ㅎㅎ)

1. styledComponents에 props로 넘겨주어야한다. 2. 스타일드 컴포넌트에 조건부 랜더링을 할때도 object deconstructing 을 사용할 수 있다.

initialState에는 값이 한번만 들어가므로 useEffect대신 일단 todos를 하드코딩하여 구현함

1. OutputRow로 props전달하지 않고 title,id 를 랜더할 수 있도록 구현 - 이때 props로 각 OutputRow를 구분할 수 있도록 idx는 넘겨주었음 ( 더 나은 방법이 있는지는 의문) - key값을 안넘겨줘도 warning이 나오지 않음 - filter가 아닌 find를 이용해서 각 idx에 맞는 title,id값을 찾고 OutputRow에 랜더링 하도록 하였음 2. delete기능 구현

- delete 액션일떄 ...state가 없어서 추가함

1. OutputRow에서 todoReducer에 changeStatus 타입 추가 - 이때 갯수는 reduce로도 할 수 있지만 filter하고 length사용하는게 마음이 편해서 사용함. 2. changeStatus에선 선택된 todo를 찾고, status만 다르게해서 덮어씨우고 그 todo가 포함된 nextTodo를 state에 넣는 방식으로 구현 3. 토글을 true , false로 어떻게 해야할지 몰라 selected.status === "todo" ? "done" : "todo" 이런식으로 구현했고, 하드코딩 하는게 싫어서 내부함수를 만들어서 처리하였음.

crongro · 2019-08-21T21:29:28Z

깨달은 것처럼 dispatch만을 전달할 때 장점이 있더라고요.

디버깅은 도구를 익히는 것도 중요해요. React의 디버깅방법을 밀도있게 알아두고, 디버깅을 통해서 문제의 원인을 찾는 습관일 갖는 게 중요해 보여요.

crongro

학습에 필요한 부분을 코드에 다 담았네요. 잘했어요. context/reducer를 잘 섞어서 썼어요.
hooks에 대한 이해가 좀 올라가셨기를~

전체적인 naming은 좀더 신경써보세요.

Component이름, 디렉토리이름 등도요.
예)
Output 디렉토리라는 이름은 배포용 디렉토리 같아보여요.

crongro · 2019-08-21T21:31:45Z

package.json

+    "react-prop-types": "^0.4.0"
+  },
+  "devDependencies": {
+    "@babel/cli": "^7.5.5",


'^' 이 의미하는 건 뭘까요?
공부하고 모듈 업데이트관련해서 왜 '^' 이런 것이 것이 필요한지 깨달음을 얻으시길~

모듈 업데이트는 유지보수할때 중요해요.

tilde (~) and caret (^)
https://stackoverflow.com/questions/22343224/whats-the-difference-between-tilde-and-caret-in-package-json

지원가능한 버전의 범위를 나타냅니다.

설정에 따라서,
업데이트 할때 어디까지 자동으로 업데이트가 되는 것인지 알아둘 필요가 있어요.

crongro · 2019-08-21T21:39:40Z

src/Output/OutputTable.js

+const Background = styled.div`
+  background: #a6d0d1;
+`;
+const Table = styled.div`


Table 이라는 이름을 파일명에도 쓰고 했는데, 이게 잘 안쓰이는 이름이라 의도를 알기 어렵네요.
Table은 표를 뜻하니까, 표형태의 UI라고 생각할 수 있을거 같아요. 실제로 table 태그도 존재하기도 하고요.

crongro · 2019-08-21T21:42:42Z

src/TodoTable.js

+
+  const todosReducer = (state, action) => {
+    switch (action.type) {
+      case "add":


reducer 구현 잘 했네요.
immutable 의 장점을 깨닫고(shallow equality check가능) 그렇게 코딩계속해보세요.

action은 변수명과는 다르게 구분하는게 좋아요. 상수처럼 다루는 게 좋아서 대문자로 많이 표현하곤 해요.
'ADD_TODO' , 'CHANGE_TODO_STATUS' 등

crongro · 2019-08-21T21:44:03Z

src/TodoTable.js

+    try {
+      const response = await fetch(url);
+      const data = await response.json();
+      if (data.statusCode === "404")


404 처리는 좋고요. 다른 status code에 대해서도 방어적으로 처리해보는 것도 좋겠죠.

다른 status code는 어떤 경우에 에러가 나는지 경험해보아야 이해할 수 있을것같습니다 ㅠㅠ
바로 떠오르는 코드로는 500번대가 있는데..요.

사실 저렇게 방어적으로 처리를 해도 잘못된 url로 작성을 할시 (물론 이경우에는 url자체가 잘못되서 이런 에러가 생기는것이지만)

경험해보지 못해서 와닫지가 못하는것같습니다..

crongro · 2019-08-21T21:45:52Z

src/TodoTable.js


+  useEffect(() => {
+    fetchInitialData(
+      "https://dxvinci.github.io/react-todo/todolist.json"


API주소는 로직과 관련없는 부분임에도, 자주 변경될 수 있죠. (실무에서는 test API, real API주소가 있곤해요)

이런 부분을 동적으로 변경할수 있도록 하려면
javascript파일로 config.js 에 넣어줄 수도 있겠고,
그것보다는 환경설정파일을 활용해서 webpack 빌드단계에서 변경시킬 수도 있을거에요.

FetchUrl을 보니, DefinePlugin 을 사용하긴 했네요!!

crongro · 2019-08-21T21:52:05Z

src/components/RadiusDisplayer.js

+import React from "react";
+import styled, { css } from "styled-components";
+
+const Div = styled.div`


Div라는 이름은 진짜별로 인데요. 맥락이 없는 이름이라서 그래요.
이놈의 역할이 무엇인지 생각해서 그걸로 이름을 지어보세요.

이름 생각하는게 너무 어려워서 그냥.. html 태그명을 가져다 쓰려고 했습니다 ㅠㅠ 애매한 맥락보다는 명시적인 div가 더 나을수도 있다구 생각해서요

crongro · 2019-08-21T21:54:32Z

src/Output/OutputTable.js

+
+  console.log("OutputTable is render...");
+  return (
+    <Background>


만약 여기에 다른 CSS 스타일 속성이 추가되면 Background라는 이름어 거짓이 될 거 같고..
차라리 라는 이름이 더 나을 거 같네요.

crongro · 2019-08-21T21:55:44Z

src/state.js

@@ -0,0 +1,15 @@
+import React, { createContext, useContext, useReducer } from "react";


state.js 도 너무 일반적인 이름이죠.
의도와 맥락을 생각하면서 네이밍을...

실제로
import { useStateValue } from "../state";
이렇게 부르면 어떤 state를 사용하는 것인지 의도를 알수가 없죠.

crongro · 2019-08-21T21:56:34Z

src/state.js

@@ -0,0 +1,15 @@
+import React, { createContext, useContext, useReducer } from "react";
+
+export const StateContext = createContext();


context부분을 분리해서 관리하는 게 좋아보이네요.

crongro · 2019-08-21T21:58:15Z

src/Header/InputBar.js

+  const [{ todos,newTodo }, dispatch] = useStateValue();
+
+
+  const handleChangeInput = ({ target: { value } }) => {


디스트럭츄링도 잘했네요 드디어 ㅎ

useReducer를 잘 쓰고 있어서 좋네요.

1. radius displayer 에서 displayer 로 컴포넌트를 변경하고, 그 안의 displayer의 스타일드 컴포넌트를 radius 로 변경 2. OutputRow, Table, TodoTable 을 TodoItem , TodoList ,TodoApp으로 변경

이때 export default 함수이름을 사용하면 안됨. - export default function 으로 사용시 expression(표현식)으로 표현해야하고, - arrow function을 사용하고 싶다면 declaration해주고 아래에 export default 로 빼줘야한다.

1. TodoApp에 있던 newTodo를 InputBar로 state의 위치를 변경하였음 2. todoReducer 가볍게 만듦 - initialState로 newTodo를 받으면 위에 있어야하는게 맞지만, 1. reducer함수가 무거워지고 2. newTodo가 바뀔때마다 state가 바뀌어 TodoApp부터 랜더링이 일어나기때문에 newTodo 를 state에서 분리, newTodo를 change해주는 로직을 reducer에서 분리 3. 기존의 handleInputChange 함수 삭제 setNewTodo 함수가 변경해주는것이기때문에 굳이 handle함수로 감싸서 넘겨줄 필요없음.

TodoApp에서 reducer를 import 해서 Provider의 인자로 넣어주는것이 아닌, todoState에서 import해서 바로 사용하도록 하여 TodoApp -> todoState로 인자를 넘겨주는 의존관계(?)를 없앰

1. todoApp 안의 fetch 관련 로직을 todoApp으로 이동 2. todoState의 value에 isLoading을 추가 , fetch가 성공하면 initTodoData 함수 추가후 실행 3. TodoItem은 TodoList에서 todo를 props로 받아 처리하도록 변경 4. ResultBar에서 status의 갯수를 계산하는 calcStatusCnt 함수만들고 useEffect로 실행 5. 기존의 Loader 컴포넌트삭제

1. TodoState에서 생성한 init함수를 인자로 넘겨준다 2. useFetch.js의 의미 - useFetch라는 함수를 가져와서 TodoState.js파일 안의 StateProvider 컴포넌트에서 useEffect 를 사용하는것. 즉 컴포넌트가 생성된다음에 useEffect가 일어나고 useFetch에서 useEffect가 포함되어있는이유는 불러진 컴포넌트에서 사용하라는뜻이다. 3. isLoading을 리턴한다.

pocokim · 2019-08-26T00:46:23Z

말씀해주신 수정사항 변경하였습니다. 다시 리뷰 요청부탁드립니다 ㅎㅎ 🙇‍♂️

변수명 변경
state변경 : todoReducer의 newTodo를 InputBar의 내부 state로 관리하도록 변경
todoReducer 함수를 TodoApp이 아닌 다른 파일에 위치하도록 분리
로딩상태에 따른 조건부 랜더링 가능하도록 구현

ResultBar의 status출력 조건에 따라 실행가능하도록 calcStatusCnt 구현
TodoList의 makeTodoList구현

useFetch 커스텀 훅 생성

crongro

네이밍 위주로 모두 수정이 잘 된 거 같네요. 👍

좋은코드는 의도를 잘 드러내는데요.
뭐 자주 봐야죠.

오픈소스등 남의코드를 읽는 활동을 해보세요.
그런 스터디도 좋고요.

pocokim added 19 commits August 13, 2019 14:58

feat: X버튼 클릭시 삭제기능 추가

da02abb

todos.filter(todo=>todo.id !== id); 로 넘겨받은 id가 아닌 새로운 배열을 만들어 setTodos에 넣어주는 방식으로 구현 이게 가장 좋은 방법인지는 의문

feat: 할일 클릭시 취소기능 구현

74f8f92

1. OutputRow 컴포넌트에 isClicked state를 두고 handleClick 함수를 이용하여 상태를 변경 2. isClicked를 Li 컴포넌트에 props로 전달하여 props값에 따라 다른 CSS가 적용되도록 구현

feat: OutputTable안의 접기/펼치기 기능 구현

ff9cd62

isOpened의 상태에 따라 다른 css 스타일이 적용되도록 구현

feat: 로더 컴포넌트로 로딩 메세지 노출 구현

b4ac153

기존의 fetchInitialData 함수에서 await 과 async 를 삭제할 수 없음;; 더 쉽게 짠건데 뭐하러 때야하는지 의문

chore: 폴더 구조 변경

0f10236

refactor: styledComponents 조건부 랜더링 구현

200603c

1. styledComponents에 props로 넘겨주어야한다. 2. 스타일드 컴포넌트에 조건부 랜더링을 할때도 object deconstructing 을 사용할 수 있다.

refactor: useReducer , useContext로 inputBar의 add기능 구현

1cdf2f0

initialState에는 값이 한번만 들어가므로 useEffect대신 일단 todos를 하드코딩하여 구현함

chore: 불필요한 주석삭제

eef2ea2

- delete 액션일떄 ...state가 없어서 추가함

refactor : useEffect로 fetch 로 받아온 데이터 적용

82e07b2

chore : CRA 삭제

566c8e8

chore: webpack으로 수정

9cb487e

refactor: 서버 url 변경

536f3a0

chore: 불필요한 주석삭제

f0424b5

crongro requested changes Aug 21, 2019

View reviewed changes

pocokim added 9 commits August 22, 2019 16:03

refactor: 네이밍 변경

fe689e5

1. radius displayer 에서 displayer 로 컴포넌트를 변경하고, 그 안의 displayer의 스타일드 컴포넌트를 radius 로 변경 2. OutputRow, Table, TodoTable 을 TodoItem , TodoList ,TodoApp으로 변경

chore: dev, production 용 url 변경

1da7508

refactor: 스타일드 컴포넌트 css 제거

e6cfdb1

chore: 파일명 변경

895189a

refactor: reducer 분리

6d37e18

chore: react-hot-loader 위치 변경

f08e1cf

refactor: todosReducer 위치변경

faf3415

TodoApp에서 reducer를 import 해서 Provider의 인자로 넣어주는것이 아닌, todoState에서 import해서 바로 사용하도록 하여 TodoApp -> todoState로 인자를 넘겨주는 의존관계(?)를 없앰

pocokim added 3 commits August 26, 2019 09:13

chore: 가독성을 위한 코드 정리

cabdaab

crongro approved these changes Aug 26, 2019

View reviewed changes

crongro merged commit 7deb379 into code-squad:wo0kgod Aug 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[wook] Step18 리뷰 부탁드립니다. #27

[wook] Step18 리뷰 부탁드립니다. #27

pocokim commented Aug 21, 2019

crongro commented Aug 21, 2019

crongro left a comment

crongro Aug 21, 2019

pocokim Aug 21, 2019

crongro Aug 22, 2019

crongro Aug 21, 2019

crongro Aug 21, 2019

crongro Aug 21, 2019

pocokim Aug 21, 2019

crongro Aug 21, 2019

crongro Aug 21, 2019

crongro Aug 21, 2019

pocokim Aug 22, 2019 •

edited

Loading

crongro Aug 21, 2019

crongro Aug 21, 2019

crongro Aug 21, 2019

crongro Aug 21, 2019

pocokim commented Aug 26, 2019

crongro left a comment

		@@ -0,0 +1,15 @@
		import React, { createContext, useContext, useReducer } from "react";

		@@ -0,0 +1,15 @@
		import React, { createContext, useContext, useReducer } from "react";

		export const StateContext = createContext();

		const [{ todos,newTodo }, dispatch] = useStateValue();


		const handleChangeInput = ({ target: { value } }) => {

[wook] Step18 리뷰 부탁드립니다. #27

[wook] Step18 리뷰 부탁드립니다. #27

Conversation

pocokim commented Aug 21, 2019

구현사항

느낀점

1. state를 제어하는 함수들은 줄일 수 없으며 어떻게 정리할것인가가 포인트

2. 무엇을 props로 넘겨줄것인가

3. 디버깅

데모링크

crongro commented Aug 21, 2019

crongro 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

pocokim Aug 22, 2019 • edited Loading

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

pocokim commented Aug 26, 2019

crongro left a comment

Choose a reason for hiding this comment

pocokim Aug 22, 2019 •

edited

Loading