mirror of
https://github.com/stablyai/orca.git
synced 2026-10-12 08:02:37 +00:00
fix(editor): correct URI predicate + relocate close cleanup to App level
Two follow-up bugs from PR #1353: 1. findLeakedDiffModelUris compared monaco.Uri.toString() against literal `:` prefixes, but Monaco percent-encodes `:` to `%3A` in toString(). Real-world tab ids are absolute file paths whose `:` characters got encoded, so the predicate matched zero models in production. Fixed by matching against uri.path (which preserves literal `:`) and renamed the helper to findLeakedDiffModelPaths to reflect the new shape. 2. The tab-close cleanup useEffect lived inside EditorPanel, but EditorPanel is conditionally mounted on the active tab — so when the active tab is closed, the panel unmounts before its effect can observe the change. Extracted the cleanup into useEditorTabCloseCleanup and mount it at App level so it observes openFiles regardless of which panel is active. Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
brennanb2025
Orca
@@ -28,6 +28,7 @@ import { TelemetryFirstLaunchSurface } from './components/TelemetryFirstLaunchSu
|
||||
import { ZoomOverlay } from './components/ZoomOverlay'
|
||||
import { SshPassphraseDialog } from './components/settings/SshPassphraseDialog'
|
||||
import { useGitStatusPolling } from './components/right-sidebar/useGitStatusPolling'
|
||||
import { useEditorTabCloseCleanup } from './components/editor/use-editor-tab-close-cleanup'
|
||||
import { useEditorExternalWatch } from './hooks/useEditorExternalWatch'
|
||||
import { useAutoAckViewedAgent } from './hooks/useAutoAckViewedAgent'
|
||||
import {
|
||||
@@ -192,6 +193,10 @@ function App(): React.JSX.Element {
|
||||
// sidebar is closed, which leaves stale "Rebasing"/"Merging" badges behind
|
||||
// until some unrelated view remount happens to refresh them.
|
||||
useGitStatusPolling()
|
||||
// Why: tab-close cleanup (Monaco model disposal, scroll/cursor cache eviction)
|
||||
// must run from an always-mounted hook. EditorPanel unmounts when its active
|
||||
// tab closes — see use-editor-tab-close-cleanup.ts for details.
|
||||
useEditorTabCloseCleanup()
|
||||
// Why: the editor must hear external filesystem changes regardless of
|
||||
// which right-sidebar panel is visible (Explorer unmounts when the user
|
||||
// switches to Source Control or Checks). Wiring this at App level mirrors
|
||||
|
||||
@@ -4,7 +4,6 @@ keeping that UI state together is easier to reason about than scattering it
|
||||
across multiple components. Autosave now lives in a smaller headless controller
|
||||
so hidden editor UI no longer participates in shutdown. */
|
||||
import React, { useCallback, useEffect, useRef, useState, Suspense } from 'react'
|
||||
import * as monaco from 'monaco-editor'
|
||||
import { Columns2, Copy, Eye, ExternalLink, FileText, MoreHorizontal, Rows2 } from 'lucide-react'
|
||||
import { useAppStore } from '@/store'
|
||||
import { findWorktreeById } from '@/store/slices/worktree-helpers'
|
||||
@@ -25,8 +24,6 @@ import { CLOSE_ALL_CONTEXT_MENUS_EVENT } from '../tab-bar/SortableTab'
|
||||
import type { MarkdownViewMode, OpenFile } from '@/store/slices/editor'
|
||||
import EditorViewToggle, { CSV_VIEW_MODE_METADATA } from './EditorViewToggle'
|
||||
import { EditorContent } from './EditorContent'
|
||||
import { scrollTopCache, cursorPositionCache, diffViewStateCache } from '@/lib/scroll-cache'
|
||||
import { findLeakedDiffModelUris } from './editor-model-leak'
|
||||
import type { GitDiffResult } from '../../../../shared/types'
|
||||
import {
|
||||
getOpenFilesForExternalFileChange,
|
||||
@@ -192,14 +189,6 @@ function EditorPanelInner({
|
||||
const [pathMenuPoint, setPathMenuPoint] = useState({ x: 0, y: 0 })
|
||||
const panelRef = useRef<HTMLDivElement>(null)
|
||||
|
||||
const deleteCacheEntriesByPrefix = useCallback(<T,>(cache: Map<string, T>, prefix: string) => {
|
||||
for (const key of cache.keys()) {
|
||||
if (key.startsWith(prefix)) {
|
||||
cache.delete(key)
|
||||
}
|
||||
}
|
||||
}, [])
|
||||
|
||||
// Why: When the user changes their global diff-view preference in Settings,
|
||||
// sync the local toggle to match during render (avoids flash of stale diff mode).
|
||||
if (settings?.diffDefaultView !== prevDiffView) {
|
||||
@@ -237,110 +226,10 @@ function EditorPanelInner({
|
||||
// churn as long as at least one panel is still mounted.
|
||||
useEffect(() => acquireExportPdfListener(), [])
|
||||
|
||||
// Why: keepCurrentModel / keepCurrent*Model retain Monaco models after unmount
|
||||
// so undo history survives tab switches. When a tab is *closed*, the user has
|
||||
// signalled they're done with the file — dispose the models to reclaim memory
|
||||
// and delete cache entries so a reopened file starts fresh.
|
||||
const prevOpenFilesRef = useRef<Map<string, OpenFile>>(new Map())
|
||||
|
||||
useEffect(() => {
|
||||
const currentFilesById = new Map(openFiles.map((f) => [f.id, f]))
|
||||
for (const [prevId, prevFile] of prevOpenFilesRef.current) {
|
||||
if (!currentFilesById.has(prevId)) {
|
||||
// Dispose only the kept-alive Monaco state that this tab mode owns.
|
||||
// Why: edit and diff tabs use different retained-model keys, while the
|
||||
// conflict-review surface does not create kept Monaco models today. An
|
||||
// explicit switch makes that ownership boundary visible so future mode
|
||||
// additions do not silently fall through without considering cleanup.
|
||||
switch (prevFile.mode) {
|
||||
case 'edit': {
|
||||
// Why: the edit model URI is constructed via monaco.Uri.parse(filePath)
|
||||
// to match what @monaco-editor/react creates internally when the `path`
|
||||
// prop is provided. This convention is version-dependent.
|
||||
monaco.editor.getModel(monaco.Uri.parse(prevFile.filePath))?.dispose()
|
||||
// Why: edit-mode tabs that ever entered Changes view mode mounted a
|
||||
// DiffViewer with `keepCurrent*Model`, which leaves both diff models
|
||||
// in `monaco.editor.getModels()` after this tab closes. Disposing
|
||||
// the plain edit model above does not touch them. Iterate the model
|
||||
// registry and dispose any that match this tab's id (covers both
|
||||
// single-pane and split-pane URI shapes plus the rotated-original
|
||||
// hash). `closeFile` clears `editorViewMode[fileId]` synchronously
|
||||
// so we cannot gate on it; doing the prefix scan unconditionally
|
||||
// is a no-op for tabs that never entered Changes mode.
|
||||
const allModels = monaco.editor.getModels()
|
||||
const leakedUris = new Set(
|
||||
findLeakedDiffModelUris(
|
||||
allModels.map((m) => m.uri.toString()),
|
||||
prevId,
|
||||
'changes'
|
||||
)
|
||||
)
|
||||
for (const model of allModels) {
|
||||
if (leakedUris.has(model.uri.toString())) {
|
||||
model.dispose()
|
||||
}
|
||||
}
|
||||
diffViewStateCache.delete(prevId)
|
||||
deleteCacheEntriesByPrefix(diffViewStateCache, `${prevId}::`)
|
||||
scrollTopCache.delete(prevFile.filePath)
|
||||
deleteCacheEntriesByPrefix(scrollTopCache, `${prevFile.filePath}::`)
|
||||
// Why: markdown edit tabs keep separate source/rich scroll caches,
|
||||
// and older sessions may still have the legacy in-place preview key.
|
||||
// Clear all of them so reopened files never inherit stale viewport
|
||||
// state from a prior tab incarnation.
|
||||
scrollTopCache.delete(`${prevFile.filePath}:rich`)
|
||||
scrollTopCache.delete(`${prevFile.filePath}:preview`)
|
||||
// Why: mermaid files use a mode-scoped cache key just like markdown.
|
||||
// Without this, a reopened .mmd file would restore a stale scroll
|
||||
// position from the previous session even if the content changed.
|
||||
scrollTopCache.delete(`${prevFile.filePath}:mermaid-diagram`)
|
||||
cursorPositionCache.delete(prevFile.filePath)
|
||||
deleteCacheEntriesByPrefix(cursorPositionCache, `${prevFile.filePath}::`)
|
||||
break
|
||||
}
|
||||
case 'markdown-preview':
|
||||
// Why: preview tabs have no retained Monaco models, but they do
|
||||
// own pane-scoped preview scroll cache entries that should be
|
||||
// dropped on close so reopening the preview starts fresh.
|
||||
scrollTopCache.delete(`${prevFile.id}:preview`)
|
||||
deleteCacheEntriesByPrefix(scrollTopCache, `${prevFile.id}::`)
|
||||
break
|
||||
case 'diff': {
|
||||
// Why: kept diff models are keyed by tab id, not file path, because the
|
||||
// same file can appear in multiple diff tabs with different contents.
|
||||
// Split-pane layouts append `::${viewStateScopeId}` to the URI, so a
|
||||
// prefix scan over the model registry catches the secondary-pane
|
||||
// models that an exact-URI lookup would miss.
|
||||
const allModels = monaco.editor.getModels()
|
||||
const leakedUris = new Set(
|
||||
findLeakedDiffModelUris(
|
||||
allModels.map((m) => m.uri.toString()),
|
||||
prevId,
|
||||
'diff'
|
||||
)
|
||||
)
|
||||
for (const model of allModels) {
|
||||
if (leakedUris.has(model.uri.toString())) {
|
||||
model.dispose()
|
||||
}
|
||||
}
|
||||
diffViewStateCache.delete(prevId)
|
||||
deleteCacheEntriesByPrefix(diffViewStateCache, `${prevId}::`)
|
||||
// Why: single-file markdown diffs now have a rendered preview mode
|
||||
// whose scroll position is keyed off the diff tab identity rather
|
||||
// than a Monaco view-state cache entry. Clear those mode-scoped
|
||||
// keys alongside the diff models so reopened diff tabs start fresh.
|
||||
scrollTopCache.delete(`${prevId}:preview`)
|
||||
deleteCacheEntriesByPrefix(scrollTopCache, `${prevId}::`)
|
||||
break
|
||||
}
|
||||
case 'conflict-review':
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
prevOpenFilesRef.current = currentFilesById
|
||||
}, [deleteCacheEntriesByPrefix, openFiles])
|
||||
// Why: tab-close cleanup (Monaco model disposal, scroll/cursor cache eviction)
|
||||
// lives in `useEditorTabCloseCleanup` mounted at the App level. EditorPanel
|
||||
// unmounts whenever its active tab closes, so an effect inside this component
|
||||
// cannot reliably observe the close event for the active tab.
|
||||
|
||||
// Load file content when active file changes
|
||||
useEffect(() => {
|
||||
|
||||
@@ -1,116 +1,143 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { findLeakedDiffModelUris } from './editor-model-leak'
|
||||
import { findLeakedDiffModelPaths, type DiffModelCandidate } from './editor-model-leak'
|
||||
|
||||
describe('findLeakedDiffModelUris — changes mode', () => {
|
||||
function diffModel(path: string): DiffModelCandidate {
|
||||
return { scheme: 'diff', path }
|
||||
}
|
||||
|
||||
describe('findLeakedDiffModelPaths — changes mode', () => {
|
||||
const tabId = 'tab-abc'
|
||||
|
||||
it('flags single-pane modified and rotated original URIs', () => {
|
||||
const uris = [
|
||||
`diff:modified:${tabId}`,
|
||||
`diff:original:${tabId}:original:deadbeef`,
|
||||
`diff:original:other-tab:original:cafebabe`,
|
||||
`diff:modified:other-tab`,
|
||||
`unrelated:foo`
|
||||
it('flags single-pane modified and rotated original paths', () => {
|
||||
const models = [
|
||||
diffModel(`modified:${tabId}`),
|
||||
diffModel(`original:${tabId}:original:deadbeef`),
|
||||
diffModel(`original:other-tab:original:cafebabe`),
|
||||
diffModel(`modified:other-tab`),
|
||||
{ scheme: 'unrelated', path: 'foo' }
|
||||
]
|
||||
expect(findLeakedDiffModelUris(uris, tabId, 'changes')).toEqual([
|
||||
`diff:modified:${tabId}`,
|
||||
`diff:original:${tabId}:original:deadbeef`
|
||||
expect(findLeakedDiffModelPaths(models, tabId, 'changes')).toEqual([
|
||||
`modified:${tabId}`,
|
||||
`original:${tabId}:original:deadbeef`
|
||||
])
|
||||
})
|
||||
|
||||
it('flags split-pane modified and rotated original URIs', () => {
|
||||
const uris = [
|
||||
`diff:modified:${tabId}::pane-2`,
|
||||
`diff:original:${tabId}::pane-2:original:deadbeef`,
|
||||
`diff:modified:${tabId}::pane-3`,
|
||||
`diff:original:${tabId}::pane-3:original:cafebabe`
|
||||
it('flags split-pane modified and rotated original paths', () => {
|
||||
const models = [
|
||||
diffModel(`modified:${tabId}::pane-2`),
|
||||
diffModel(`original:${tabId}::pane-2:original:deadbeef`),
|
||||
diffModel(`modified:${tabId}::pane-3`),
|
||||
diffModel(`original:${tabId}::pane-3:original:cafebabe`)
|
||||
]
|
||||
expect(findLeakedDiffModelUris(uris, tabId, 'changes').sort()).toEqual(
|
||||
expect(findLeakedDiffModelPaths(models, tabId, 'changes').sort()).toEqual(
|
||||
[
|
||||
`diff:modified:${tabId}::pane-2`,
|
||||
`diff:modified:${tabId}::pane-3`,
|
||||
`diff:original:${tabId}::pane-2:original:deadbeef`,
|
||||
`diff:original:${tabId}::pane-3:original:cafebabe`
|
||||
`modified:${tabId}::pane-2`,
|
||||
`modified:${tabId}::pane-3`,
|
||||
`original:${tabId}::pane-2:original:deadbeef`,
|
||||
`original:${tabId}::pane-3:original:cafebabe`
|
||||
].sort()
|
||||
)
|
||||
})
|
||||
|
||||
it('handles multiple rotations of the same tab simultaneously', () => {
|
||||
const uris = [
|
||||
`diff:modified:${tabId}`,
|
||||
`diff:original:${tabId}:original:hash1`,
|
||||
`diff:original:${tabId}:original:hash2`,
|
||||
`diff:original:${tabId}:original:hash3`
|
||||
const models = [
|
||||
diffModel(`modified:${tabId}`),
|
||||
diffModel(`original:${tabId}:original:hash1`),
|
||||
diffModel(`original:${tabId}:original:hash2`),
|
||||
diffModel(`original:${tabId}:original:hash3`)
|
||||
]
|
||||
expect(findLeakedDiffModelUris(uris, tabId, 'changes').sort()).toEqual(
|
||||
expect(findLeakedDiffModelPaths(models, tabId, 'changes').sort()).toEqual(
|
||||
[
|
||||
`diff:modified:${tabId}`,
|
||||
`diff:original:${tabId}:original:hash1`,
|
||||
`diff:original:${tabId}:original:hash2`,
|
||||
`diff:original:${tabId}:original:hash3`
|
||||
`modified:${tabId}`,
|
||||
`original:${tabId}:original:hash1`,
|
||||
`original:${tabId}:original:hash2`,
|
||||
`original:${tabId}:original:hash3`
|
||||
].sort()
|
||||
)
|
||||
})
|
||||
|
||||
it('does not match a tab id whose prefix is shared with a longer id', () => {
|
||||
const uris = [
|
||||
`diff:modified:${tabId}-extended`,
|
||||
`diff:original:${tabId}-extended:original:hash`
|
||||
const models = [
|
||||
diffModel(`modified:${tabId}-extended`),
|
||||
diffModel(`original:${tabId}-extended:original:hash`)
|
||||
]
|
||||
expect(findLeakedDiffModelUris(uris, tabId, 'changes')).toEqual([])
|
||||
expect(findLeakedDiffModelPaths(models, tabId, 'changes')).toEqual([])
|
||||
})
|
||||
|
||||
it('does not flag a plain-diff style original URI in changes mode', () => {
|
||||
// Why: in changes mode the original URI always has a `:original:${hash}`
|
||||
// suffix, so a bare `diff:original:${tabId}` (no trailing `:`) belongs to
|
||||
it('does not flag a plain-diff style original path in changes mode', () => {
|
||||
// Why: in changes mode the original path always has a `:original:${hash}`
|
||||
// suffix, so a bare `original:${tabId}` (no trailing `:`) belongs to
|
||||
// a plain diff tab and must not match.
|
||||
const uris = [`diff:original:${tabId}`]
|
||||
expect(findLeakedDiffModelUris(uris, tabId, 'changes')).toEqual([])
|
||||
const models = [diffModel(`original:${tabId}`)]
|
||||
expect(findLeakedDiffModelPaths(models, tabId, 'changes')).toEqual([])
|
||||
})
|
||||
|
||||
it('matches when tab id is an absolute file path (real-world Changes-mode shape)', () => {
|
||||
// Why: the real-world tab id is an absolute file path, so the URI looks
|
||||
// like `monaco.Uri.parse('diff:modified:' + filePath + '::' + scope)`.
|
||||
// Monaco's `URI.toString()` percent-encodes the `:` characters in the
|
||||
// path segment, but `URI.path` keeps them literal — which is why the
|
||||
// helper matches against `path` rather than `toString()`. This locks in
|
||||
// the encoding-mismatch fix.
|
||||
const filePath = '/Users/me/repo/file.md'
|
||||
const scope = 'scope-id-1'
|
||||
const hash = 'a9ce8f37'
|
||||
const models = [
|
||||
diffModel(`modified:${filePath}::${scope}`),
|
||||
diffModel(`original:${filePath}::${scope}:original:${hash}`)
|
||||
]
|
||||
expect(findLeakedDiffModelPaths(models, filePath, 'changes').sort()).toEqual(
|
||||
[`modified:${filePath}::${scope}`, `original:${filePath}::${scope}:original:${hash}`].sort()
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('findLeakedDiffModelUris — diff mode', () => {
|
||||
describe('findLeakedDiffModelPaths — diff mode', () => {
|
||||
const tabId = 'tab-xyz'
|
||||
|
||||
it('flags single-pane modified and exact original URIs', () => {
|
||||
const uris = [
|
||||
`diff:modified:${tabId}`,
|
||||
`diff:original:${tabId}`,
|
||||
`diff:modified:other-tab`,
|
||||
`diff:original:other-tab`
|
||||
it('flags single-pane modified and exact original paths', () => {
|
||||
const models = [
|
||||
diffModel(`modified:${tabId}`),
|
||||
diffModel(`original:${tabId}`),
|
||||
diffModel(`modified:other-tab`),
|
||||
diffModel(`original:other-tab`)
|
||||
]
|
||||
expect(findLeakedDiffModelUris(uris, tabId, 'diff').sort()).toEqual(
|
||||
[`diff:modified:${tabId}`, `diff:original:${tabId}`].sort()
|
||||
expect(findLeakedDiffModelPaths(models, tabId, 'diff').sort()).toEqual(
|
||||
[`modified:${tabId}`, `original:${tabId}`].sort()
|
||||
)
|
||||
})
|
||||
|
||||
it('flags split-pane modified and original URIs', () => {
|
||||
const uris = [
|
||||
`diff:modified:${tabId}::pane-2`,
|
||||
`diff:original:${tabId}::pane-2`,
|
||||
`diff:modified:${tabId}::pane-3`,
|
||||
`diff:original:${tabId}::pane-3`
|
||||
it('flags split-pane modified and original paths', () => {
|
||||
const models = [
|
||||
diffModel(`modified:${tabId}::pane-2`),
|
||||
diffModel(`original:${tabId}::pane-2`),
|
||||
diffModel(`modified:${tabId}::pane-3`),
|
||||
diffModel(`original:${tabId}::pane-3`)
|
||||
]
|
||||
expect(findLeakedDiffModelUris(uris, tabId, 'diff').sort()).toEqual(
|
||||
expect(findLeakedDiffModelPaths(models, tabId, 'diff').sort()).toEqual(
|
||||
[
|
||||
`diff:modified:${tabId}::pane-2`,
|
||||
`diff:modified:${tabId}::pane-3`,
|
||||
`diff:original:${tabId}::pane-2`,
|
||||
`diff:original:${tabId}::pane-3`
|
||||
`modified:${tabId}::pane-2`,
|
||||
`modified:${tabId}::pane-3`,
|
||||
`original:${tabId}::pane-2`,
|
||||
`original:${tabId}::pane-3`
|
||||
].sort()
|
||||
)
|
||||
})
|
||||
|
||||
it('does not flag a rotated-original URI in diff mode', () => {
|
||||
// Why: plain diff tabs do not rotate the original URI, so a URI with a
|
||||
it('does not flag a rotated-original path in diff mode', () => {
|
||||
// Why: plain diff tabs do not rotate the original path, so a path with a
|
||||
// `:original:${hash}` suffix belongs to a Changes-mode tab and must not
|
||||
// be disposed when a plain diff tab closes.
|
||||
const uris = [`diff:original:${tabId}:original:hash`]
|
||||
expect(findLeakedDiffModelUris(uris, tabId, 'diff')).toEqual([])
|
||||
const models = [diffModel(`original:${tabId}:original:hash`)]
|
||||
expect(findLeakedDiffModelPaths(models, tabId, 'diff')).toEqual([])
|
||||
})
|
||||
|
||||
it('returns empty when no URIs match', () => {
|
||||
const uris = [`diff:modified:other`, `unrelated:foo`, `bar`]
|
||||
expect(findLeakedDiffModelUris(uris, tabId, 'diff')).toEqual([])
|
||||
it('returns empty when no paths match', () => {
|
||||
const models = [
|
||||
diffModel(`modified:other`),
|
||||
{ scheme: 'unrelated', path: 'foo' },
|
||||
diffModel('bar')
|
||||
]
|
||||
expect(findLeakedDiffModelPaths(models, tabId, 'diff')).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -2,39 +2,52 @@
|
||||
// edit tabs in Changes view mode survive component unmount because of
|
||||
// @monaco-editor/react's `keepCurrent*Model` flags. Tab close is the only
|
||||
// signal we get that the user is done with these models. The URI scheme
|
||||
// embeds the tab id, so a prefix scan over `monaco.editor.getModels()`
|
||||
// reliably finds every leaked model — including split-pane variants whose
|
||||
// URIs gain a `::${viewStateScopeId}` segment, and Changes-mode original
|
||||
// models whose URIs gain a `:original:${hash}` rotation segment.
|
||||
// embeds the tab id, so a prefix match against `monaco.Uri.path` reliably
|
||||
// finds every leaked model — including split-pane variants whose paths
|
||||
// gain a `::${viewStateScopeId}` segment, and Changes-mode original models
|
||||
// whose paths gain a `:original:${hash}` rotation segment.
|
||||
//
|
||||
// Why match against `uri.path` rather than `uri.toString()`: Monaco's
|
||||
// `URI.toString()` percent-encodes `:` to `%3A`, but the constructed paths
|
||||
// embed literal `:` as a delimiter. Comparing against `uri.path` keeps the
|
||||
// predicate readable and avoids re-encoding the prefixes on every call.
|
||||
|
||||
export type DiffModelLeakMode = 'changes' | 'diff'
|
||||
|
||||
export function findLeakedDiffModelUris(
|
||||
uris: readonly string[],
|
||||
export type DiffModelCandidate = {
|
||||
scheme: string
|
||||
path: string
|
||||
}
|
||||
|
||||
export function findLeakedDiffModelPaths(
|
||||
models: readonly DiffModelCandidate[],
|
||||
tabId: string,
|
||||
mode: DiffModelLeakMode
|
||||
): string[] {
|
||||
const modifiedPrefix = `diff:modified:${tabId}`
|
||||
const originalPrefix = `diff:original:${tabId}`
|
||||
const modifiedPrefix = `modified:${tabId}`
|
||||
const originalPrefix = `original:${tabId}`
|
||||
const out: string[] = []
|
||||
for (const uri of uris) {
|
||||
if (uri === modifiedPrefix || uri.startsWith(`${modifiedPrefix}::`)) {
|
||||
out.push(uri)
|
||||
for (const { scheme, path } of models) {
|
||||
if (scheme !== 'diff') {
|
||||
continue
|
||||
}
|
||||
if (path === modifiedPrefix || path.startsWith(`${modifiedPrefix}::`)) {
|
||||
out.push(path)
|
||||
continue
|
||||
}
|
||||
if (mode === 'changes') {
|
||||
// Why: Changes mode rotates the original-side URI by appending
|
||||
// Why: Changes mode rotates the original-side path by appending
|
||||
// `:original:${hash}` (single-pane) or `::${scope}:original:${hash}`
|
||||
// (split-pane). Both are caught by the trailing `:` prefix.
|
||||
if (uri.startsWith(`${originalPrefix}:`)) {
|
||||
out.push(uri)
|
||||
if (path.startsWith(`${originalPrefix}:`)) {
|
||||
out.push(path)
|
||||
}
|
||||
} else {
|
||||
// Why: plain diff tabs use the original prefix exactly for single-pane
|
||||
// and `::${scope}` for split-pane. `::` is tighter than `:` and avoids
|
||||
// accidentally matching unrelated URIs that happen to share a prefix.
|
||||
if (uri === originalPrefix || uri.startsWith(`${originalPrefix}::`)) {
|
||||
out.push(uri)
|
||||
// accidentally matching unrelated paths that happen to share a prefix.
|
||||
if (path === originalPrefix || path.startsWith(`${originalPrefix}::`)) {
|
||||
out.push(path)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,131 @@
|
||||
import { useEffect, useRef } from 'react'
|
||||
import * as monaco from 'monaco-editor'
|
||||
import { useAppStore } from '@/store'
|
||||
import type { OpenFile } from '@/store/slices/editor'
|
||||
import { scrollTopCache, cursorPositionCache, diffViewStateCache } from '@/lib/scroll-cache'
|
||||
import { findLeakedDiffModelPaths } from './editor-model-leak'
|
||||
|
||||
// Why: keepCurrentModel / keepCurrent*Model retain Monaco models after unmount
|
||||
// so undo history survives tab switches. When a tab is *closed*, the user has
|
||||
// signalled they're done with the file — dispose the models to reclaim memory
|
||||
// and delete cache entries so a reopened file starts fresh.
|
||||
//
|
||||
// Why this lives at the App level rather than inside EditorPanel: EditorPanel
|
||||
// is conditionally mounted on the active tab, so when the active tab is
|
||||
// closed the panel unmounts before its own cleanup effect can run, leaking
|
||||
// the kept models. An always-mounted hook observes `openFiles` from the
|
||||
// store directly and disposes models for any tab that has disappeared,
|
||||
// regardless of which tab was active.
|
||||
|
||||
function deleteCacheEntriesByPrefix<T>(cache: Map<string, T>, prefix: string): void {
|
||||
for (const key of cache.keys()) {
|
||||
if (key.startsWith(prefix)) {
|
||||
cache.delete(key)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export function useEditorTabCloseCleanup(): void {
|
||||
const openFiles = useAppStore((s) => s.openFiles)
|
||||
const prevOpenFilesRef = useRef<Map<string, OpenFile>>(new Map())
|
||||
|
||||
useEffect(() => {
|
||||
const currentFilesById = new Map(openFiles.map((f) => [f.id, f]))
|
||||
for (const [prevId, prevFile] of prevOpenFilesRef.current) {
|
||||
if (currentFilesById.has(prevId)) {
|
||||
continue
|
||||
}
|
||||
// Dispose only the kept-alive Monaco state that this tab mode owns.
|
||||
// Why: edit and diff tabs use different retained-model keys, while the
|
||||
// conflict-review surface does not create kept Monaco models today. An
|
||||
// explicit switch makes that ownership boundary visible so future mode
|
||||
// additions do not silently fall through without considering cleanup.
|
||||
switch (prevFile.mode) {
|
||||
case 'edit': {
|
||||
// Why: the edit model URI is constructed via monaco.Uri.parse(filePath)
|
||||
// to match what @monaco-editor/react creates internally when the `path`
|
||||
// prop is provided. This convention is version-dependent.
|
||||
monaco.editor.getModel(monaco.Uri.parse(prevFile.filePath))?.dispose()
|
||||
// Why: edit-mode tabs that ever entered Changes view mode mounted a
|
||||
// DiffViewer with `keepCurrent*Model`, which leaves both diff models
|
||||
// in `monaco.editor.getModels()` after this tab closes. Disposing
|
||||
// the plain edit model above does not touch them. Iterate the model
|
||||
// registry and dispose any that match this tab's id (covers both
|
||||
// single-pane and split-pane URI shapes plus the rotated-original
|
||||
// hash). `closeFile` clears `editorViewMode[fileId]` synchronously
|
||||
// so we cannot gate on it; doing the prefix scan unconditionally
|
||||
// is a no-op for tabs that never entered Changes mode.
|
||||
const allModels = monaco.editor.getModels()
|
||||
const leakedPaths = new Set(
|
||||
findLeakedDiffModelPaths(
|
||||
allModels.map((m) => m.uri),
|
||||
prevId,
|
||||
'changes'
|
||||
)
|
||||
)
|
||||
for (const model of allModels) {
|
||||
if (model.uri.scheme === 'diff' && leakedPaths.has(model.uri.path)) {
|
||||
model.dispose()
|
||||
}
|
||||
}
|
||||
diffViewStateCache.delete(prevId)
|
||||
deleteCacheEntriesByPrefix(diffViewStateCache, `${prevId}::`)
|
||||
scrollTopCache.delete(prevFile.filePath)
|
||||
deleteCacheEntriesByPrefix(scrollTopCache, `${prevFile.filePath}::`)
|
||||
// Why: markdown edit tabs keep separate source/rich scroll caches,
|
||||
// and older sessions may still have the legacy in-place preview key.
|
||||
// Clear all of them so reopened files never inherit stale viewport
|
||||
// state from a prior tab incarnation.
|
||||
scrollTopCache.delete(`${prevFile.filePath}:rich`)
|
||||
scrollTopCache.delete(`${prevFile.filePath}:preview`)
|
||||
// Why: mermaid files use a mode-scoped cache key just like markdown.
|
||||
// Without this, a reopened .mmd file would restore a stale scroll
|
||||
// position from the previous session even if the content changed.
|
||||
scrollTopCache.delete(`${prevFile.filePath}:mermaid-diagram`)
|
||||
cursorPositionCache.delete(prevFile.filePath)
|
||||
deleteCacheEntriesByPrefix(cursorPositionCache, `${prevFile.filePath}::`)
|
||||
break
|
||||
}
|
||||
case 'markdown-preview':
|
||||
// Why: preview tabs have no retained Monaco models, but they do
|
||||
// own pane-scoped preview scroll cache entries that should be
|
||||
// dropped on close so reopening the preview starts fresh.
|
||||
scrollTopCache.delete(`${prevFile.id}:preview`)
|
||||
deleteCacheEntriesByPrefix(scrollTopCache, `${prevFile.id}::`)
|
||||
break
|
||||
case 'diff': {
|
||||
// Why: kept diff models are keyed by tab id, not file path, because the
|
||||
// same file can appear in multiple diff tabs with different contents.
|
||||
// Split-pane layouts append `::${viewStateScopeId}` to the URI, so a
|
||||
// prefix scan over the model registry catches the secondary-pane
|
||||
// models that an exact-URI lookup would miss.
|
||||
const allModels = monaco.editor.getModels()
|
||||
const leakedPaths = new Set(
|
||||
findLeakedDiffModelPaths(
|
||||
allModels.map((m) => m.uri),
|
||||
prevId,
|
||||
'diff'
|
||||
)
|
||||
)
|
||||
for (const model of allModels) {
|
||||
if (model.uri.scheme === 'diff' && leakedPaths.has(model.uri.path)) {
|
||||
model.dispose()
|
||||
}
|
||||
}
|
||||
diffViewStateCache.delete(prevId)
|
||||
deleteCacheEntriesByPrefix(diffViewStateCache, `${prevId}::`)
|
||||
// Why: single-file markdown diffs now have a rendered preview mode
|
||||
// whose scroll position is keyed off the diff tab identity rather
|
||||
// than a Monaco view-state cache entry. Clear those mode-scoped
|
||||
// keys alongside the diff models so reopened diff tabs start fresh.
|
||||
scrollTopCache.delete(`${prevId}:preview`)
|
||||
deleteCacheEntriesByPrefix(scrollTopCache, `${prevId}::`)
|
||||
break
|
||||
}
|
||||
case 'conflict-review':
|
||||
break
|
||||
}
|
||||
}
|
||||
prevOpenFilesRef.current = currentFilesById
|
||||
}, [openFiles])
|
||||
}
|
||||
Reference in New Issue
Block a user