[Fix] Feature refactoring booknote #59

joohaem · 2022-12-22T04:29:56Z

📌 나 이런 거 했어요

🔗 영상

Feature refactor booknote kkm #58 :: 이전 카톡과 PR 참고하여 진행하였습니다 :)) 감사해요
- 이후에 진행해주신 변수명 수정 및 forwardRef 작업 통합하도록 하겠습니다!
꼬리질문 추가/삭제(C/D) 로직 버그를 해결하였습니다
1. mapping 에서의 key 값을 index로써 설정해서, 리스트 삭제 및 추가가 뒤엉켰던 이유였습니다
2. 이를 msw 라이브러리로 mocking 하여 기존 데이터에서 id가 유일한 데이터를 통신해서 개발을 진행했습니다
3. id 값을 모두 유일하게 설정해주고, append 할 때에는 Math.random() 으로 유일한 id 를 설정해서 추가하였습니다
4. 위 이유로, post 보낼 때에도 우리가 유일한 id를 설정해서 보내주므로 서버팀에게 공지가 필요할 듯 싶습니다! (💌 @holmir97 :: root/question/answer 데이터들의 DTO(?)에 id 값 추가가 필요할 것 같고, post 할 때에도 id가 붙어서 갈텐데 이에 대한 예외처리도 필요하지 않을까 생각해서 일단 태그 드려요 !!! 이후에 다시 전달드릴게요 💌)
꼬리질문 추가/삭제(C/D)만 확인되었고, 데이터 저장 및 독서 전/중 이동은 확인하지 못했어요! 오늘 회의 때 이야기해보도록 해요

📌 나 이런 거 알게 되었어요

next js 에서 서버 사이드 렌더링 기능때문에 브라우저 상에서의 동작인지, node 상에서의 동작인지를 구별하여 msw 를 실행시킵니다. 이에 대해 typeof window === "undefined" 조건으로써 분기처리하여 진행됩니다
- Next.js 가 웹 페이지를 렌더링 할 때 Node.js는 초기에 window나 document 전역객체가 선언되지 않아, 해당 변수를 참조할 수 없다. 이를 해결하기 위해 Next에서는 dynamic 등을 지원한다

📌 나 이런 거 궁금해요

useFetchBookNote 같은 함수들의 pre/peri 의 분리가 필요할 것 같아요 (타입지정 등을 명확히 함으로써 이후의 유지/보수가 원활해질 것이라고 생각합니다!)
- 현재 제너릭 타입으로써 pre/peri 를 공통으로 이용하는 것도 하는건데, 페이지 로드 시 - 데이터 Fetching 시 - 데이터 업데이트 시의 데이터 구조가 모두 다른 걸 확인했어요! 수정이 필요할 것 같아요
  -현재 swr 로 get 요청들에 대한 hook 이 모델링 되어있는데 (게다가 북노트는 그냥 axios 통신) 모델 정의들이 필요할 것 같아요! react-query 와 같은 동작을 할 수 있다고 하는데, swr 모델링은 어떻게 하는 게 좋을지 공부가 필요할 것 같네요
saveStatelessPeriNoteData 함수로써 useForm 으로 관리되는 데이터를 UI에 보여지는 state로 업데이트하고 -> PATCH 하는 로직으로 구성되어있는데, 이 saveStatelessPeriNoteData 함수의 정의를 명확히 할 것이 필요해 보여요. addChildAnswer 에서도 쓰였었는데, 여기서 에러가 좀 터져서 걷어낸 상태입니다! 🔗 react 레포 코드

Gyuminn · 2022-12-22T12:58:23Z

웹 파트장 폼 미쳤다...

soryeongk

약 9개월의 고통을 해결해주신 갓주함... 사랑합니다...

soryeongk · 2022-12-22T13:08:14Z

util/bookNoteTree.ts

@@ -21,7 +21,8 @@ export const deepCopyTree = (root: PeriNoteTreeNode): PeriNoteTreeNode => {

 export const getNodeByPath = (node: PeriNoteTreeNode, path: number[]): PeriNoteTreeNode => {
  if (node === undefined) {
-    throw new Error("something wrong getting node by path");
+    // throw new Error("something wrong getting node by path");
+    console.log("something wrong getting node by path");


Error는 일단 던지고 사용하는 쪽에서 핸들링하는 것이 더 좋을 것 같아요!

soryeongk · 2022-12-22T13:09:39Z

components/bookNote/periNote/ChildQANode.tsx

@@ -70,7 +70,6 @@ export default function ChildQANode(props: ChildQANodeProps) {
        <StInputWrapper isquestion={isQuestion}>
          <StInput
            {...formController.register(inputKey)}
-            defaultValue={node.content}


예전에 formController쓸 때 모종의 이유로 에러가 나서 추가했던 것으로 기억하는데, 혹시 모르니 주석으로 남겨주실 수 있을까요?

@soryeongk
혹시
useUpdatePeriNote 의 urgentQuery 를 Uncontrolled Input 에서 사용하는 이유가 있나요??
해당 훅이 Uncontrolled 로 관리하는 form 을 Controlled 로 바꾸어서
결국엔 이도저도 아닌 form 이 되는 것 같아요

useUpdatePeriNote 의 urgentQuery 를 Uncontrolled Input 에서 사용하는 이유가 있나요??

이것도 이유를 경험했네요 ...

각각을 state로 관리하지 않을 시에,
사이답변을 추가했을 때 이미 저장되어있던 useForm의 path 값으로 값이 채워진 상태로 생성이 되네요..

근본적인 해결이 필요할 것 같아요

soryeongk · 2022-12-22T13:13:00Z

components/bookNote/periNote/PeriNote.tsx

@@ -34,13 +34,22 @@ interface PeriNoteProps {

 const initialPeriNoteData: PeriNoteData = {
  answerThree: {
+    id: `${Math.random()}11`,


애증의 id 군요... uuidv4를 써보면 어떨까요?

import { v4 as uuidv4 } from "uuid"; ... const id = uuidv4();

soryeongk · 2022-12-22T13:17:25Z

components/bookNote/periNote/PeriNote.tsx

-    const newRoot = isAddAnswer ? saveStatelessPeriNoteData() : deepCopyTree(data.answerThree);
+    // FIX :: const newRoot = isAddAnswer ? saveStatelessPeriNoteData() : deepCopyTree(data.answerThree);
+    const newRoot = deepCopyTree(data.answerThree);


state가 변경될때(특히 블록이 추가될 때) stateless하게 관리되고 있던 아이들을 한 번 state 속에 저장하지 않으면, 중간에 수정한 내용이 반영되지 않을 수도 있을 것 같아요! 테스트 부탁드립니다 :D
꼬리질문을 여러개 작성한 뒤에 큰 답변을 추가해보는 정도?

@soryeongk
현재 코드로는 꼬리 질답 추가/삭제 시에 stateless input 을 추가/삭제 한 후에, setData를 해주기 때문에 데이터 유실은 일어나지 않고

saveStatelessPeriNoteData 는 쓰여진 "내용"을 업데이트 해주는 거라, addChild 에 쓰이지 않아도 괜찮을 것 같아요!!

saveStatelessPeriNoteData 함수는 다음 시점에만 동작합니다!

자동 저장

독서 완료

soryeongk · 2022-12-22T13:18:16Z

components/bookNote/periNote/ChildQANode.tsx

@@ -40,7 +42,7 @@ export default function ChildQANode(props: ChildQANodeProps) {
      e.preventDefault();
      // 꼬리질문과 답변은 자신의 아래에 추가하는 것이 아닌 자신의 부모의 children에 추가해야함
      if (isQuestion) onAddChild(path.slice(0, -1));
-      else onAddChild(path.slice(0, -1), index + 1);


joohaem · 2022-12-24T08:35:22Z

last 5 commit :: #58 작업 토대로 변수명 수정 작업

Gyuminn

웹 파트장 폼 미쳤다

Gyuminn · 2022-12-26T18:20:47Z

components/bookNote/periNote/PeriNote.tsx

@@ -134,7 +134,8 @@ export default function PeriNote(props: PeriNoteProps) {
    // 삭제할 때는 자신의 부모를 찾아서 children을 제거
    const parent = getNodeByPath(newRoot, path.slice(0, -1));

-    parent.children.splice(path[path.length - 1], 1);
+    // parent.children.splice(path[path.length - 1], 1);
+    parent.children[path[path.length - 1]] = { ...parent.children[path[path.length - 1]], type: "deleted" };


이 부분에서 삭제된 노드는 type을 deleted 처리해 주는 것이로군요!

Gyuminn · 2022-12-26T18:22:31Z

components/bookNote/periNote/ChildQANode.tsx

@@ -1,7 +1,7 @@
 /*
 마지막 편집자: 22-06-21 joohaem
 변경사항 및 참고:
-  - path: [0, 0, 0, ...], [0, 0, 1, ...] 
+  - pathStack: [0, 0, 0, ...], [0, 0, 1, ...] 


저의 클로즈된 pr에서 참고해주신거 맞죠..? 눈물이 주르륵 흐르네요 감사합니다!

Gyuminn · 2022-12-26T18:24:05Z

components/bookNote/periNote/PeriNote.tsx

@@ -15,6 +15,7 @@ import { css } from "@emotion/react";
 import styled from "@emotion/styled";
 import React, { useEffect, useState } from "react";
 import { useForm } from "react-hook-form";
+import { v4 as uuidv4 } from "uuid";


랜덤 메서드가 아닌 바로 uuid를 적용시켜 주셨군요 멋지십니다

Gyuminn · 2022-12-26T18:31:57Z

components/bookNote/periNote/ChildQANode.tsx

+    if (isDeleted) return;
+
    is4Depth && formController.setFocus(formPathKey);
  }, []);

-  if (node.type === "deleted") return <></>;
+  if (isDeleted) return <></>;


제가 이해한 것으로는, 삭제 시에 객체에서 정말로 지워지는 것이 아니라 삭제 표시만 해준다는 것이 맞을까요!
그렇다면 삭제를 하더라도 데이터 자체는 잔재하고 이를 POST 시에도 서버에 넘겨주는 것이 맞는지 확인부탁드립니다!

우선 이런 방법을 생각해내신 것 자체에 경의를 표합니다. 고생하셨어요

제가 이해한 것으로는, 삭제 시에 객체에서 정말로 지워지는 것이 아니라 삭제 표시만 해준다는 것이 맞을까요!
그렇다면 삭제를 하더라도 데이터 자체는 잔재하고 이를 POST 시에도 서버에 넘겨주는 것이 맞는지 확인부탁드립니다!

둘 다 맞습니다 :) !!! 최선의 방법인지는 더 봐야하지만요

joohaem · 2022-12-29T05:57:35Z

💌 Slack 내용과 동일합니다

대충 해결된 것 같은데, 에러 해결의 2가지 방안에 대해서 공유하려구요!

우리가 계속 Error 나면서 터졌던 이유가, 유일하지 않은 key 값 때문도 있는데
이유가 하나 더 있었어요

saveStatelessPeriNoteData (useForm의 데이터를 UI state에 동기화 시켜주는 함수) 안에서,
getNodeByPath (useForm으로 저장된 path를 통해 node들을 확인하고 내용을 업데이트 하여 setState하는 함수) 를 도는 과정에!
삭제가 되고 나서의 객체의 순서와 / useForm 에 저장되어 있는 path 값이 달라서
해당 path에 node가 없어 undefined가 들어오고, Error를 내뱉는 거죠

ex) 1, 2, 3, 4 순서에서 3을 제거 했을 때 --> 4는 객체의 3번째 순서이지만 / useForm에서는 4의 path로 "관리되던" 게 남아있어서 문제였죠!

이를 어떻게 해결하냐 ~ 하면 또 2가지 방법이 있는데

삭제할 때 그 아래 path 로 관리되는 useForm을 지워준다
삭제하는 객체를 지우지 말고, "삭제 표시"만 해준다
정도가 있을 것 같아요 (아니면 뭐... 링크드리스트나 우리가 계속 얘기하던 방법들)

사실 1번으로 하는 게 깔끔하려나 했는데, 이는 useForm 관련된 로직이니 @웹_령이 가 한 번 어떤지 생각해주시면 좋을 것 같구
일단 빨리 해결해놓으려고 2번으로 해놨어요

(몇 번에 어느 사이드 이펙트가 있을지는 잘 모르겠네요:joy::joy:)

joohaem · 2023-01-21T01:24:36Z

북노트의 비제어 컴포넌트를 모두 제거하였습니다 (23.01.21)

#68

joohaem added 9 commits December 21, 2022 11:14

fix: prevent throw error

1cb400a

fix: remove defaultValue on reactTextareaAutosize

c7297bc

init: MSW on Next js

61d2cea

fix: 질문/답변 노드별 id property 추가

e2d3020

etc: delete test code

013b545

fix: saveStatelessPeriNoteData (useForm -> setData) 시점 분리

f5993df

fix: modify adding child index

3482a5c

fix: append id property all of PeriNoteTreeNode

49e1aad

fix: start msw worker on development env

1c60553

joohaem added 🚧 refactor 🛠️ fix 🐶주함 labels Dec 22, 2022

joohaem added this to the 기존 기능 마이그레이션 후 1차 배포 milestone Dec 22, 2022

joohaem requested review from soryeongk and Gyuminn December 22, 2022 04:29

joohaem self-assigned this Dec 22, 2022

This was linked to issues Dec 22, 2022

[BookNote] 세부 동작 점검 #40

Closed

[Hotfix/BookNote] 재귀 컴포넌트 버그 #55

Closed

init: add organizeImports

d10e2da

soryeongk reviewed Dec 22, 2022

View reviewed changes

joohaem added 9 commits December 23, 2022 09:13

style: rename variable

c6004e0

etc: 주석 추가/제거

d1316a1

init: yarn add uuid

2cc9b5b

feat: change random ids to uuidv4

3e90912

etc: saveStatelessPeriNoteData() 시점 확인

94df698

etc: 콘솔 제거

20de869

fix: 삭제 노드 표시하여 UI 제거

553f2bc

fix: UnControlled Input의 state 값 제거

d7a0e48

bug: setFocus undefined 에러

c18b9ba

joohaem added 5 commits December 23, 2022 11:14

fix: 독서노트에서의 deleted 노드 처리

ea9e002

style: 변수명 변경 (path -> pathStack)

d04a575

refactor: separate peri note's initial data

69399bf

style: 변수명 변경 (data -> periNoteData)

a95d9f3

fix: correct link href

d8ec66f

Gyuminn reviewed Dec 26, 2022

View reviewed changes

joohaem mentioned this pull request Dec 29, 2022

[Refactor] Feature booknote saving #60

Merged

joohaem changed the title ~~Feature refactoring booknote~~ [Fix] Feature refactoring booknote Dec 31, 2022

soryeongk approved these changes Jan 1, 2023

View reviewed changes

soryeongk merged commit 74cba94 into develop Jan 4, 2023

soryeongk deleted the feature-refactoring_booknote branch January 4, 2023 13:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fix] Feature refactoring booknote #59

[Fix] Feature refactoring booknote #59

joohaem commented Dec 22, 2022 •

edited

Gyuminn commented Dec 22, 2022

soryeongk left a comment

soryeongk Dec 22, 2022

soryeongk Dec 22, 2022

joohaem Dec 23, 2022 •

edited

joohaem Dec 23, 2022

soryeongk Dec 22, 2022

soryeongk Dec 22, 2022

joohaem Dec 23, 2022 •

edited

soryeongk Dec 22, 2022

joohaem commented Dec 24, 2022

Gyuminn left a comment

Gyuminn Dec 26, 2022

Gyuminn Dec 26, 2022

joohaem Dec 27, 2022

Gyuminn Dec 26, 2022

Gyuminn Dec 26, 2022

joohaem Dec 27, 2022

joohaem commented Dec 29, 2022 •

edited

joohaem commented Jan 21, 2023

[Fix] Feature refactoring booknote #59

[Fix] Feature refactoring booknote #59

Conversation

joohaem commented Dec 22, 2022 • edited

📌 나 이런 거 했어요

📌 나 이런 거 알게 되었어요

📌 나 이런 거 궁금해요

Gyuminn commented Dec 22, 2022

soryeongk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joohaem Dec 23, 2022 • edited

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

joohaem Dec 23, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joohaem commented Dec 24, 2022

Gyuminn 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

joohaem commented Dec 29, 2022 • edited

joohaem commented Jan 21, 2023

joohaem commented Dec 22, 2022 •

edited

joohaem Dec 23, 2022 •

edited

joohaem Dec 23, 2022 •

edited

joohaem commented Dec 29, 2022 •

edited