Keep Markdown completions available when a sibling pane changes files (#34975)

* Keep Markdown completion catalogs owned by surviving panes

* Assign completion test editor ref in a layout effect

---------

Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>

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<OnMount>[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<editor.IStandaloneCodeEditor | null>(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([])
+  })
+})

src/renderer/src/components/editor/monaco-markdown-doc-completions.ts

@@ -10,7 +10,12 @@ type MonacoApi = Parameters<OnMount>[1]
 
 let provider: IDisposable | null = null
 let providerMonaco: MonacoApi = null
-let documentsByModel = new WeakMap<editor.ITextModel, MarkdownDocument[]>()
+type ModelCompletionDocuments = {
+  owners: Map<symbol, MarkdownDocument[]>
+  documents: MarkdownDocument[]
+}
+
+let documentsByModel = new WeakMap<editor.ITextModel, ModelCompletionDocuments>()
 
 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
+  }
 }

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<editor.ITextModel | null>(null)
   const markdownDocLinkDecorationsRef = useRef<MarkdownDocLinkDecorationController | null>(null)
   const conflictDecorationsRef = useRef<editor.IEditorDecorationsCollection | null>(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