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

Refactor/#408 SongDetailListPage의 Youtube Player 스크립트 로딩 성능 개선 #409

Merged
merged 3 commits into from
Sep 14, 2023

Conversation

Creative-Lee
Copy link
Collaborator

@Creative-Lee Creative-Lee commented Sep 12, 2023

📝작업 내용

youtube player 생성 시점을 intersection observer로 제어 하도록 개선
paint 전 단계에 스크롤 이동 하도록 개선

💬리뷰 참고사항

player 스크립트 로드 제어 성공

그리고....캐싱 성공🤪🤪🤪🤪

image

스크립트 로드를 제어하여 1개의 player를 구성하는데 필요한 3개의 js파일을 로드하면서,
이후 요청에 대한 캐시까지 적용되는 모습입니다.

기존에는 21개의 player를 병렬적으로 한번에 로드했기 때문에,
중복되는 스크립트임에도 각 요청시 캐시된 스크립트가 없어, 63개의 js파일을 불러오고, 실행해버리는 대참사가 발생했던 거죠.😅
저희 모두에게 좋은 경험이 된 것 같네요.


개선 전/후 비교

  • 스크립트 컴파일 + 평가 시간

개선 전 - 무수히 많은 스크립트

컴파일+평가

개선 후 - 단 1개의 스크립트

한개의 스크립트만 컴파일+평가

  • Total blocking time

개선 전

토탈 블록킹 타임

개선 후

개선 후 토탈 블록킹 타임

코드에 대한 설명

커밋 메세지 바디에 상세 설명 적어놓았습니다.
callbackRef, observer ref 생성 이유, layoutEffect 사용 이유에 대한 설명이 더 필요하시면 바로 말해주세요~
대화로 설명 드리겠습니다~😀

#️⃣연관된 이슈

close #408

1. loading상태에 따른 조건부 preview 이미지 랜더 및 옵저버 트리거로 사용
2. useEffect 제거 및 callbackRef로 대체
3. 옵저버 메모리 누수 방지를 위한 observer ref 생성
노래 목록의 첫번째 요소의 player 로드가 되는 버그가 있었음.
첫번째 요소의 preview 이미지가 랜더된 후 스크롤 이동하기 때문에 발생한 문제.
paint 되기 전, 스크롤을 이동하도록 하여 수정
@Creative-Lee Creative-Lee added [ 🌞 FE ] 프론트엔드 크루들의 빛나는 개발 이야기 하나둘셋 호! 🔨 Refactor 꾸준한 개선이 더 나은 애플리케이션을 만든다 labels Sep 12, 2023
@github-actions
Copy link

github-actions bot commented Sep 12, 2023

Unit Test Results

  73 files    73 suites   9s ⏱️
287 tests 287 ✔️ 0 💤 0
290 runs  290 ✔️ 0 💤 0

Results for commit 6061ac4.

♻️ This comment has been updated with latest results.

@@ -64,7 +64,7 @@ const SongDetailListPage = () => {
return () => nextObserver.disconnect();
}, [fetchExtraNextSongDetails, songDetailEntries]);

useEffect(() => {
useLayoutEffect(() => {
itemRef.current?.scrollIntoView({ behavior: 'instant', block: 'start' });
}, [songDetailEntries]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

도밥 설명 정리: useEffect로 구현되어 있을 경우, 랜더링 시에 컴포넌트가 real DOM에 붙인 뒤에 scroll이 발생하기 때문에 예상치 못하게 intersectionObserver가 터집니다. 따라서 useLayoutEffect를 사용하여 컴포넌트가 real DOM에 붙기 전에 scroll을 발생시키도록 하였습니다.

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
조금 더 정확히 설명하자면,
위 그림과 같이 react의 dom update는 끝난 상태로,
브라우저가 paint 하기 전! 스크롤 이벤트를 실행해서 1번 노래 요소의 옵저버가 터지지 않도록 개선한것입니다 ㅎㅎ

Copy link
Collaborator

@ukkodeveloper ukkodeveloper left a comment

Choose a reason for hiding this comment

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

작은 코드 수정으로 많은 점이 개선된 것 같네요!! 고생하셨습니다.

@Creative-Lee Creative-Lee merged commit 118894a into main Sep 14, 2023
@Creative-Lee Creative-Lee deleted the refactor/#408 branch September 14, 2023 03:25
@Creative-Lee Creative-Lee restored the refactor/#408 branch December 12, 2023 03:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[ 🌞 FE ] 프론트엔드 크루들의 빛나는 개발 이야기 하나둘셋 호! 🔨 Refactor 꾸준한 개선이 더 나은 애플리케이션을 만든다
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[REFACTOR] SongDetailListPage의 Youtube Player 스크립트 로딩 성능 개선
3 participants