Skip to content

refactor(view): processMessage 함수 공통화 및 리팩토링#938

Merged
nxnaxx merged 4 commits into
githru:mainfrom
nxnaxx:refactor/#900_extract-process-message
Oct 8, 2025
Merged

refactor(view): processMessage 함수 공통화 및 리팩토링#938
nxnaxx merged 4 commits into
githru:mainfrom
nxnaxx:refactor/#900_extract-process-message

Conversation

@nxnaxx

@nxnaxx nxnaxx commented Oct 7, 2025

Copy link
Copy Markdown
Contributor

Related issue

#900

Result

Detail.tsx, Content.tsx에 중복되어 있던 커밋 메시지 파싱/이슈 링크 렌더링 로직을 공통 유틸과 헬퍼로 분리하는 리팩토링 작업을 진행했습니다.

Work list

1. 커밋 메시지 분리 유틸 함수 추가

splitMessageByIssueRefs (src/utils/commitMessage.ts)

  • 기존 processMessage 함수 로직을 공통 유틸로 분리
    • 이슈 링크 컴포넌트 생성 로직 제외, 문자열 파싱(텍스트/이슈 참조 분리)만 추출
    • 이슈 #900에 작성된 내용과 다르게 "이슈 번호 앞 텍스트 추출 / 마지막 텍스트 추가"와 같은 세부 단계를 별도 헬퍼로 쪼개지 않은 이유
      -> 한 번의 스캔으로 메시지를 분리하기 위함. 세부 단계로 나누면 각 단계에서 문자열을 분리/탐색하고 재조립하는 과정에서 중복 탐색이나 불필요한 메모리 할당이 발생할 수 있음. 단일 함수에서 커서 기반으로 처리

2. 파싱 오류 수정 / 기능 보강

  • 괄호 패턴 지원
    • 기존 코드상 주석 처리된 부분에 괄호가 포함된 이슈 참조도 지원한다고 되어있으나 실제로는 처리되지 않음
    const processMessage = (message: string) => {
        // GitHub 이슈 번호 패턴: #123 또는 (#123)
        const regex = /(?:^|\s)(#\d+)(?:\s|$)/g;
        ....
    }
    
    -> (#123), [#123]과 같은 패턴도 정확히 매칭하여 링크로 치환할 수 있도록 수정
  • 커밋 메시지 원문 보존
    • 문제) 기존 메시지 분리 시 공백 누락. 재조립 시 원문 보존 X
      e.g. "Merge pull request #​921 from user/main" -> ["Merge pull request", "#​921" "from user/main"] -> "Merge pull request#​921from user/main"
    • 개선) 공백까지 포함하여 추출
      e.g. "Merge pull request #​921 from user/main" -> ["Merge pull request ", "#​921" " from user/main"] -> "Merge pull request #​921 from user/main"

(전)
스크린샷 2025-10-07 오전 1 24 52

(후)
스크린샷 2025-10-07 오전 1 16 24

3. splitMessageByIssueRefs 단위 테스트 추가

  • #​123, (#​123), [#​123], 다중 참조 패턴 매칭 확인
  • 이슈 참조가 문장 첫 번째, 중간, 마지막에 위치하는 경우 정확히 파싱되는지 확인
  • edge case 확인
    • #​123abc와 같이 문자와 붙어있는 경우 파싱 X
    • 커밋 메시지 내 이슈 참조가 존재하지 않는 경우 원문 텍스트 반환

4. Detail, Content 컴포넌트 리팩토링

  • renderIssueLinkedNodes 공용 렌더 헬퍼 추출
    • 메시지 내 이슈 참조를 GithubIssueLink로 렌더하는 로직 분리
  • 기존 useEffect, setState로 관리하던 제목/본문 파싱+렌더링 로직을 useMemo로 전환
    • 순수 파생값이므로 불필요한 추가 렌더링 제거 및 의존성 관리 단순화
  • useState로 관리하던 linkedMessge -> issueLinkedMessage로 이름 변경
  • Detail, Content에 중복으로 정의되어 있던 LinkedMessage 타입을 단일 타입으로 통합

5. 이전 PR 피드백 반영

Related issue: #921

  • GithubIssueLink 컴포넌트 내 css 클래스명 변경 commit-message__issue-link -> github-issue-link

nxnaxx added 4 commits October 7, 2025 13:35
… text and issue references

- Supports 'githru#123', '(githru#123)', and '[githru#123]'
- Preserves original spacing by extracting surrounding whitespace from the commit message
…Content

- Extract shared helper for issue-linked rendering (renderIssueLinkedNodes)
- Unify IssueLinkedMessage type across components
- Switch derived computations from useEffect to useMemo to avoid extra renders
@nxnaxx nxnaxx self-assigned this Oct 7, 2025
@nxnaxx
nxnaxx requested review from a team as code owners October 7, 2025 11:04

@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!!!
테스트코드가 너무 잘 작성되어있네요~ PR 문서와 함께 봐서 이해하기 좋았습니다! 고생하셨습니다 👍

Comment on lines +43 to -100
const issueLinkedMessage: IssueLinkedMessage = useMemo(() => {
const message = commitNodeListInCluster?.[0]?.commit?.message;
if (!message) return { title: [], body: null };

parts.push(
<GithubIssueLink
key={`issue-${issueNumber}-${match.index}`}
owner={owner}
repo={repo}
issueNumber={issueNumber}
/>
);
const [title, ...rest] = message.split("\n");
const body = rest.filter((line) => line.trim()).join("\n");

lastIndex = match.index + match[0].length;
}

// 마지막 부분 추가
if (lastIndex < message.length) {
parts.push(message.slice(lastIndex));
}

return parts.length > 0 ? parts : [message];
return {
title: renderIssueLinkedNodes(title, owner, repo),
body: body ? renderIssueLinkedNodes(body, owner, repo) : null,
};

if (commitNodeListInCluster?.[0]?.commit?.message) {
const { message } = commitNodeListInCluster[0].commit;
const messageLines = message.split("\n");
const title = messageLines[0];
const body = messageLines
.slice(1)
.filter((line: string) => line.trim())
.join("\n");

// 제목과 본문을 각각 처리
const processedTitle = processMessage(title);
const processedBody = body ? processMessage(body) : null;

setLinkedMessage({
title: processedTitle,
body: processedBody,
});
}

@chae-dahee chae-dahee Oct 7, 2025

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.

로직 분리와 useMemo 를 통해 코드가 훨신 깔끔해진게 직관적으로 보입니다. 너무 좋네요~~ 👍 👍

Comment on lines +8 to +10
for (;;) {
const match = GITHUB_ISSUE_REGEX.exec(message);
if (!match) break;

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

Choose a reason for hiding this comment

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

(사소) 이런 표현도 좋긴 한데, 명확하게 loop를 exit하는 condition을 기술할 수 있는 방법도 좋은 것 같습니다.
코드 보는 사람이 break가 어디있는지 loop문을 다 훑지 않아도 어떤 조건일 때 계속 도는지 바로 확인할 수 있으니까요!

@hyemimi hyemimi 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.

리팩토링, 버그 수정, 테스트까지 멋진 PR 잘 봤습니당!! 수고 많으셨습니다~ 👍👍

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.

테스트 파일 작성 너무 좋습니다👍👍

@nxnaxx nxnaxx changed the title refactor(view): ProcessMessage 함수 공통화 및 리팩토링 refactor(view): processMessage 함수 공통화 및 리팩토링 Oct 8, 2025

@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.

아 코드 깔끔하게 넘 좋습니다!! 이제 리팩토링 장인이 되신 듯!! 👍👍👍👍👍

["fix: 500 on null (", "#10", ") and [", "#20", "], also ", "#30"],
["#10", "#20", "#30"],
],
] as const satisfies readonly IssueRefSplitCase[];

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.

아 이런 고급 문법 느좋입니다!! 🥇

issueParts.forEach((part) => expect(message.slice(part.index, part.index + part.value.length)).toBe(part.value));
});

it("remains stateless across calls", () => {

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.

추후에 negative Test case도 한번 만들어보시면 좋을 것 같습니다.
경계값 테스 등 두요!

Comment on lines +8 to +10
for (;;) {
const match = GITHUB_ISSUE_REGEX.exec(message);
if (!match) break;

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.

(사소) 이런 표현도 좋긴 한데, 명확하게 loop를 exit하는 condition을 기술할 수 있는 방법도 좋은 것 같습니다.
코드 보는 사람이 break가 어디있는지 loop문을 다 훑지 않아도 어떤 조건일 때 계속 도는지 바로 확인할 수 있으니까요!

@nxnaxx
nxnaxx merged commit 949f34c into githru:main Oct 8, 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.

4 participants