From 3e2f5238963386c5093ca3b220f2a014f81fc420 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Sat, 30 May 2026 02:58:28 -0700 Subject: [PATCH] Fix contextual copy dispose cleanup Co-authored-by: Orca --- .../editor/setup-contextual-copy.test.ts | 108 ++++++++++++++++++ .../editor/setup-contextual-copy.ts | 14 ++- 2 files changed, 118 insertions(+), 4 deletions(-) create mode 100644 src/renderer/src/components/editor/setup-contextual-copy.test.ts diff --git a/src/renderer/src/components/editor/setup-contextual-copy.test.ts b/src/renderer/src/components/editor/setup-contextual-copy.test.ts new file mode 100644 index 00000000000..ce3da798b73 --- /dev/null +++ b/src/renderer/src/components/editor/setup-contextual-copy.test.ts @@ -0,0 +1,108 @@ +import type { editor } from 'monaco-editor' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { setupContextualCopy } from './setup-contextual-copy' + +vi.mock('@/hooks/useShortcutLabel', () => ({ + formatShortcutLabel: () => 'Copy Context' +})) + +vi.mock('@/lib/monaco-setup', () => ({ + monaco: { + editor: { + ContentWidgetPositionPreference: { + ABOVE: 1, + BELOW: 2 + } + } + } +})) + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => ({ keybindings: {} }) + } +})) + +vi.mock('@/lib/primary-selection', () => ({ + PRIMARY_SELECTION_MAX_LENGTH: 10_000, + isPrimarySelectionEnabled: () => false, + setPrimarySelectionText: () => {} +})) + +describe('setupContextualCopy', () => { + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('clears editor-scoped contextual copy cleanup on dispose', () => { + const clearTimeout = vi.fn() + const clearInterval = vi.fn() + vi.stubGlobal('window', { + clearInterval, + clearTimeout, + setInterval: vi.fn(() => 1), + setTimeout: vi.fn(() => 2) + }) + vi.stubGlobal('document', { + createElement: () => ({ + className: '', + offsetHeight: 28, + style: { display: '' }, + textContent: '' + }) + }) + + const editorDomNode = { + addEventListener: vi.fn(), + removeEventListener: vi.fn() + } + const selectionDispose = vi.fn() + const scrollDispose = vi.fn() + const focusDispose = vi.fn() + const blurDispose = vi.fn() + let disposeEditor = (): void => {} + const editorInstance = { + addContentWidget: vi.fn(), + getContainerDomNode: () => editorDomNode, + getModel: () => null, + getSelection: () => null, + hasTextFocus: () => false, + layoutContentWidget: vi.fn(), + onDidBlurEditorText: () => ({ dispose: blurDispose }), + onDidChangeCursorSelection: () => ({ dispose: selectionDispose }), + onDidDispose: (listener: () => void) => { + disposeEditor = listener + return { dispose: vi.fn() } + }, + onDidFocusEditorText: () => ({ dispose: focusDispose }), + onDidScrollChange: () => ({ dispose: scrollDispose }), + removeContentWidget: vi.fn() + } as unknown as editor.IStandaloneCodeEditor + const copyToastTimeoutRef = { current: 42 } + const setCopyToast = vi.fn() + + setupContextualCopy({ + editorInstance, + filePath: 'src/example.ts', + setCopyToast, + propsRef: { + current: { + language: 'typescript', + relativePath: 'src/example.ts' + } + }, + copyToastTimeoutRef + }) + + disposeEditor() + + expect(selectionDispose).toHaveBeenCalledTimes(1) + expect(scrollDispose).toHaveBeenCalledTimes(1) + expect(focusDispose).toHaveBeenCalledTimes(1) + expect(blurDispose).toHaveBeenCalledTimes(1) + expect(clearTimeout).toHaveBeenCalledWith(42) + expect(copyToastTimeoutRef.current).toBeNull() + expect(setCopyToast).toHaveBeenCalledWith(null) + expect(editorDomNode.removeEventListener).toHaveBeenCalledTimes(3) + }) +}) diff --git a/src/renderer/src/components/editor/setup-contextual-copy.ts b/src/renderer/src/components/editor/setup-contextual-copy.ts index baf34a4a8f4..6f94b7a5e88 100644 --- a/src/renderer/src/components/editor/setup-contextual-copy.ts +++ b/src/renderer/src/components/editor/setup-contextual-copy.ts @@ -255,7 +255,7 @@ export function setupContextualCopy({ return true } - editorInstance.onDidChangeCursorSelection((event) => { + const selectionListener = editorInstance.onDidChangeCursorSelection((event) => { if (event.source !== 'restoreState') { schedulePrimarySelectionBufferUpdate() } @@ -264,13 +264,13 @@ export function setupContextualCopy({ } updateCopyHint() }) - editorInstance.onDidScrollChange(() => { + const scrollListener = editorInstance.onDidScrollChange(() => { updateCopyHint() }) - editorInstance.onDidFocusEditorText(() => { + const focusListener = editorInstance.onDidFocusEditorText(() => { startCopyHintPolling() }) - editorInstance.onDidBlurEditorText(() => { + const blurListener = editorInstance.onDidBlurEditorText(() => { stopCopyHintPolling() copyHintNode.style.display = 'none' copyHintWidgetPosition = null @@ -294,6 +294,12 @@ export function setupContextualCopy({ editorDomNode.addEventListener('mouseup', updateCopyHint, true) editorDomNode.addEventListener('keyup', updateCopyHint, true) editorInstance.onDidDispose(() => { + // Why: Monaco owns these emitters, but disposing explicitly keeps this + // feature's lifecycle symmetrical with the DOM listener cleanup below. + selectionListener.dispose() + scrollListener.dispose() + focusListener.dispose() + blurListener.dispose() // Why: the confirmation toast timeout belongs to the Monaco editor that // scheduled it, so editor disposal is the earliest reliable cleanup point. if (copyToastTimeoutRef.current !== null) {