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

Feat/lazyloading #20

Merged
merged 10 commits into from
Sep 16, 2024
Merged

Feat/lazyloading #20

merged 10 commits into from
Sep 16, 2024

Conversation

jjh4450
Copy link
Collaborator

@jjh4450 jjh4450 commented Sep 15, 2024

관련 Issue

PR 설명

라우팅 요소를 기준으로 lazy 로딩을 적용했습니다.
폰트 최적화, 번들링 최적화도 같이 수정했으며, 아래에 코드와 함께 자세한 사항을 적어 놓았습니다.

  • 설명

기타 사항

#18 먼저 pr 부탁드립니다.
모든 항목에 대해 자체적인 테스트는 완료했습니다.

  • 번들링 정상 작동 여부
  • 폰트 적용 여부
  • 폰트 성능 개선 여부
  • 번들링 용량 최적화 여부
  • 18번 이슈 해결 여부

Fast refresh only works when a file only exports components. Use a new file to share constants or functions between components 경고를 수정함
Copy link

vercel bot commented Sep 15, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
knuland ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 16, 2024 5:27am

@jjh4450
Copy link
Collaborator Author

jjh4450 commented Sep 15, 2024

#18 리뷰 후 머지 되면, 받고 자체적으로 머지후 다시 pr 걸겠습니다. 우선 괜찮은 변경인지 검토 부탁드립니다.

@flareseek
Copy link
Owner

찾아보니 lazy로 로드할때 장점이 많네요. 굳굳

Comment on lines +36 to +38
if (module.includes("react")) {
return "react-vendor";
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

번들링 최적화를 수행했습니다.
과도한 번들링은 오히려 오버헤드를 유발할 수 있을 거 같아, react 관련 라이브러리는 한데 모았습니다.

구현이 어렵지도 않았고, 용량이 그리 크지도 않아 따로 비교는 하지 않았습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

나머지 모듈들은 따로 분리되어 필요할 때 import 됩니다.

@@ -27,7 +22,7 @@ export const vars = createGlobalTheme("#root", {
white: "#FFFFFF",
},
font: {
pretendardRegular,
pretendardRegular: "pretendard, sans-serif", // for debug: ,Courier New", // <-독특함
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

폰트 import 최적화를 수행했습니다.

  • 기존 폰트가 1.1메가로 너무 무거워 이를 서브셋 폰트로 적용했습니다.
    • 카톡방에 충분히 공유된 사항이어서 별도 이슈는 작성하지 않았습니다.
    • 본 브런치의 목적과 크게 다르지 않아 한 번에 포함했습니다.
  • src/index.css에 의존합니다

@@ -0,0 +1,2 @@
@import url("https://cdn.jsdelivr.net/npm/[email protected]/reset.min.css");
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

css 초기화 로직인데 그냥 가져왔습니다.
중복 여부 및 수정 방안 제시 부탁드립니다. @dandamdandam

Copy link
Collaborator

Choose a reason for hiding this comment

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

https://github.com/jjh4450/KNU-festival24/blob/874601fc3ba4f9dd53864e3a297d9ec95f00c438/index.html#L7

이전에 작성해둔 건데, index.css에서 관리하는게 더 좋아보입니다. index.html에 위 코드 삭제해주세요!

@jjh4450 jjh4450 requested a review from flareseek September 16, 2024 03:56
Copy link
Collaborator

@dandamdandam dandamdandam left a comment

Choose a reason for hiding this comment

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

최적화 신경 많이 쓰셨네요! 라우터 객체화 한 것도 확인했습니다 감사합니다!

index.html에 중복되는 코드만 삭제하고 머지해주시면 될 것 같아요.

@dandamdandam
Copy link
Collaborator

그리고 이전 PR에서 말하려고 했는데 지금 닫혀서..
image
여기 로고 부분 디자인 상 삭제되어서 없애도 될 것 같아요.

@jjh4450
Copy link
Collaborator Author

jjh4450 commented Sep 16, 2024

그 부분 디자인 팀 한분이랑 어제 예기하는데 별 지적 없어서 넘겼었습니다. 헤더 전반적으로 수정하면서 디자인 팀이랑 얘기 해볼게요

@jjh4450 jjh4450 merged commit 4557bd4 into flareseek:develop Sep 16, 2024
3 checks passed
@jjh4450 jjh4450 deleted the feat/lazyloading branch September 16, 2024 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants