Skip to content

[#889] refactor(view): Summary 컴포넌트 리팩토링#890

Merged
SingTheCode merged 7 commits into
mainfrom
feature/#889_summary-refactor
Sep 23, 2025
Merged

[#889] refactor(view): Summary 컴포넌트 리팩토링#890
SingTheCode merged 7 commits into
mainfrom
feature/#889_summary-refactor

Conversation

@SingTheCode

@SingTheCode SingTheCode commented Sep 18, 2025

Copy link
Copy Markdown
Contributor

📋 관련 이슈

Closes #889
Related #800

🎯 작업 내용

#800 이슈(CSMNode 클릭 시 Stem 닫기 성능 개선)를 효과적으로 구현하기 위한 사전 리팩토링 작업입니다.

📝 변경 사항

컴포넌트 구조 개선 ✨

  • ClusterRow 컴포넌트 추출 (1e704a2)

    • Summary 컴포넌트에서 클러스터 행 렌더링 로직 분리
    • 재사용성 및 유지보수성 향상
    • 코드 복잡도 감소
  • CommitItem 컴포넌트 추출 (8d42770)

    • 커밋 아이템 렌더링 로직을 별도 컴포넌트로 분리
    • 코드 구조 개선으로 가독성 향상
    • 단일 책임 원칙 적용

상태 관리 최적화 🚀

  • Detail 컴포넌트 리팩토링 (1f66707)

    • Props drilling 제거
    • 전역 상태 직접 구독으로 마이그레이션
    • 불필요한 리렌더링 방지를 위한 기반 마련
  • 네이밍 일관성 개선 (34abcc5)

    • selectedClusterIdselectedClusterIds로 변경
    • 복수 선택 지원을 위한 일관된 네이밍
    • 타입 정의 업데이트

개발 환경 개선 🛠️

  • .gitignore 업데이트 (4cfe4d2)
    • Claude 관련 파일 추가
    • 개발 환경 정리

🔍 리팩토링 효과

성능 측면

  • 컴포넌트 분리로 렌더링 최적화 기반 마련
  • 불필요한 리렌더링 방지 구조 개선
  • 메모이제이션 적용 가능한 구조로 변경

유지보수성

  • 단일 책임 원칙에 따른 컴포넌트 분리
  • 코드 가독성 및 테스트 용이성 향상
  • 향후 기능 추가 시 확장성 개선

✅ 테스트 계획

  • 기존 기능 정상 동작 확인
  • 리팩토링된 컴포넌트들의 렌더링 확인
  • 상태 변경 시 올바른 업데이트 확인

📌 다음 단계

이 리팩토링을 기반으로 #800 이슈 구현 예정:

  1. Stem 닫기 애니메이션 시간 300ms 이하로 단축
  2. 애니메이션 중 추가 클릭으로 인한 성능 저하 방지
  3. 기존 기능과의 호환성 유지

📊 변경 통계

 7 files changed, 221 insertions(+), 153 deletions(-)

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

SingTheCode and others added 5 commits September 8, 2025 22:42
… for consistency

- Update ContentProps type to use plural selectedClusterIds
- Fix parameter destructuring in Content component
- Align prop naming with array type usage
…e subscription

- Remove selectedData prop from Detail component
- Add direct useDataStore subscription in Detail.tsx
- Update DetailProps interface to remove selectedData
- Remove selectedData prop passing in Summary component
- Improve null checking for selectedData validation

🤖 Generated with Claude Code (https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
…rganization

- Extract commitNodeList.map logic into separate CommitItem component
- Add CommitItemProps interface for type safety
- Update ESLint config to allow function declarations
- Improve code readability and maintainability

🤖 Generated with Claude Code (https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Extract inline rowRenderer function to separate ClusterRow component
- Add ClusterRowProps type definition for better type safety
- Follow same pattern as Detail.tsx for consistency
- Add react/prop-types: off to ESLint config for TypeScript project

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@SingTheCode
SingTheCode requested review from a team as code owners September 18, 2025 04:11
"@typescript-eslint/consistent-type-imports": ["error"],
"@typescript-eslint/no-unused-vars": "warn",
"@typescript-eslint/no-use-before-define": ["error"],
"@typescript-eslint/no-use-before-define": ["off"],

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

메인 컴포넌트 상단은 “무엇을 렌더링하는가”에만 집중하고, 세부 구현은 스크롤 다운 시에만 드러나게 하여 독해 흐름을 단순화하는데 목적이 있습니다.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

앗. 제가 헷갈려서 그러는데, 선언 먼저 하기 전에 사용되는 걸 막는 옵션아니었나용?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

네 맞습니다! 기존에는 호이스팅을 위한 구현이 아닌 선언후 호출 순서로 구현하게끔 eslint에서 가드하고 있었습니다.

로직을 위에서 아래로 읽어내려갈 때 위 영역에 서브 컴포넌트 로직이 과도하게 많아 메인 컴포넌트의 흐름이 바로 파악되지 않는다는 생각이 들어서 서브 컴포넌트를 함수 호이스팅을 통해 메인 컴포넌트 하위에 선언하여 좀 더 메인에 집중할 수 있도록 수정했습니다.

이부분 이슈로 만들어서 논의하는게 좋을까요??

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

호이스팅에 익숙한 세대들은 그렇게 생각할 수 있군요 ㅋㅋ 전 옛날 사람이라 선언이 안되어 있으면 불안... ㅋㅋ

이슈로 논의하는 건 매우 좋은 것 같습니다.
보통 lint rule 바꾸는 건 모두의 동의가 필요한 일이긴 하니까요!

(참고로 전 찬성입니다 ㅋㅋ 전 왠만하면 rule 싹 걸러내고 사용하는 스타일이라 😈 )

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

의견 감사합니다ㅎㅎ

discussion 이슈 생성하였고, 빠른 진행을 위해 금일 자정까지 논의할 예정입니다!

#892


export default Detail;

function CommitItem({ commit, owner, repo, authSrcMap, handleCommitIdCopy }: CommitItemProps) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

내부 로직은 변경되지 않았고, Detail 에서 추출해 하단으로 이동하였습니다.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(질문) 기존것과 다르게 function type으로 진행한 이유가 궁금합니다!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

별도의 component로 때네는 거면 별도의 파일로 분리해도 괜찮지 않을까요?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

별도의 component로 때네는 거면 별도의 파일로 분리해도 괜찮지 않을까요?

CommitItem 을 별도의 파일로 분리하지 않고 해당 파일 하단에 위치시킨 이유는 아래와 같습니다!

  1. 메인컴포넌트 내부에 종속되어있는 로직을 추출한 컴포넌트이기 때문에 해당 파일에 종속되는게 코드 응집성에 좋다고 생각했습니다.
  2. 별도의 컴포넌트로 분리하게되면 작은 단위의 파일이 너무 많아져 타고 들어가야하는 파일이 너무 많아질 것 같다고 생각했습니다.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

(질문) 기존것과 다르게 function type으로 진행한 이유가 궁금합니다!

선언부가 호출부보다 아래에 선언되기 때문에 함수 호이스팅을 사용한다는 의미에서 function으로 변경하였습니다!
메인 컴포넌트는 const, 하단에 선언하는 서브 컴포넌트는 function으로 선언하는 쪽으로 구현하였습니다.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(질문) arrow function + main component 위에 선언 하는 방법은 어떨까요??

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

여기서도 호이스팅이!!! ㅋㅋ
함수 위치를 매우 중요시 여기시는 군요 : )

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@lee-kyu-hwan 님 의견 감사합니다ㅎㅎ

(질문) arrow function + main component 위에 선언 하는 방법은 어떨까요??

추상화를 통해 한번에 파악해야하는 맥락의 크기를 줄이는게 목적인 리팩토링이라 추상화한 래퍼컴포넌트는 메인 컴포넌트 하단에 위치시켰습니다ㅎㅎ

토스 Frontend Fundamentals의 구현 상세 추상화하기
참고해보시면 좋을 것 같아요ㅎㅎ

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@lee-kyu-hwan 님 의견 감사합니다ㅎㅎ

(질문) arrow function + main component 위에 선언 하는 방법은 어떨까요??

추상화를 통해 한번에 파악해야하는 맥락의 크기를 줄이는게 목적인 리팩토링이라 추상화한 래퍼컴포넌트는 메인 컴포넌트 하단에 위치시켰습니다ㅎㅎ

토스 Frontend Fundamentals의 구현 상세 추상화하기 참고해보시면 좋을 것 같아요ㅎㅎ

오 좋은 자료 감사합니다!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@lee-kyu-hwan 님 의견 감사합니다ㅎㅎ

(질문) arrow function + main component 위에 선언 하는 방법은 어떨까요??

추상화를 통해 한번에 파악해야하는 맥락의 크기를 줄이는게 목적인 리팩토링이라 추상화한 래퍼컴포넌트는 메인 컴포넌트 하단에 위치시켰습니다ㅎㅎ
토스 Frontend Fundamentals의 구현 상세 추상화하기 참고해보시면 좋을 것 같아요ㅎㅎ

오 좋은 자료 감사합니다!

방금 자료 확인해봤는데 응집성을 코드의 위아래 순서까지 케어하는 내용이군요 : ) 흥미롭군요.
저도 관련 있는 것끼리 순서까지는 아니더라도 아래 위로 좀 붙여서 적는 스타일이긴 한데요.
요즘은 워낙 IDE기능을 잘 쓰면 저런것들도 잘 네비게이션도 되고, 방해가 안되게 함수별로 folding/collapse도 되니까 신경을 크게 안 쓰고 있었던 같기도 하네요.

그런 의미에서 저런 자유도를 떨어뜨리는 rule같은 건 과감하게 빼는게 좋을 것 같긴 합니다.
특히나 이런 오픈소스에서는요 : )


export default Summary;

function ClusterRow({

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

마찬가지로 추출해 이동만 진행하였습니다.

@ytaek ytaek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

고생하셨습니다!! LGTM!

Comment thread .gitignore
Comment on lines +4 to +6
/.claude/
/CLAUDE.md
/.rules/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

이제 이런것들이 많이 늘어나겠군요!!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ai 에이전트의 초기 설정에 대한 파일(CLAUDE.md, GEMINI.md 등)은 어쩔 수 없겠지만, 프로젝트를 관통하는 룰에 대한 내용은 .rules에 넣고, 어떤 에이전트를 사용하더라도 .rules를 바라보도록 하면 좋을 것 같다는 생각이 듭니다ㅎㅎ

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

네. private rules랑 public rules랑 잘 분리하는 것도 좋을 것 같습니다.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

나중에 공통으로 사용할 rules 이 정해진다면 확실히 구분이 필요하겠네요ㅎㅎ 의견 감사합니다!

"@typescript-eslint/consistent-type-imports": ["error"],
"@typescript-eslint/no-unused-vars": "warn",
"@typescript-eslint/no-use-before-define": ["error"],
"@typescript-eslint/no-use-before-define": ["off"],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

앗. 제가 헷갈려서 그러는데, 선언 먼저 하기 전에 사용되는 걸 막는 옵션아니었나용?

"react/no-unstable-nested-components": ["error", { "allowAsProps": true }],
"react/react-in-jsx-scope": "off",
"react/require-default-props": "off",
"react/prop-types": "off",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(잘모름) 요것 disable 하신 이유가 궁금합니다!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

props의 타입을 PropTypes로 명시하고 검증하라는 옵션입니다!
JavaScript 환경에서는 props 내부의 프로퍼티를 명확히 선언하고 사용하도록하여 런타임 시 에러를 방지하는데요!

TypeScript 환경에서는 정적타입으로 props를 검증하기 때문에 (ListRowProps에 index 프로퍼티가 있는지 이미 알고 있음) 별도의 선언이 필요없다고 생각해서 해당 옵션 off 했습니다ㅎㅎ

참조

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

rule 하나씩 빼는거 너무너무나도 좋습니다 🥇🥇🥇🥇🥇🥇

"error",
{
"namedComponents": ["arrow-function"],
"namedComponents": ["arrow-function", "function-declaration"],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(잘모름) 요것 disable 하신 이유가 궁금합니다!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

함수 호이스팅을 하기 위함입니다!


export default Detail;

function CommitItem({ commit, owner, repo, authSrcMap, handleCommitIdCopy }: CommitItemProps) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(질문) 기존것과 다르게 function type으로 진행한 이유가 궁금합니다!


export default Detail;

function CommitItem({ commit, owner, repo, authSrcMap, handleCommitIdCopy }: CommitItemProps) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

별도의 component로 때네는 거면 별도의 파일로 분리해도 괜찮지 않을까요?

import type { ContentProps } from "../Summary.type";

const Content = ({ content, clusterId, selectedClusterId }: ContentProps) => {
const Content = ({ content, clusterId, selectedClusterIds }: ContentProps) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

아 명확한 표현 좋습니다!!

@yuminnnnni

Copy link
Copy Markdown
Member

전역 상태관리 적용하신 거 너무 좋은 것 같습니다~~
수고하셨습니다!!

@SingTheCode

Copy link
Copy Markdown
Contributor Author

전역 상태관리 적용하신 거 너무 좋은 것 같습니다~~ 수고하셨습니다!!

@yuminnnnni 님 리뷰 감사합니다ㅎㅎㅎ 어푸 부탁드립니다!!ㅋㅋㅋ

@SingTheCode SingTheCode self-assigned this Sep 20, 2025
ytaek
ytaek previously approved these changes Sep 20, 2025
yuminnnnni
yuminnnnni previously approved these changes Sep 21, 2025
Zy0ung
Zy0ung previously approved these changes Sep 22, 2025

@Zy0ung Zy0ung left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!!! 고생하셨습니다!!!

@nxnaxx

nxnaxx commented Sep 22, 2025

Copy link
Copy Markdown
Contributor

고생하셨습니다!!

SingTheCode and others added 2 commits September 22, 2025 23:19
- Clean up line formatting in summary commit title rendering

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@SingTheCode

SingTheCode commented Sep 22, 2025

Copy link
Copy Markdown
Contributor Author

@lee-kyu-hwan @ytaek @Zy0ung @yuminnnnni
컨플릭해결하니 어푸가 풀렸네요..ㅎㅎ 한번더 어푸 부탁드립니다!

@chae-dahee chae-dahee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!! 고생하셨습니당 👍

@chysis chysis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

변경 사항 확인했습니다! 호이스팅을 고려한 코드가 새로웠어요👍🏻
이번 이슈에서 CommitItem을 컴포넌트로 분리해주셔서, 분리 작업을 제외한 나머지 렌더링 최적화 부분만 제가 #861 이슈에서 이어서 작업하겠습니다

@SingTheCode
SingTheCode merged commit b9a661f into main Sep 23, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[refactor]: #800 Stem 닫기 성능 개선을 위한 사전 리팩토링

8 participants