[FE] feat: 사용자가 작성하던 리뷰를 로컬 스토리지와 연동하고, 복원하는 기능 추가 #987
Conversation
…가, 그에 따른 NavigateBlockerModal 삭제
…eview-me into fe/feat/893-store-review-on-progress-in-localstorage
|
테스트 터지네요 🤣 확인해볼게요 |
Deploying 2024-review-me-release with
|
| Latest commit: |
339c387
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c463c4f8.2024-review-me-release.pages.dev |
| Branch Preview URL: | https://fe-feat-893-store-review-on.2024-review-me-release.pages.dev |
| answers: 'answers', | ||
| visitedCardIdList: 'visitedCardIdList' | ||
| } as const; | ||
|
|
There was a problem hiding this comment.
기존에 LOCAL_STORAGE_KEY라는 변수가 있는데 따로 만든 이유가 있나요?
There was a problem hiding this comment.
파일명이 storageKey라서, 일단 이 상수 객체가 어떤 용도인지 간접적으로 알 수 있다고 생각했어요. 그렇다면 모든 key값들을 하나의 객체에 몰아두기보다 용도별로 다른 객체를 사용하는 게 좋을 것 같아 분리했습니다.
다만 이 파일대로라면 '그럼 STORED_DATA_NAME은 로컬 스토리지 키가 아닌가?' 하는 혼동이 올 수 있겠네요 😅
스토리지에 저장할 값이 크게 늘어날 일은 별로 없을 것 같아 LOCAL_STORAGE_KEY 하나로 관리하는 것도 괜찮아 보이기는 하는데, 바다의 자세한 의견이 궁금합니다~!
There was a problem hiding this comment.
개인적으로 LOCAL_STORAGE_KEY 객체 하나로 관리하는 것이 좋을 것 같아요.
처음에 해당 폴더로 들어오게 되면 STORED_DATA_NAME, LOCAL_STORAGE_KEY, SESSION_STORAGE_KEY가 있는데 STORED_DATA_NAME이 로컬 스토리지 키인지? 다른 키인지? 헷갈릴 것 같아요. 저 네이밍만 보고 로컬 스토리지 키라는 걸 추측해야 하니까 하나로 관리하는 것이 좋을 것 같아요!
만약 용도별로 나누고 싶다면 아래처럼 해도 괜찮을 것 같아요😊
export const LOCAL_STORAGE_KEYS = {
userSelections: {
selectedCategories: 'selectedCategories',
answerValidations: 'answerValidations',
answers: 'answers',
visitedCardIdList: 'visitedCardIdList',
},
highlights: {
isHighlightEditable: 'isHighlightEditable',
isHighlightError: 'isHighlightError',
},
}
``
| [key: string]: boolean; | ||
| export type Modals = Record<string, boolean>; | ||
|
|
||
| interface useModalsProps { |
There was a problem hiding this comment.
앞으로의 불상사를 막기 위해 eslint에 @typescript-eslint/naming-convention을 추가했습니다... ^_ㅠ
| @@ -34,6 +40,34 @@ const useMultipleChoice = ({ question, handleModalOpen }: UseMultipleChoiceProps | |||
| }, | |||
| ); | |||
|
|
|||
| interface findSelectedOptionIdsParams { | |||
There was a problem hiding this comment.
타입명은 앞에 대문자로 시작해주세요.
| const findSelectedOptionIds = ({ answerMap, questionId }: findSelectedOptionIdsParams) => { | ||
| if (!answerMap) return null; | ||
|
|
||
| for (const [, value] of answerMap) { |
There was a problem hiding this comment.
answerMap의 key가 questioId라서 반복문 필요 없이, answerMap.get(quertionId)로도 selectedOptionIds를 찾을 수 있어요.
| @@ -15,6 +15,10 @@ const useOptionSelection = () => { | |||
| checked: boolean; | |||
| } | |||
|
|
|||
| const initSelectedOptionList = (newSelectedOptionList: number[]) => { | |||
There was a problem hiding this comment.
init이라는 단어가, 초기화 의미라서 선택 문항들이 아무것도 선택되지 않은 초기값 형태가 생각나네요. 구현의도를 보면 로컬 스토리지에 저장된 값으로 상태를 업데이트하는 거라서, 함수명이 구현의도를 더 잘 담았으면 좋겠어요.
| }; | ||
|
|
||
| // 저장된 객관식 답변이 있다면 복원 | ||
| useEffect(() => { |
There was a problem hiding this comment.
선택된 객관식 문항 상태를 관리하는 훅인 useOptionSelection이 아니라, useMultipleChoice에서 로컬 상태에 저장된 값에서 해당 질문에서 선택된 객관식 문항들을 useOptionSelection으로 넘겨주는 플로우를 선택한 이유가 있나요?
리뷰 작성 페이지를 리팩토링할 때, 구조가 복잡해져서 최대한 훅,컴포넌트의 역할을 작게 나누려고 했어요. useOptionSelection은 선택된 객관식 문항 상태를 관리하고, useMultipleChoice는 객관식의 답변 선택 액션과 useOptionSelection에서 관리하는 상태를 change이벤트에서 업데이트할 수 있게 하는 역할로 구현했어요.
제가 봤을때는, 책임분리에 따르면 useOptionSelection에서 로컬 스토리지에 저장된 데이터를 바탕으로 selectedOptionList의 상태를 업데이트하는 게 좋을 것 같아요.
There was a problem hiding this comment.
선택된 객관식 문항 상태와 관련된 로직이라 저도 useOptionSelection에서 복원하는 것이 나을 것 같다 생각합니다
| !!localStorage.getItem(`${STORED_DATA_NAME.selectedCategories}_${reviewRequestCode}`) || | ||
| !!localStorage.getItem(`${STORED_DATA_NAME.answers}_${reviewRequestCode}`) || | ||
| !!localStorage.getItem(`${STORED_DATA_NAME.answerValidations}_${reviewRequestCode}`), |
There was a problem hiding this comment.
코드기 길어서, 가독성을 위해서 따로 변수로 만들어주면 좋을 것 같아요
| const storedAnswers = localStorage.getItem(`${STORED_DATA_NAME.answers}_${reviewRequestCode}`); | ||
|
|
||
| const selectedCategories = storedSelectedCategories ? JSON.parse(storedSelectedCategories) : null; | ||
| const answerValidations = storedAnswerValidations ? new Map(JSON.parse(storedAnswerValidations)) : new Map(); |
There was a problem hiding this comment.
답변 유효성과 강점 선택을 로컬 스토리지에 담지 않는 것이 어떨까요?
사용자가 로컬 스토리지의 데이터를 변경해서, 답변과 답변 유효성, 강점 선택 사이의 데이터 불일치가 일어날 가능성이 높아요. 그래서 로컬 스토리지에는 답변만 담은 후 로컬 스토리지에 저장된 답변을 바탕으로 답변 유효성과 강점 선택 여부를 검사한 후 전역 상태에 업데이트하는 게 맞다고 봐요.
아마 이렇게 변경한다면, 답변 유효성을 검사하는 로직을 객관식과 주관식 답변을 관리하는 훅에서 따로 빼서 재사용가능한 유틸함수로 바꾸어야할거에요.
There was a problem hiding this comment.
사용자가 로컬 스토리지의 데이터를 변경 하는 동작이 말 그대로 로컬 스토리지 값을 임의로 수정한다는 의미일까요?
로컬 스토리지 값을 마음대로 수정하면 사용자에게만 불이익이 있을 뿐이라, 결제 시스템처럼 데이터 정합성이나 보안이 중요하지 않으니 이 점은 고려하지 않아도 된다고 생각했습니다.
실시간 업데이트 도중 데이터 동기화 문제가 생길 수 있다는 의미인지 궁금합니다. 동기화 리스크는 실시간 저장이라면 있을 수밖에 없지만 저장하는 데이터가 많아지면 더 리스크가 커지겠네요.
전자의 의미도 있다면 로컬 스토리지 사용 자체가 위험한 것 같아서요!
하지만 이것과 별개로 로컬 스토리지에 저장하는 값을 줄이면 좋겠다는 점은 동감합니다 😂
실시간으로 저장하는 값이 많아질수록 동기화 리스크가 커지니까요.
로컬 스토리지에 접근하는 동작은 서버 통신보다 빠르고 안정적이긴 하지만, 사용자에게 날것의 데이터를 (많이) 노출한다는 것부터 좀 꺼려지는 것도 사실이라서...
구현 초창기에 일단 전역 상태들은 어차피 전역이니까~ 싶어서 다 로컬에 저장하고 시작했더니 점점 값이 많아졌네요 ㅠ_ㅠ
다만 지역 상태를 사용하는 컴포넌트들은 복원이 까다로워서(예: 프로그레스 바) 상태를 로컬 스토리지에 저장해뒀습니다.
코멘트에서 언급된 값들이 전역 상태로 관리하는 답변 유효성과 선택된 강점인데 리팩토링 대상에서 이런 지역 상태들은 제외된다고 봐도 될까요? 아니면 answerMap을 제외한 모든 데이터일까요?
일단 바다의 제안대로 모든 값의 근간인 answerMap과 지역 상태들만 저장/복원한 다음 차차 연관된 상태들을 업데이트해주는 방향으로 리팩토링해보겠습니다!
| }); | ||
|
|
||
| const getCurrentSelectedCategory = useCallback(() => { | ||
| if (!selectedCategory || selectedCategory.length === 0) return null; |
| const getCurrentAnswerValidation = useCallback(() => { | ||
| if (!answerValidation || answerValidation.size === 0) return null; | ||
|
|
||
| const plainObjectAnswers = Array.from(answerValidation.entries()); |
There was a problem hiding this comment.
Array.from(answerMap.entries())도 plainObjectAnswers라고 표현하던데,
변수명이 plainObjectAnswers인 이유가 있나요?
| /** | ||
| * 리뷰와 관련된 데이터들을 실시간으로 로컬 스토리지에 저장하는 훅 | ||
| */ | ||
| const useSaveReviewToLocalStorage = () => { |
There was a problem hiding this comment.
atom의 effects를 사용하지 않고, useEffect로 로컬 스토리지에 저장하는 훅을 만든 이유가 있나요?
|
|
||
| const [text, setText] = useState(''); | ||
| const [errorMessage, setErrorMessage] = useState(TEXT_ANSWER_ERROR_MESSAGE.noError); | ||
|
|
||
| // 로컬 스토리지에 저장했던 답변으로부터, questionId를 통해 해당 질문의 서술형 답변을 찾는 함수 | ||
| // TODO: 복원을 위한 find 함수들을 별도 유틸로 분리 및 통합 | ||
| interface findTextAnswerParams { |
| setAnswerMap(newAnswerMap); | ||
| setAnswerValidationMap(newAnswerValidationMap); | ||
| }, [cardSectionList]); | ||
| // answerMap이 비어 있을 때만 작성 페이지 초기화 작업 수행 |
There was a problem hiding this comment.
(.... 노트북 포맷했는데, env 저장한 usb를 집에 놓고 와서 로컬에서 실행이 안되네요. 이 부분은 집에 가서 다시 확인할게요...)
| @@ -16,11 +16,6 @@ const useNavigateBlocker = ({ openNavigateConfirmModal }: UseNavigateBlockerProp | |||
| return [...answerMap.values()].some((answer) => !!answer.selectedOptionIds?.length || !!answer.text?.length); | |||
There was a problem hiding this comment.
로컬 스토리지에 저장하는 기능이 들어온다면, 이 훅은 삭제해도 괜찮지 않을까요?
There was a problem hiding this comment.
뭔가 바다의 훌륭한 훅을 지우기 아까워서... 남겨놨는데 역시 지워야겠죠 ...
There was a problem hiding this comment.
제보
default.webm
리뷰 작성 시 카테고리 선택 후 모두 해제할 때, local storage가 비워지지 않는 문제를 발견했어요. 나갔다가 다시 들어왔을 때 복원하면 체크되는 것은 없지만 프로그레스 바에는 추가되고 있어서, 이 부분은 수정해야 할 것 같아요!
추가로, 프로그레스 바가 활성화되지 않고 있어요! visitedCardIdList가 업데이트되고 있지 않아서 isMovingAvailable이 항상 false 상태입니다. 이 부분은 로컬에서 코드를 수정해서 확인해봤는데, 해당 부분에 코멘트 따로 남겨두었어요.
| export const STORED_DATA_NAME = { | ||
| selectedCategories :'selectedCategories', | ||
| answerValidations: 'answerValidations', | ||
| answers: 'answers', | ||
| visitedCardIdList: 'visitedCardIdList' | ||
| } as const; | ||
|
|
There was a problem hiding this comment.
속성값을 불변으로 유지하고, 겸사겸사 타입 추론도 각 문자열 리터럴로 해 줘서 상수라는 의미에 적합한 것 같았습니다.
확실히 기존에는 상수를 선언할 때 as const를 사용한 코드가 없었던 것 같은데, 혹시 제가 기억하지 못하는 이슈가 있었나요?
| }; | ||
|
|
||
| // 저장된 객관식 답변이 있다면 복원 | ||
| useEffect(() => { |
There was a problem hiding this comment.
선택된 객관식 문항 상태와 관련된 로직이라 저도 useOptionSelection에서 복원하는 것이 나을 것 같다 생각합니다
| interface useCardFormModalProps { | ||
| initialStates: Modals; | ||
| } |
| }, [visitedCardIdList]); | ||
|
|
||
| useEffect(() => { | ||
| if (cardSectionList.length === 0 || visitedCardIdList.length === 0) return; |
There was a problem hiding this comment.
제보 관련
visitedCardIdList는 처음에 비어있기 때문에 두 번째 조건으로 인해 visitedCardIdList가 업데이트되지 않고, 프로그레스 바의 모든 버튼이 비활성화 되어 있어요. return 조건을 다시 고려해보면 좋을 것 같아요!
두 번째 조건을 지워보았는데, 복원 시 사진과 같이 꼬리 질문 카드는 복원되지 않네요. local storage에 저장될 때는 꼬리 질문 카드의 Id가 잘 들어가는데, 복원 시 사라져요.
2.webm
저도 문제의 원인을 찾아보고 공유할게요. 우선은 제보합니다!
There was a problem hiding this comment.
에프이가 코멘트 달아줬군요! 지금 코드로는 프로그레스 바 버튼이 활성화가 제대로 되고 있지 않아요.
혹시 아래 if문을 추가하신 이유가 있을까요??
if (cardSectionList.length === 0 || visitedCardIdList.length === 0) return;에프이 말대로 visitedCardIdList 초기값이 빈 값으로 되어 있어서 해당 코드를 추가하면 안 될 것 같아요.
const [visitedCardIdList, setVisitedCardIdList] = useState<number[]>([]);There was a problem hiding this comment.
개발할 때는 상태 충돌이 있었는지(...) 잘 돌아갔는데 저도 이제 프로그레스 바 상태 복원이 불가능하네요 🙃🙃 고쳐보겠습니다...
두 번째 조건을 넣었던 이유는 복원할 때 "값을 복원해옴 -> 그런데 다시 visited 배열의 초기화 로직 실행됨 -> 최종 빈 배열" 이 일어나서 넣었던 것 같아요. 즉 복원한 값을 초기화 로직이 덮어씌워버립니다...초기화 로직을 잘 만져봐야겠네요
There was a problem hiding this comment.
PR과 커밋에서 뭔가 올리의 깊은 고민이 느껴졌어요...🥹
🤔 개인적인 의견
리액트 상태를 로컬 스토리지에 그대로 저장해도 괜찮은가?
로컬 스토리지 사용, 이대로 괜찮은가? 입니다.
뭔가 한편으론 이런 생각이 드네요. 로컬 스토리지를 사용하면 같은 브라우저, 같은 기기에서만 데이터를 유지하는데 만약 노트북에서 리뷰를 작성하다가, 데스크탑이나 모바일로 넘어오게 된다면 저장이 안되는 거잖아요? 이런 경우도 있을 거라 생각해서... 범위를 명확하게 짚고 넘어간다면 좋을 것 같아요!
그렇다고 db에 저장하자니 너무 많은 부담을 주는 것 같아서 개인적으로는 로컬 스토리지에 저장하는 방법이 괜찮을 것 같아요! 로컬 스토리지를 사용하면 기기 간 동기화는 불가능하겠지만 리뷰를 작성하다가 다른 기기에서 이어서 작성할 확률이 그리 크진 않다고 생각하고, 말 그대로 기능이 임시저장이다 보니 가볍게 로컬 스토리지로 저장하는 게 좋을 것 같다는 생각이 드네요.
훅에서 reviewRequestCode를 불러오는 방법
지금 reviewRequestCode를 불러오는 useSearchParamAndQuery 훅을 여러 곳에서 사용하다보니 이 방식에 대해서도 얘기를 나눠봐야 겠네요!
지금 리뷰 작성 페이지에서만 useSearchParamAndQuery 훅을 사용하는 곳이 5군데가 있는데요. 만약 이 훅을 호출하는 곳들이 대부분 동일한 위치에 있다면, 상위 컴포넌트에서 한번만 호출하고 props로 넘겨주는 방식이 더 좋을 것 같아요. 근데 지금은 각기 다른 위치에서 호출되고 있어서 지금의 방식도 괜찮다고 생각은 합니다,,
| answers: 'answers', | ||
| visitedCardIdList: 'visitedCardIdList' | ||
| } as const; | ||
|
|
There was a problem hiding this comment.
개인적으로 LOCAL_STORAGE_KEY 객체 하나로 관리하는 것이 좋을 것 같아요.
처음에 해당 폴더로 들어오게 되면 STORED_DATA_NAME, LOCAL_STORAGE_KEY, SESSION_STORAGE_KEY가 있는데 STORED_DATA_NAME이 로컬 스토리지 키인지? 다른 키인지? 헷갈릴 것 같아요. 저 네이밍만 보고 로컬 스토리지 키라는 걸 추측해야 하니까 하나로 관리하는 것이 좋을 것 같아요!
만약 용도별로 나누고 싶다면 아래처럼 해도 괜찮을 것 같아요😊
export const LOCAL_STORAGE_KEYS = {
userSelections: {
selectedCategories: 'selectedCategories',
answerValidations: 'answerValidations',
answers: 'answers',
visitedCardIdList: 'visitedCardIdList',
},
highlights: {
isHighlightEditable: 'isHighlightEditable',
isHighlightError: 'isHighlightError',
},
}
``| const deleteAllReviewDataInLocalStorage = () => { | ||
| Object.values(STORED_DATA_NAME).forEach((key) => { | ||
| localStorage.removeItem(`${STORED_DATA_NAME[key]}_${reviewRequestCode}`); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
key 가 STORED_DATA_NAME[key]니까 아래처럼 변경해야할 것 같아요!
localStorage.removeItem(`${key}_${reviewRequestCode}`);There was a problem hiding this comment.
쑤쑤가 남겨준 코멘트에 각각 코멘트를 달고 싶은데 바로 작성할 수 있는 곳이 여기밖에 없네요 😂
로컬스토리지
오 확실히 리뷰미 플랫폼에서 로컬 스토리지를 사용할 때의 가장 큰 단점이 디바이스별 상태 공유 불가능이겠네요. 컴퓨터로 쓰다가 중간에 그만두고 다시 모바일로 접속해서 리뷰를 작성할 수도 있을 것 같아서요. 꽤 치명적인 이슈 같은데... 이걸 해결하려면 무조건 서버를 빌려와야 하겠네요 🙃
상수 객체 관련
앞으로 로컬에 저장할 값이 많아지지는 않을 것 같아 그냥 LOCAL_STORAGE_KEY에 합치는 쪽으로 수정할 것 같습니다. 객체 중첩이 생기면 사용하는 쪽에서 길게 체이닝을 해야 해서 아마 심플하게 가져갈 것 같아용
if문 조건
일단 두 번째 조건은, 상태를 로컬 스토리지에서 가져와서 업데이트해놔도 최종적으로 기존의 초기화 로직이 실행돼서 최종 렌더링을 빈 배열로 하던 (=UI에 복원 안 되고 로컬 데이터도 날아감)문제를 해결하기 위해 둔 조건인데 지금은 또 안 먹히네요ㅋㅋㅋㅋㅠ 아마 앞 조건도 비슷한 이유로 놔뒀던 것 같아요. 만든 지 조금 지나니까 기억이 가물가물한데 버그 고치면서 수정해볼게요~
| }, [visitedCardIdList]); | ||
|
|
||
| useEffect(() => { | ||
| if (cardSectionList.length === 0 || visitedCardIdList.length === 0) return; |
There was a problem hiding this comment.
에프이가 코멘트 달아줬군요! 지금 코드로는 프로그레스 바 버튼이 활성화가 제대로 되고 있지 않아요.
혹시 아래 if문을 추가하신 이유가 있을까요??
if (cardSectionList.length === 0 || visitedCardIdList.length === 0) return;에프이 말대로 visitedCardIdList 초기값이 빈 값으로 되어 있어서 해당 코드를 추가하면 안 될 것 같아요.
const [visitedCardIdList, setVisitedCardIdList] = useState<number[]>([]);
🚀 어떤 기능을 구현했나요 ?
🔥 어떻게 해결했나요 ?
로컬 스토리지에 실시간으로 상태 저장
왜 로컬 스토리지인가?
왜 실시간으로 저장하는가?
→ 라우터를 통한 이동이 일어날 때 자동 저장/복원을 시키려면, 사용자가 나갈 때 로컬스토리지와 동기화하는 게 아니라 상태 데이터들을 실시간으로 연동해야 했습니다.
따라서 beforeunload 등 별도의 DOM 이벤트를 사용하지 않고, useEffect만 사용해서 사용자가 선택한 값이 달라질 때마다 로컬 스토리지에 저장하는 방식으로 구현했습니다.
📝 어떤 부분에 집중해서 리뷰해야 할까요?
가장 중요한 것! 리뷰 작성 도중 삭제되는 데이터 관련 논의입니다.
강점 답변을 작성했다가 해당 강점의 선택을 해제할 때, 지금은 저장한 답변이 삭제된다는 안내 모달이 뜹니다.
기존에는 현재 선택한 카테고리의 답변만 저장하고 있었지만, 지금은 작성했던 답변을 다 저장할 수 있는 만큼 이 기능을 유지할지 삭제할지 고민이 됩니다.
선택 해제했던 카테고리 답변이 갑자기 부활하면 놀랄 것 같기도 하고, 편할 것 같기도 하고...
예) 강점 1 선택 → 강점 1에 대한 답변 작성 → 강점 1 선택 해제
를 한 뒤에 다시 강점 1을 선택하고 넘어가보면 기존에 저장했던 답변이 그대로 나옵니다.
리액트 상태를 로컬 스토리지에 그대로 저장해도 괜찮은가?
로컬 스토리지 사용, 이대로 괜찮은가? 입니다.
훅에서 reviewRequestCode를 불러오는 방법
reviewRequestCode가 사실상 전역 변수처럼 사용되고 있어서, 각 훅에서 이게 필요하면 바로 useSearchParamAndQuery 훅으로 끌어다 사용하고 있습니다.
하지만 훅을 호출하는 곳에서 reviewRequestCode를 props로 바로 넘겨줄 수 있다면 넘겨주는 것도 괜찮아 보이는데, 어파치 전역 변수처럼 사용하는 것을 넘겨주는 게 어색한 것 같기도 합니다. 여러분의 의견이 궁금합니다!!
📚 참고 자료, 할 말