Skip to content

Conversation

@mimizae
Copy link
Collaborator

@mimizae mimizae commented Dec 25, 2024

주요 작업 내용

  • Home에서 Feed들을 불러올 때 스크롤 이벤트를 이용해 무한 스크롤을 구현한 기존 방식을 Intersection Observer API를 이용해 구현되도록 리팩토링 했습니다. 아직 피드 수가 많지 않고... 현재 게시글 업로드가... 안 되는 상황이라 테스트는 못해 봤습니다 ㅠ..ㅠ
  • 매 스크롤마다 이벤트가 호출되지 않도록 디바운싱을 적용했습니다.

기타 작업 내용

  • Home의 Feed에서 쓰이던 svg 파일들 중에서 heart-fill와 message-white를 컴포넌트화 된 Heart와 Message로 대체했습니다.

코드 리뷰 포인트

  • 없음

작업 화면

  • 없음

@mimizae mimizae added the refactor Improve code structure and readability label Dec 25, 2024
@mimizae mimizae requested a review from gustn99 December 25, 2024 10:16
@mimizae mimizae self-assigned this Dec 25, 2024
Copy link
Collaborator

@gustn99 gustn99 left a comment

Choose a reason for hiding this comment

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

feeds가 비어 있는 경우 서버 데이터를 3~4번 추가하고(setFeeds((prev)=>[...prev, ...response, ...response, ...response])), 비어 있지 않은 경우 현재 feeds 데이터를 다시 추가하는 방식(setFeeds((prev)=>[...prev, ...prev]))으로 테스트 해 보았을 때 전반적으로 잘 동작하는 것 같습니다! 코멘트 확인하셔서 추가 점검 부탁드립니닷

Comment on lines 242 to 250
{isLikeClicked ? (
<img className="button" onClick={handleLikeButtonClick} src={likeFillBtn} />
<div className="button" onClick={handleLikeButtonClick}>
{/*Heart 컴포넌트 자체적으로 onClick 이벤트 핸들러가 없기에 이벤트 버블링으로 인한 부모 div의 onClick 이벤트가 실행 됨*/}
<Heart isFilled={true} />{' '}
</div>
) : (
<img className="button" onClick={handleLikeButtonClick} src={likeBtn} />
<div className="button" onClick={handleLikeButtonClick}>
<Heart />
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

isFilled 프로퍼티에서만 isLikeClicked에 대한 분기처리를 해 주면 조건부렌더링 없이 코드가 간결해질 수 있을 것 같습니다! 무한스크롤 로직 점검하시면서 이 부분도 같이 수정 부탁드립니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

image

이런 식으로 isLikeClicked 값을 직접 전달해 조건부렌더링을 없앴는데 이를 말씀하신 것 맞나용??

Copy link
Collaborator

Choose a reason for hiding this comment

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

isLikeClicked ? true : undefined 같은 형태를 생각했었는데 요게 더 좋을 것 같네요!! 굿굿입니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

헤헷 .. 😋

Comment on lines 17 to 18
const [reachedEnd, setReachedEnd] = useState(false);
const isFetchingRef = useRef(false);
const feedPageRef = useRef(1);
const [isFetching, setIsFetching] = useState(false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

제가 테스트 해 봤을 때는 isFetching을 state로 관리할 때 (ref로 관리할 때에 비해) 추가적인 api 호출이 있는 것 같습니다. 한번 확인해 보시면 좋을 것 같습니다! 추가적인 api 호출이 없더라도, reachedEnd나 isFetching 값을 직접 렌더링할 필요가 없기 때문에 ref로 관리하는 편이 좋지 않을까 싶기는 합니다

추가로 reachedEnd보다는 isReachedEnd가 변수명명규칙에 보다 적합할 것 같습니다! 제가 리팩토링할 때 놓쳤네요 ㅠㅠ... 수정해 주시면 감사드리겠습니다!!!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오호!! 일단 isReachedEnd로 수정했습니다 ㅎㅎ 스크롤 시 api 중복 호출에 대해서는 디바운스를 적용시켜 해결했다 생각했는데 이와 별개로 state 업데이트로 인해 발생하는 추가적인 api 호출은 해결이 안 되는군요. . . ref로 관리하는 로직으로 수정했습니다!!

Copy link
Collaborator

Choose a reason for hiding this comment

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

observer 관련 useEffect 의존성 배열에 기존에는 isFetching, reachedEnd가 있었는데, isFetching.current, isReachedEnd.current가 안 들어가도 괜찮은가용?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

저도 처음에는 그대로 의존성 배열에 추가했었는데 좀 더 찾아보니 안 넣어도 될 것 같더라고용!!! useEffect는 의존성 배열의 값이 바뀔 때마다 다시 실행되는데 ref의 current 값은 React 상태(state)가 아니기 때문에 useEffect가 이를 감지하지 못하고 (React 상태처럼 렌더링 사이클에 영향을 주지 않으므로) 그 값이 변경 되어도 useEffect가 다시 실행되지 않으니 굳이 넣을 필요 없다고 생각했습니다

제가 본 velog 첨부하겠습니답
참고자료

Copy link
Collaborator

Choose a reason for hiding this comment

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

우왓 좋은 자료 감사합니다! 의존성배열이 비어 있으면 초기 렌더링 이후에는 스크롤 감지가 되지 않는 것이 아닌가 싶어 찾아봤는데, 초기 렌더링 시에 생성된 IntersectionObserver 객체에서 자체적으로 콜백을 관리하여 의존성 배열이 비어 있어도 문제가 없는 것 같네요~~ 고생하셨습니닷 머지할게요!

// 전체 게시글(피드) 조회 API
const getPostList = async () => {
if (reachedEnd) return;
if (reachedEnd || isFetching) return;
Copy link
Collaborator

Choose a reason for hiding this comment

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

기존에 response.data.post의 길이가 0일 때, 즉 page가 일정 값을 넘어가 더 이상 추가 게시글 데이터가 오지 않을 때 reachedEnd를 true로 설정했는데, 이때 응답 객체가 response.data.posts로 오고 있어 제대로 동작하지 않고 있습니다 혹시 테스트하실 때 혼란이 생길까 봐 말씀드립니다! 대신 isSuccess가 false로 오고 있기 때문에 테스트 시 참고하시면 좋겠습니닷

@gustn99 gustn99 merged commit b0318cc into dev Dec 26, 2024
1 check passed
@gustn99 gustn99 deleted the feat/OD-151 branch December 26, 2024 04:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Improve code structure and readability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants