Skip to content

refactor: 하이라이트 JS 리소스 분리 + 메시지 버스 단일화 (step 1·2) - #105

Merged
gomminjae merged 2 commits into
developfrom
refactor/highlight-engine-extract
Jun 9, 2026
Merged

gomminjae merged 2 commits into
developfrom
refactor/highlight-engine-extract

Conversation

@gomminjae

Copy link
Copy Markdown
Collaborator

Article Highlight 리팩토링 설계(docs/architecture-review.md)의 step 1·2. 모두 동작 보존.

step 1 — JS 리소스 분리

  • ArticleHighlightJS의 JS 757줄 → Resources/highlight-core.js(221) + highlight-selection.js(410)
  • ArticleHighlightJS는 Bundle.module 로더로 축소
  • Swift 문자열 이스케이프 4중첩(/[.*+?^${}()|[\\]\\\\]/g 등) 제거 → JS 원본 그대로

step 2 — 메시지 버스 단일화

  • WKWebView messageHandler 5개 → 단일 highlightEvent 봉투({type, payload})
  • HighlightEvent enum으로 디코딩, rawValue→HighlightStyle 변환을 디코드 지점으로 이동
  • 죽은 getSelectedText 핸들러 제거
  • 콜백 3개 + selectedText 바인딩 → ViewModel handle(_:) 단일 진입점

범위 밖 (후속 PR)

  • step 3 포트(HighlightRenderable/WebViewHighlightRenderer) — SwiftUI WebView 소유권 재배치라 테스트 가능 단계와 함께
  • step 4 TS 승격, step 5 HighlightLocator

검증

  • Detail 빌드 통과 (BUILD SUCCEEDED)
  • ⚠️ 동작 변경 리팩토링이라 머지 전 수동 스모크 필요: 텍스트 선택→색칠→재진입 시 복원, 색 변경, 삭제, 스크롤-투-하이라이트

gomminjae added 2 commits June 9, 2026 16:57
step1: ArticleHighlightJS의 JS 757줄을 highlight-core.js/highlight-selection.js
리소스로 분리. Bundle.module 로더로 축소해 이스케이프 4중첩 제거.

step2: WKWebView messageHandler 5개를 단일 highlightEvent 봉투로 통합하고
HighlightEvent enum으로 디코딩. 죽은 getSelectedText 핸들러 제거. 콜백 3개와
selectedText 바인딩을 ViewModel의 handle(_:) 단일 진입점으로 통합.

동작 보존(최종 주입 JS·이벤트 의미 동일). 포트(HighlightRenderable)와
HighlightLocator는 후속 PR.

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request refactors the Article Highlight feature in the Detail module by extracting inline JavaScript code from ArticleHighlightJS.swift into dedicated .js resource files (highlight-core.js and highlight-selection.js). It also simplifies the communication bridge between the WKWebView and Swift by introducing a unified HighlightEvent enum and a single message handler, cleaning up view and view model bindings. Additionally, an architecture review document has been added. The review feedback suggests adding defensive checks in the JavaScript files, specifically verifying the existence of the webkit message handler to prevent runtime errors in non-iOS environments, and adding a null guard for el.parentNode when deleting highlights to avoid potential crashes.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +1 to +3
function postHighlightEvent(type, payload) {
window.webkit.messageHandlers.highlightEvent.postMessage({ type: type, payload: payload });
}

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.

medium

window.webkit.messageHandlers.highlightEvent가 정의되어 있지 않은 환경(예: 일반 브라우저에서 페이지를 열거나 테스트 환경 등)에서 호출될 경우 TypeError가 발생하여 JavaScript 실행이 중단될 수 있습니다.

안전한 실행을 위해 해당 객체의 존재 여부를 먼저 확인하는 방어적 코드를 추가하는 것을 권장합니다.

function postHighlightEvent(type, payload) {
    if (window.webkit && window.webkit.messageHandlers && window.webkit.messageHandlers.highlightEvent) {
        window.webkit.messageHandlers.highlightEvent.postMessage({ type: type, payload: payload });
    }
}

Comment on lines +385 to +392
groupElements.forEach(function(el) {
const parent = el.parentNode;
while (el.firstChild) {
parent.insertBefore(el.firstChild, el);
}
parent.removeChild(el);
parent.normalize();
});

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.

medium

el.parentNode가 어떤 이유로든 null인 경우(예: DOM에서 이미 분리된 요소 등), parent.insertBefore 호출 시 에러가 발생할 수 있습니다.

parent가 유효한지 확인하는 방어적 코드(Null Guard)를 추가하는 것이 안전합니다.

Suggested change
groupElements.forEach(function(el) {
const parent = el.parentNode;
while (el.firstChild) {
parent.insertBefore(el.firstChild, el);
}
parent.removeChild(el);
parent.normalize();
});
groupElements.forEach(function(el) {
const parent = el.parentNode;
if (!parent) return;
while (el.firstChild) {
parent.insertBefore(el.firstChild, el);
}
parent.removeChild(el);
parent.normalize();
});

@gomminjae
gomminjae merged commit 519d8cb into develop Jun 9, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant