-
Notifications
You must be signed in to change notification settings - Fork 2
[SRLT-118] 리팩토링 #87
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
[SRLT-118] 리팩토링 #87
The head ref may contain hidden characters: "SRLT-118-\uB9AC\uD329\uD1A0\uB9C1"
Conversation
개요이 PR은 WriteForm 컴포넌트의 에디터 초기화를 중앙 집중식 팩토리 함수로 리팩토링하고, 이미지 업로드 로직을 paste 핸들러에서 WriteFormToolbar로 이동하며, 저장 API를 saveSingleItem으로 변경합니다. 동시에 에디터 상태 관리, 커서 위치 복원, 에디터 리스너 등록을 개선합니다. 변경 사항
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant Toolbar as WriteFormToolbar
participant Editor as TipTap Editor
participant Upload as uploadImage
participant Store as Business Store
User->>Toolbar: 이미지 버튼 클릭
Toolbar->>Editor: defaultEditor 포커스 (필요시)
Toolbar->>Toolbar: 파일 선택 다이얼로그 오픈
User->>Toolbar: 이미지 파일 선택
Toolbar->>Upload: uploadImage(file)
rect rgb(200, 220, 240)
Upload-->>Upload: 이미지 업로드 처리
Upload-->>Toolbar: 업로드된 이미지 URL 반환
end
Toolbar->>Toolbar: 이미지 크기 계산<br/>(getImageDimensions)
Toolbar->>Toolbar: 사용 가능한 너비 기반<br/>이미지 크기 고정
Toolbar->>Editor: editor.chain()<br/>.focus()<br/>.setImage({src, width, height})<br/>.run()
rect rgb(230, 240, 230)
Editor->>Editor: 에디터에 이미지 삽입
end
Note over Toolbar,Editor: 저장은 debounced<br/>saveSingleItem으로<br/>자동 처리됨
예상 코드 리뷰 소요 시간🎯 4 (복잡) | ⏱️ ~45분 관련 PR 목록
제안 검토자
축하 시
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/store/business.store.ts (1)
301-316: 중복된 타입 정의를 추출하여 재사용하는 것을 권장합니다.
SidebarChecklist,SidebarItem,SidebarSection타입이saveAllItems(lines 243-255)와 동일하게 중복 정의되어 있습니다. 이 타입들을 파일 상단이나 별도 타입 파일로 추출하면 유지보수성이 향상됩니다.🔎 제안된 리팩토링
파일 상단에 타입을 한 번만 정의:
// 파일 상단에 추가 type SidebarChecklist = { title: string; content: string; checked?: boolean; }; type SidebarItem = { name: string; number: string; title: string; subtitle: string; checklist?: SidebarChecklist[]; }; type SidebarSection = { title: string; items: SidebarItem[] }; const getAllSidebarItems = () => (sections as SidebarSection[]).flatMap((section) => section.items);src/lib/business/editor/editorConstants.ts (1)
68-73: 주석 처리된 파일 크기 제한 코드를 제거하거나 구현해주세요.주석 처리된 코드가 남아있습니다. 파일 크기 제한이 필요하다면 구현하고, 불필요하다면 제거하는 것이 좋습니다. 주석 처리된 코드는 혼란을 야기할 수 있습니다.
src/app/business/components/editor/WriteFormToolbar.tsx (1)
92-100:activeEditor상태 업데이트와 파일 선택 간의 타이밍을 확인해주세요.
onActiveEditorChange(defaultEditor)는 React 상태 업데이트이므로 비동기적으로 처리됩니다.fileInputRef.current?.click()이 즉시 호출되어handleImageUpload가 실행될 때activeEditor가 아직 업데이트되지 않았을 수 있습니다.다행히
handleImageUpload의 line 67에서activeEditor를 다시 확인하고 있지만, 이 경우 업로드가 무시될 수 있습니다.defaultEditor를 직접 사용하는 것을 고려해보세요.🔎 제안된 수정
const handleImageButtonClick = () => { + const targetEditor = activeEditor || (defaultEditor && !defaultEditor.isDestroyed ? defaultEditor : null); if (!activeEditor) { if (defaultEditor && !defaultEditor.isDestroyed) { defaultEditor.commands.focus(); onActiveEditorChange(defaultEditor); } } - fileInputRef.current?.click(); + if (targetEditor) { + fileInputRef.current?.click(); + } };그리고
handleImageUpload에서activeEditor대신 클로저나 ref를 통해 타겟 에디터를 전달하는 방식을 고려해보세요.src/app/business/components/WriteForm.tsx (1)
230-231: 자동 저장 디바운스 시간(300ms)이 짧을 수 있습니다.300ms는 일반적인 자동 저장 간격으로는 다소 공격적입니다. 사용자가 잠시 멈출 때마다 API 요청이 발생할 수 있습니다. 1000-2000ms 정도로 늘리는 것을 고려해보세요. 단, 이는 UX 요구사항에 따라 다를 수 있습니다.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/app/business/components/WriteForm.tsxsrc/app/business/components/editor/WriteFormToolbar.tsxsrc/lib/business/editor/editorConstants.tssrc/lib/business/editor/types.tssrc/lib/business/editor/useEditorConfig.tssrc/store/business.store.tssrc/types/business/business.store.type.tssrc/types/business/business.type.ts
💤 Files with no reviewable changes (2)
- src/lib/business/editor/types.ts
- src/lib/business/editor/useEditorConfig.ts
🧰 Additional context used
🧬 Code graph analysis (4)
src/types/business/business.type.ts (1)
src/util/checkcontent.ts (1)
Editor(9-15)
src/app/business/components/editor/WriteFormToolbar.tsx (4)
src/lib/imageUpload.ts (1)
uploadImage(4-72)src/lib/getImageDimensions.ts (2)
getImageDimensions(1-20)clampImageDimensions(22-43)src/lib/business/editor/getSelectionAvailableWidth.ts (1)
getSelectionAvailableWidth(36-71)src/types/business/business.type.ts (1)
ImageCommandAttributes(227-230)
src/app/business/components/WriteForm.tsx (5)
src/lib/business/editor/editorConstants.ts (6)
createEditorFeaturesConfig(109-121)createEditorSkillsConfig(123-135)createEditorGoalsConfig(137-149)createEditorItemNameConfig(151-162)createEditorOneLineIntroConfig(164-175)createEditorGeneralConfig(177-189)src/util/checkcontent.ts (1)
Editor(9-15)src/store/spellcheck.store.ts (1)
useSpellCheckStore(30-46)src/hooks/mutation/useSpellCheck.ts (1)
useSpellCheck(8-16)src/store/editor.store.ts (1)
useEditorStore(17-23)
src/store/business.store.ts (3)
src/types/feedback/sections.ts (1)
sections(1-17)src/lib/business/requestBuilder.ts (1)
buildSubsectionRequest(8-78)src/api/business.ts (1)
postBusinessPlanSubsections(24-30)
🔇 Additional comments (9)
src/types/business/business.store.type.ts (1)
54-55: LGTM!타입 선언이 구현체(
business.store.ts)와 일치하며, 단일 항목 저장을 위한 새로운 메서드 시그니처가 적절하게 정의되었습니다.src/store/business.store.ts (1)
337-345: LGTM!단일 항목 저장 로직이 적절하게 구현되었습니다. 에러 처리와
lastSavedTime업데이트가 올바르게 동작합니다.src/lib/business/editor/editorConstants.ts (1)
109-189: LGTM!에디터 설정 팩토리 함수들이 잘 구성되었습니다.
COMMON_EXTENSIONS와SIMPLE_EXTENSIONS의 분리가 적절하고, SSR 호환성을 위한immediatelyRender: false설정도 올바릅니다.src/types/business/business.type.ts (2)
227-230: LGTM!
ImageCommandAttributes타입이 TipTap의setImage커맨드 파라미터를 타입 안전하게 추출하고,width와height속성으로 확장하고 있습니다.
2-2: 이 comment는 실제 코드베이스에 존재하지 않는 문제입니다.
src/types/business/business.type.ts는@tiptap/core에서만Editor를 import하고 있으며,src/util/checkcontent.ts와 어떤 관계도 없습니다. 두Editor정의는 완전히 다른 모듈에 있으므로 naming collision이 발생하지 않습니다:
@tiptap/core의Editor: 외부 라이브러리의 rich text editor 타입src/util/checkcontent.ts의Editor: 로컬 JSON 콘텐츠 구조 타입이는 표준적인 관행이며 서로 다른 모듈에서 같은 이름의 타입을 사용하는 것은 문제가 되지 않습니다.
Likely an incorrect or invalid review comment.
src/app/business/components/editor/WriteFormToolbar.tsx (1)
56-90: LGTM!이미지 업로드 핸들러가 적절하게 구현되었습니다. 파일 타입 검증, 에러 처리, 사용자 피드백이 잘 갖춰져 있습니다.
src/app/business/components/WriteForm.tsx (3)
112-142: LGTM!에디터 콘텐츠 복원 로직이 잘 구현되었습니다. 문자열과 JSONContent 타입을 적절히 처리하고, overview와 general 섹션을 올바르게 구분하고 있습니다.
144-151: LGTM!
saveSingleItem을 사용하여 현재 작성 중인 항목만 저장하도록 변경된 것이 PR 목표와 일치합니다.
379-390: LGTM!
WriteFormToolbar에 새로운 props(defaultEditor,onActiveEditorChange)가 올바르게 전달되고 있습니다.
| const timeoutRefs = useRef({ | ||
| main: null as NodeJS.Timeout | null, | ||
| skills: null as NodeJS.Timeout | null, | ||
| goals: null as NodeJS.Timeout | null, | ||
| itemName: null as NodeJS.Timeout | null, | ||
| oneLineIntro: null as NodeJS.Timeout | null, | ||
| }); | ||
|
|
||
| if (editorGoals) { | ||
| const handleGoalsUpdate = createUpdateHandler(goalsTimeoutRef); | ||
| editorGoals.on('update', handleGoalsUpdate); | ||
| cleanup.push(() => { | ||
| if (goalsTimeoutRef.current) clearTimeout(goalsTimeoutRef.current); | ||
| editorGoals.off('update', handleGoalsUpdate); | ||
| }); | ||
| } | ||
| } | ||
| const registerEditorListener = useCallback( | ||
| ( | ||
| editor: Editor | null, | ||
| timeoutKey: keyof typeof timeoutRefs.current | ||
| ): (() => void) | null => { | ||
| if (!editor) return null; | ||
| const timeoutRef = { current: timeoutRefs.current[timeoutKey] }; | ||
| const handler = createUpdateHandler(timeoutRef); | ||
| editor.on('update', handler); | ||
| timeoutRefs.current[timeoutKey] = timeoutRef.current; | ||
| return () => { | ||
| if (timeoutRef.current) clearTimeout(timeoutRef.current); | ||
| editor.off('update', handler); | ||
| }; | ||
| }, | ||
| [createUpdateHandler] | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
timeoutRef 관리 로직에 잠재적 문제가 있습니다.
registerEditorListener 내에서 timeoutRef를 새 객체로 생성한 후 timeoutRefs.current[timeoutKey]를 업데이트하고 있습니다(line 267). 하지만 createUpdateHandler는 전달받은 timeoutRef의 current를 사용하므로, timeoutRefs.current[timeoutKey]의 업데이트가 실제 timeout 관리에 반영되지 않을 수 있습니다.
🔎 제안된 수정
timeoutRefs.current[timeoutKey]를 직접 참조하는 래퍼 객체를 사용하거나, createUpdateHandler가 key를 받아서 직접 timeoutRefs.current에 접근하도록 수정하세요:
const registerEditorListener = useCallback(
(
editor: Editor | null,
timeoutKey: keyof typeof timeoutRefs.current
): (() => void) | null => {
if (!editor) return null;
- const timeoutRef = { current: timeoutRefs.current[timeoutKey] };
- const handler = createUpdateHandler(timeoutRef);
+ // timeoutRefs.current를 직접 참조하는 객체 생성
+ const timeoutRef = {
+ get current() { return timeoutRefs.current[timeoutKey]; },
+ set current(v) { timeoutRefs.current[timeoutKey] = v; },
+ };
+ const handler = createUpdateHandler(timeoutRef as React.MutableRefObject<NodeJS.Timeout | null>);
editor.on('update', handler);
- timeoutRefs.current[timeoutKey] = timeoutRef.current;
return () => {
- if (timeoutRef.current) clearTimeout(timeoutRef.current);
+ if (timeoutRefs.current[timeoutKey]) clearTimeout(timeoutRefs.current[timeoutKey]!);
editor.off('update', handler);
};
},
[createUpdateHandler]
);🤖 Prompt for AI Agents
In src/app/business/components/WriteForm.tsx around lines 250 to 274, the local
timeoutRef object is detached from the shared timeoutRefs.current slot so
updates made by createUpdateHandler may not reflect back into
timeoutRefs.current; fix by making timeoutRef a live proxy into
timeoutRefs.current[timeoutKey] or by changing createUpdateHandler to accept the
timeoutKey and read/write timeoutRefs.current[timeoutKey] directly: create a
wrapper whose current getter/setter delegates to timeoutRefs.current[timeoutKey]
(or refactor the handler signature to use the key), ensure you assign and clear
the same slot in timeoutRefs.current for setup and cleanup so
clearTimeout/assignment operate on the actual shared ref.
| .then(async (imageUrl) => { | ||
| if (imageUrl) { | ||
| const editor = (view as EditorView & { editor?: Editor }).editor; | ||
| const { width, height } = await getImageDimensions(imageUrl); | ||
| const selectionWidth = getSelectionAvailableWidth(editor ?? null); | ||
| const fallbackWidth = view.dom?.clientWidth ? view.dom.clientWidth - 48 : undefined; | ||
| const maxWidth = selectionWidth ?? fallbackWidth; | ||
| const { width: clampedWidth, height: clampedHeight } = clampImageDimensions(width, height, maxWidth ?? undefined); | ||
| const imageAttributes: ImageCommandAttributes = { | ||
| src: imageUrl, | ||
| width: clampedWidth ?? undefined, | ||
| height: clampedHeight ?? undefined, | ||
| }; | ||
| editor | ||
| ?.chain() | ||
| .focus() | ||
| .setImage(imageAttributes) | ||
| .run(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the actual file and surrounding context
wc -l src/lib/business/editor/editorConstants.tsRepository: StartUpLight/STARLIGHT_FE
Length of output: 115
🏁 Script executed:
# Get the specific section mentioned in the review
sed -n '70,100p' src/lib/business/editor/editorConstants.tsRepository: StartUpLight/STARLIGHT_FE
Length of output: 1736
🏁 Script executed:
# Search for how editor is being passed/accessed elsewhere
rg "handlePaste|EditorView" src/lib/business/editor/ -A 5 -B 5Repository: StartUpLight/STARLIGHT_FE
Length of output: 8153
🌐 Web query:
TipTap EditorView handlePaste access editor instance recommended approach
💡 Result:
Short answer — don’t try to reach into Tiptap’s Editor from a raw ProseMirror handlePaste; use Tiptap’s APIs instead (paste rules, extension plugins or editor events). Recommended options:
-
For content transforms (text/HTML → nodes/marks): implement addPasteRules() in a Node/Mark/Extension. It’s the high‑level Tiptap way to intercept and transform pasted content. [1]
-
For more complex/custom paste logic (need access to view/state/dispatch): add a ProseMirror plugin from an extension via addProseMirrorPlugins() and implement handlePaste(view, event, slice) there. You receive the ProseMirror EditorView and Slice (ProseMirror API) to dispatch transactions. Use this only when pasteRules aren’t enough. (ProseMirror handlePaste signature). [2][3]
-
For file/image pastes: use the official @tiptap/extension-file-handler (onPaste / onDrop) which supplies the Tiptap editor instance and pasted files — convenient and recommended for uploads. [4]
-
Alternatively, for global behavior: register an editor-level "paste" event when creating the Editor (editor.on('paste', ...)) or use the EditorContext/hooks in React integrations to access the editor instance. [1][5]
References
- Tiptap — Paste rules (addPasteRules) and extension API. [1]
- ProseMirror — handlePaste(view, event, slice) (signature & behavior). [2]
- Tiptap — ProseMirror integration / core concepts (how to add plugins). [3]
- @tiptap/extension-file-handler — onPaste/onDrop that pass the Tiptap editor and files. [4]
- Tiptap — Editor events (paste) and React EditorContext usage. [5]
If you want, tell me which exact paste behavior you need and I’ll show the minimal addPasteRules or addProseMirrorPlugins example.
🏁 Script executed:
# Check if @tiptap/extension-file-handler is available
rg "extension-file-handler|file-handler" src/Repository: StartUpLight/STARLIGHT_FE
Length of output: 51
🏁 Script executed:
# Check package.json for available TipTap extensions
cat package.json | grep -A 30 "dependencies"Repository: StartUpLight/STARLIGHT_FE
Length of output: 1081
ProseMirror 플러그인을 통해 에디터 인스턴스에 안전하게 접근하세요.
(view as EditorView & { editor?: Editor }).editor는 TipTap의 공식 API가 아닌 내부 구현에 의존합니다. TipTap 버전 업그레이드 시 동작이 변경될 수 있습니다.
권장 방안:
- addProseMirrorPlugins(): Extension에서 ProseMirror 플러그인으로 handlePaste를 구현하면 view/state/dispatch에 접근 가능합니다.
- 클로저 패턴: handlePaste를 생성할 때 에디터 인스턴스를 매개변수로 전달받아 클로저로 캡슐화합니다.
- 에디터 레벨 이벤트: editor.on('paste', ...) 또는 React 통합 시 EditorContext를 통해 에디터에 접근합니다.
현재 패턴을 위의 공식 방식으로 리팩토링하는 것을 권장합니다.
🤖 Prompt for AI Agents
In src/lib/business/editor/editorConstants.ts around lines 77 to 95, the code
directly casts view to access a non-public editor instance ((view as EditorView
& { editor?: Editor }).editor) which relies on TipTap internals and is brittle
across upgrades; replace this pattern by exposing the editor through a supported
mechanism: implement the paste handler inside addProseMirrorPlugins so you
receive view/state/dispatch directly, or create the handler as a closure that
receives and captures the editor instance when the extension is initialized, or
register a proper editor-level paste event (editor.on('paste', ...) or via
EditorContext in React). Move the image-insert logic out of direct view casting
and instead call editor.chain().focus().setImage(...) using the
injected/captured editor reference obtained via one of those supported
approaches.
Chasyuss
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확인했어요! 고생했다..
#️⃣ 관련 이슈
✍️ 작업 내용 요약
💡 작업 내용
기능
🖥️ 구현 화면
기타 사항
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정
개선
✏️ Tip: You can customize this high-level summary in your review settings.