diff --git a/src/renderer/src/components/editor/monaco-markdown-doc-completions-retention.test.ts b/src/renderer/src/components/editor/monaco-markdown-doc-completions-retention.test.ts index caa69321d9e..e0460b88fbb 100644 --- a/src/renderer/src/components/editor/monaco-markdown-doc-completions-retention.test.ts +++ b/src/renderer/src/components/editor/monaco-markdown-doc-completions-retention.test.ts @@ -1,4 +1,9 @@ -import { describe, expect, it, vi } from 'vitest' +// @vitest-environment happy-dom +import { afterEach, describe, expect, it, vi } from 'vitest' +import { cleanup, render } from '@testing-library/react' +import { createElement, useLayoutEffect, useRef } from 'react' +import type { MarkdownDocument } from '../../../../shared/filesystem-entry-types' +import { useMonacoEditorDecorations } from './use-monaco-editor-decorations' import type { editor, languages, Position } from 'monaco-editor' import type { OnMount } from '@monaco-editor/react' import { @@ -7,6 +12,8 @@ import { clearMarkdownDocCompletionDocuments } from './monaco-markdown-doc-completions' +afterEach(cleanup) + type MonacoApi = Parameters[1] function model(key: string): editor.ITextModel { @@ -52,7 +59,10 @@ function register() { textModel, position, { triggerKind: 0 }, - { isCancellationRequested: false, onCancellationRequested: () => ({ dispose() {} }) } + { + isCancellationRequested: false, + onCancellationRequested: () => ({ dispose() {} }) + } ) return result?.suggestions ?? [] } @@ -60,7 +70,12 @@ function register() { function documents(name: string) { return [ - { filePath: `/repo/${name}.md`, relativePath: `${name}.md`, basename: `${name}.md`, name } + { + filePath: `/repo/${name}.md`, + relativePath: `${name}.md`, + basename: `${name}.md`, + name + } ] } @@ -68,22 +83,108 @@ describe('live Markdown completion ownership', () => { it('preserves every mounted model when many other scopes are supplied', async () => { const complete = register() const active = model('active') - setMarkdownDocCompletionDocuments(active, documents('Target')) + const owner = Symbol() + setMarkdownDocCompletionDocuments(active, documents('Target'), owner) for (let scope = 0; scope < 300; scope++) { - setMarkdownDocCompletionDocuments(model(`scope-${scope}`), documents(`Target-${scope}`)) + const other = model(`scope-${scope}`) + setMarkdownDocCompletionDocuments(other, documents(`Target-${scope}`), Symbol()) } expect((await complete(active)).map((item) => item.label)).toEqual(['Target']) - clearMarkdownDocCompletionDocuments(active) + clearMarkdownDocCompletionDocuments(active, owner) expect(await complete(active)).toEqual([]) }) it('keeps different model incarnations separate even when their URI is identical', async () => { const complete = register() const old = model('same-uri') + const owner = Symbol() const current = model('same-uri') - setMarkdownDocCompletionDocuments(old, documents('Target old')) - setMarkdownDocCompletionDocuments(current, documents('Target current')) - clearMarkdownDocCompletionDocuments(old) + setMarkdownDocCompletionDocuments(old, documents('Target old'), owner) + setMarkdownDocCompletionDocuments(current, documents('Target current'), owner) + clearMarkdownDocCompletionDocuments(old, owner) expect((await complete(current)).map((item) => item.label)).toEqual(['Target current']) }) }) + +function Pane({ + textModel, + catalog, + language = 'markdown' +}: { + textModel: editor.ITextModel + catalog: MarkdownDocument[] + language?: string +}): null { + const editorRef = useRef(null) + // Layout effects commit before the hook's passive effects read the editor. + useLayoutEffect(() => { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: completion registration only reads the editor's model in this hook fixture. + editorRef.current = { + getModel: () => textModel + } as editor.IStandaloneCodeEditor + }, [textModel]) + useMonacoEditorDecorations({ + editorRef, + mountedEditor: null, + content: '[[Ta', + language, + markdownDocuments: catalog, + conflictDecorationsEnabled: false + }) + return null +} + +describe('pane-owned Markdown completion catalogs', () => { + it('preserves a shared-model survivor and releases its catalog on last close', async () => { + const complete = register() + const shared = model('shared') + const catalog = documents('Target') + const first = render(createElement(Pane, { textModel: shared, catalog })) + const second = render(createElement(Pane, { textModel: shared, catalog })) + expect((await complete(shared)).map((item) => item.label)).toEqual(['Target']) + + first.unmount() + expect((await complete(shared)).map((item) => item.label)).toEqual(['Target']) + second.rerender( + createElement(Pane, { + textModel: shared, + catalog: documents('Target next') + }) + ) + expect((await complete(shared)).map((item) => item.label)).toEqual(['Target next']) + second.unmount() + expect(await complete(shared)).toEqual([]) + }) + + it('restores surviving catalogs on language changes and isolates replacement models', async () => { + const complete = register() + const shared = model('same-uri') + const replacement = model('same-uri') + const first = render( + createElement(Pane, { + textModel: shared, + catalog: documents('Target first') + }) + ) + const catalog = documents('Target second') + const second = render(createElement(Pane, { textModel: shared, catalog })) + expect((await complete(shared)).map((item) => item.label)).toEqual(['Target second']) + + second.rerender( + createElement(Pane, { + textModel: shared, + catalog, + language: 'plaintext' + }) + ) + expect((await complete(shared)).map((item) => item.label)).toEqual(['Target first']) + second.rerender(createElement(Pane, { textModel: replacement, catalog })) + expect((await complete(shared)).map((item) => item.label)).toEqual(['Target first']) + expect((await complete(replacement)).map((item) => item.label)).toEqual(['Target second']) + second.unmount() + expect(await complete(replacement)).toEqual([]) + expect((await complete(shared)).map((item) => item.label)).toEqual(['Target first']) + first.unmount() + expect(await complete(shared)).toEqual([]) + }) +}) diff --git a/src/renderer/src/components/editor/monaco-markdown-doc-completions.ts b/src/renderer/src/components/editor/monaco-markdown-doc-completions.ts index cd4efb98fd3..c307d3c312c 100644 --- a/src/renderer/src/components/editor/monaco-markdown-doc-completions.ts +++ b/src/renderer/src/components/editor/monaco-markdown-doc-completions.ts @@ -10,7 +10,12 @@ type MonacoApi = Parameters[1] let provider: IDisposable | null = null let providerMonaco: MonacoApi = null -let documentsByModel = new WeakMap() +type ModelCompletionDocuments = { + owners: Map + documents: MarkdownDocument[] +} + +let documentsByModel = new WeakMap() export function ensureMarkdownDocCompletionProvider(monaco: MonacoApi): void { // Re-register when Monaco is recreated; the old provider belongs to the previous instance. @@ -32,7 +37,7 @@ export function ensureMarkdownDocCompletionProvider(monaco: MonacoApi): void { return { suggestions: [] } } - const documents = documentsByModel.get(model) ?? [] + const documents = documentsByModel.get(model)?.documents ?? [] const suffix = line.slice(position.column - 1) const range = { startLineNumber: position.lineNumber, @@ -58,11 +63,27 @@ export function ensureMarkdownDocCompletionProvider(monaco: MonacoApi): void { export function setMarkdownDocCompletionDocuments( model: editor.ITextModel, - documents: MarkdownDocument[] + documents: MarkdownDocument[], + owner: symbol ): void { - documentsByModel.set(model, documents) + const entry = documentsByModel.get(model) ?? { owners: new Map(), documents } + entry.owners.delete(owner) + entry.owners.set(owner, documents) + entry.documents = documents + documentsByModel.set(model, entry) } -export function clearMarkdownDocCompletionDocuments(model: editor.ITextModel): void { - documentsByModel.delete(model) +export function clearMarkdownDocCompletionDocuments(model: editor.ITextModel, owner: symbol): void { + const entry = documentsByModel.get(model) + if (!entry?.owners.delete(owner)) { + return + } + if (entry.owners.size === 0) { + documentsByModel.delete(model) + return + } + // Closing the latest supplier restores the newest catalog still owned by a mounted pane. + for (const documents of entry.owners.values()) { + entry.documents = documents + } } diff --git a/src/renderer/src/components/editor/use-monaco-editor-decorations.ts b/src/renderer/src/components/editor/use-monaco-editor-decorations.ts index dda1d5e117a..2b64f61c583 100644 --- a/src/renderer/src/components/editor/use-monaco-editor-decorations.ts +++ b/src/renderer/src/components/editor/use-monaco-editor-decorations.ts @@ -32,6 +32,7 @@ export function useMonacoEditorDecorations(params: { conflictDecorationsEnabled } = params + const completionOwnerRef = useRef(Symbol('markdown-completion')) const completionModelRef = useRef(null) const markdownDocLinkDecorationsRef = useRef(null) const conflictDecorationsRef = useRef(null) @@ -39,16 +40,16 @@ export function useMonacoEditorDecorations(params: { const updateMarkdownCompletionDocuments = useCallback((): void => { const model = editorRef.current?.getModel() ?? null if (completionModelRef.current && completionModelRef.current !== model) { - clearMarkdownDocCompletionDocuments(completionModelRef.current) + clearMarkdownDocCompletionDocuments(completionModelRef.current, completionOwnerRef.current) } completionModelRef.current = model if (!model) { return } if (language === 'markdown' && markdownDocuments) { - setMarkdownDocCompletionDocuments(model, markdownDocuments) + setMarkdownDocCompletionDocuments(model, markdownDocuments, completionOwnerRef.current) } else { - clearMarkdownDocCompletionDocuments(model) + clearMarkdownDocCompletionDocuments(model, completionOwnerRef.current) } }, [editorRef, language, markdownDocuments]) @@ -86,9 +87,10 @@ export function useMonacoEditorDecorations(params: { }, [updateMarkdownCompletionDocuments]) useEffect(() => { + const completionOwner = completionOwnerRef.current return () => { if (completionModelRef.current) { - clearMarkdownDocCompletionDocuments(completionModelRef.current) + clearMarkdownDocCompletionDocuments(completionModelRef.current, completionOwner) } markdownDocLinkDecorationsRef.current?.dispose() markdownDocLinkDecorationsRef.current = null