mirror of
https://github.com/stablyai/orca.git
synced 2026-11-06 12:07:58 +00:00
perf(editor): skip full-text reads for ignored Monaco content events (#34920)
Subscribe to onDidChangeModelContent directly instead of the @monaco-editor/react onChange prop, which calls editor.getValue() before Orca's ownership and programmatic-sync checks run. Programmatic echoes (external reloads, shared-record commits, read-only live tail) no longer materialize the full model text only to discard it. Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
This commit is contained in:
@@ -137,6 +137,7 @@ export default function MonacoEditor({
|
||||
useMonacoRevealScheduler()
|
||||
const contentSync = useMonacoContentSyncBridge({
|
||||
editorRef,
|
||||
mountedEditor,
|
||||
content,
|
||||
contentRef,
|
||||
contentSyncModeRef,
|
||||
@@ -280,7 +281,6 @@ export default function MonacoEditor({
|
||||
// Use defaultValue because Orca owns later content sync; controlled values would duplicate setValue.
|
||||
defaultValue={content}
|
||||
theme={isDark ? 'vs-dark' : 'vs'}
|
||||
onChange={contentSync.handleChange}
|
||||
onMount={handleMount}
|
||||
options={{
|
||||
dropIntoEditor: { enabled: false },
|
||||
|
||||
@@ -81,9 +81,12 @@ function openSharedMarkdown() {
|
||||
.result.current
|
||||
const viewText = (id: string): string =>
|
||||
store.getState().editorDrafts[id] ?? fileContents[id].content
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: content sync only calls getModel and pushUndoStop.
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the bridge uses the real model event and text through this editor facade.
|
||||
const editorA = {
|
||||
getModel: () => model,
|
||||
getValue: () => model.getValue(),
|
||||
onDidChangeModelContent: (listener: (event: editor.IModelContentChangedEvent) => void) =>
|
||||
model.onDidChangeContent(listener),
|
||||
pushUndoStop: () => true
|
||||
} as unknown as editor.IStandaloneCodeEditor
|
||||
const contentRef = { current: viewText(a.id) }
|
||||
@@ -91,6 +94,7 @@ function openSharedMarkdown() {
|
||||
({ content }: { content: string }) =>
|
||||
useMonacoContentSyncBridge({
|
||||
editorRef: { current: editorA },
|
||||
mountedEditor: editorA,
|
||||
content,
|
||||
contentRef,
|
||||
contentSyncModeRef: { current: 'undoable' },
|
||||
@@ -107,7 +111,6 @@ function openSharedMarkdown() {
|
||||
const typeInA = (text: string): void => {
|
||||
act(() => {
|
||||
model.setValue(text)
|
||||
bridge.result.current.handleChange(text)
|
||||
})
|
||||
renderA()
|
||||
}
|
||||
|
||||
@@ -0,0 +1,89 @@
|
||||
// @vitest-environment happy-dom
|
||||
import { act, cleanup, renderHook } from '@testing-library/react'
|
||||
import * as monaco from 'monaco-editor/esm/vs/editor/editor.api.js'
|
||||
import type { editor } from 'monaco-editor'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { useMonacoContentSyncBridge } from './use-monaco-content-sync-bridge'
|
||||
import {
|
||||
beginProgrammaticContentSync,
|
||||
endProgrammaticContentSync
|
||||
} from './monaco-programmatic-sync'
|
||||
|
||||
vi.mock('./editor-stale-mount-guard', () => ({ mayPushTabTextIntoModel: () => true }))
|
||||
|
||||
afterEach(() => {
|
||||
cleanup()
|
||||
for (const model of monaco.editor.getModels()) {
|
||||
model.dispose()
|
||||
}
|
||||
})
|
||||
|
||||
function mountBridge(model: editor.ITextModel) {
|
||||
const onContentChange = vi.fn()
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the bridge uses this facade's real model event, text, and undo boundary.
|
||||
const mountedEditor = {
|
||||
getModel: () => model,
|
||||
getValue: () => model.getValue(),
|
||||
onDidChangeModelContent: (listener: (event: editor.IModelContentChangedEvent) => void) =>
|
||||
model.onDidChangeContent(listener),
|
||||
pushUndoStop: () => {
|
||||
model.pushStackElement()
|
||||
return true
|
||||
}
|
||||
} as unknown as editor.IStandaloneCodeEditor
|
||||
const editorRef: { current: editor.IStandaloneCodeEditor | null } = { current: mountedEditor }
|
||||
const bridge = renderHook(
|
||||
({ content }) =>
|
||||
useMonacoContentSyncBridge({
|
||||
editorRef,
|
||||
mountedEditor,
|
||||
content,
|
||||
contentRef: { current: content },
|
||||
contentSyncModeRef: { current: 'undoable' },
|
||||
modelKey: model.uri.toString(),
|
||||
onContentChange
|
||||
}),
|
||||
{ initialProps: { content: model.getValue() } }
|
||||
)
|
||||
return { ...bridge, onContentChange, editorRef }
|
||||
}
|
||||
|
||||
describe('Monaco content event publication', () => {
|
||||
it('does not publish programmatic echoes, while later user edits reach both panes', () => {
|
||||
const model = monaco.editor.createModel('initial')
|
||||
const first = mountBridge(model)
|
||||
const sibling = mountBridge(model)
|
||||
first.rerender({ content: 'external update' })
|
||||
expect(model.getValue()).toBe('external update')
|
||||
|
||||
const key = model.uri.toString()
|
||||
beginProgrammaticContentSync(key)
|
||||
try {
|
||||
act(() => model.setValue('another owner update'))
|
||||
} finally {
|
||||
endProgrammaticContentSync(key)
|
||||
}
|
||||
expect(first.onContentChange).not.toHaveBeenCalled()
|
||||
expect(sibling.onContentChange).not.toHaveBeenCalled()
|
||||
|
||||
act(() => model.setValue('user edit'))
|
||||
expect(first.onContentChange).toHaveBeenCalledExactlyOnceWith('user edit')
|
||||
expect(sibling.onContentChange).toHaveBeenCalledExactlyOnceWith('user edit')
|
||||
})
|
||||
|
||||
it('does not publish from an editor that no longer owns the mounted ref', () => {
|
||||
const model = monaco.editor.createModel('initial')
|
||||
const bridge = mountBridge(model)
|
||||
bridge.editorRef.current = null
|
||||
act(() => model.setValue('retired editor'))
|
||||
expect(bridge.onContentChange).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not publish after unmount', () => {
|
||||
const model = monaco.editor.createModel('initial')
|
||||
const bridge = mountBridge(model)
|
||||
bridge.unmount()
|
||||
act(() => model.setValue('after close'))
|
||||
expect(bridge.onContentChange).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useCallback, useLayoutEffect, useRef, type MutableRefObject } from 'react'
|
||||
import { useLayoutEffect, useRef, type MutableRefObject } from 'react'
|
||||
import type { editor } from 'monaco-editor'
|
||||
import { syncContentUpdate, type MonacoContentSyncMode } from './monaco-content-sync'
|
||||
import { mayPushTabTextIntoModel } from './editor-stale-mount-guard'
|
||||
@@ -13,12 +13,12 @@ export type MonacoContentSyncBridge = {
|
||||
lastSyncedContentRef: MutableRefObject<string>
|
||||
contentSyncModeRef: MutableRefObject<MonacoContentSyncMode>
|
||||
isApplyingProgrammaticContentRef: MutableRefObject<boolean>
|
||||
handleChange: (value: string | undefined) => void
|
||||
}
|
||||
|
||||
/** Callers update the content/mode refs during render so mount handlers can read current props before commit. */
|
||||
export function useMonacoContentSyncBridge(params: {
|
||||
editorRef: MutableRefObject<editor.IStandaloneCodeEditor | null>
|
||||
mountedEditor: editor.IStandaloneCodeEditor | null
|
||||
content: string
|
||||
contentRef: MutableRefObject<string>
|
||||
contentSyncModeRef: MutableRefObject<MonacoContentSyncMode>
|
||||
@@ -27,34 +27,47 @@ export function useMonacoContentSyncBridge(params: {
|
||||
fileId?: string
|
||||
onContentChange: (content: string) => void
|
||||
}): MonacoContentSyncBridge {
|
||||
const { editorRef, content, contentRef, contentSyncModeRef, modelKey, onContentChange, fileId } =
|
||||
params
|
||||
const {
|
||||
editorRef,
|
||||
mountedEditor,
|
||||
content,
|
||||
contentRef,
|
||||
contentSyncModeRef,
|
||||
modelKey,
|
||||
onContentChange,
|
||||
fileId
|
||||
} = params
|
||||
|
||||
const lastSyncedContentRef = useRef<string>(content)
|
||||
|
||||
// Suppress reconciliation’s real edit events so programmatic updates do not mark the file dirty.
|
||||
const isApplyingProgrammaticContentRef = useRef(false)
|
||||
|
||||
const handleChange = useCallback(
|
||||
(value: string | undefined) => {
|
||||
if (value !== undefined) {
|
||||
if (editorRef.current?.getModel()?.uri.toString() !== modelKey) {
|
||||
return
|
||||
}
|
||||
if (
|
||||
shouldIgnoreMonacoContentChange({
|
||||
modelKey,
|
||||
isApplyingProgrammaticContent: isApplyingProgrammaticContentRef.current
|
||||
})
|
||||
) {
|
||||
return
|
||||
}
|
||||
lastSyncedContentRef.current = value
|
||||
onContentChange(value)
|
||||
useLayoutEffect(() => {
|
||||
if (!mountedEditor) {
|
||||
return
|
||||
}
|
||||
const subscription = mountedEditor.onDidChangeModelContent(() => {
|
||||
if (
|
||||
editorRef.current !== mountedEditor ||
|
||||
mountedEditor.getModel()?.uri.toString() !== modelKey
|
||||
) {
|
||||
return
|
||||
}
|
||||
},
|
||||
[editorRef, modelKey, onContentChange]
|
||||
)
|
||||
if (
|
||||
shouldIgnoreMonacoContentChange({
|
||||
modelKey,
|
||||
isApplyingProgrammaticContent: isApplyingProgrammaticContentRef.current
|
||||
})
|
||||
) {
|
||||
return
|
||||
}
|
||||
const value = mountedEditor.getValue()
|
||||
lastSyncedContentRef.current = value
|
||||
onContentChange(value)
|
||||
})
|
||||
return () => subscription.dispose()
|
||||
}, [editorRef, modelKey, mountedEditor, onContentChange])
|
||||
|
||||
// Reconcile external prop changes before paint to avoid stale-text flashes; mount handles retained models separately.
|
||||
useLayoutEffect(() => {
|
||||
@@ -82,7 +95,6 @@ export function useMonacoContentSyncBridge(params: {
|
||||
contentRef,
|
||||
lastSyncedContentRef,
|
||||
contentSyncModeRef,
|
||||
isApplyingProgrammaticContentRef,
|
||||
handleChange
|
||||
isApplyingProgrammaticContentRef
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user