Merge pull request #36494 from overleaf/em-remove-visual-options

Remove the Options object from the visual editor extensions GitOrigin-RevId: f06e60b88ef77d3a670d7d956d1a86c116d71873
2026-11-12 13:13:45 +01:00 · 2026-09-17 11:55:18 +00:00
parent 33e2a1e282
commit 2d3474fbdb
9 changed files with 1114 additions and 1109 deletions
@@ -7,6 +7,10 @@ import { EditorView } from '@codemirror/view'
 import { EditorState, StateEffect } from '@codemirror/state'
 import useIsMounted from '@/shared/hooks/use-is-mounted'
 import { docName } from '@/features/source-editor/extensions/doc-name'
+import {
+  filePreview,
+  setFilePreview,
+} from '@/features/source-editor/extensions/file-preview'
 import {
  language,
  Metadata,
@@ -88,9 +92,8 @@ export const VisualPreview: FC<{ view: EditorView }> = ({ view }) => {
        visualHighlightStyle,
        tableGeneratorTheme,
        mousedown,
-        atomicDecorations({
-          previewByPath: previewByPathRef.current,
-        }),
+        filePreview(previewByPathRef.current),
+        atomicDecorations,
        markDecorations, // NOTE: must be after atomicDecorations, so that mark decorations wrap inline widgets
        showContentWhenParsed,
        EditorView.contentAttributes.of({ 'aria-label': 'Visual preview' }),
@@ -150,6 +153,16 @@ export const VisualPreview: FC<{ view: EditorView }> = ({ view }) => {
    }
  }, [fileTreeData, view])

+  // The ref is what the preview state is built from, so keep it current for the
+  // next time the state is created, and push the new resolver into the existing
+  // state so later decoration rebuilds use it.
+  useEffect(() => {
+    previewByPathRef.current = previewByPath
+    window.setTimeout(() => {
+      viewRef.current?.dispatch(setFilePreview(previewByPath))
+    })
+  }, [previewByPath])
+
  useEffect(() => {
    return () => {
      viewRef.current?.destroy()
@@ -0,0 +1,30 @@
+import {
+  Compartment,
+  Extension,
+  Facet,
+  TransactionSpec,
+} from '@codemirror/state'
+import { PreviewPath } from '../../../../../types/preview-path'
+
+export type PreviewByPath = (path: string) => PreviewPath | null
+
+const previewByPathConf = new Compartment()
+
+/**
+ * Resolves a path in the project to a preview URL. The value comes from the
+ * file tree, so it changes whenever the file tree does. Read it from the state
+ * at the point of use rather than capturing it, so decorations and widgets
+ * don't hold a resolver for a stale file tree.
+ */
+export const previewByPathFacet = Facet.define<PreviewByPath, PreviewByPath>({
+  combine: values => values[0] ?? (() => null),
+})
+
+export const filePreview = (previewByPath: PreviewByPath): Extension =>
+  previewByPathConf.of(previewByPathFacet.of(previewByPath))
+
+export const setFilePreview = (
+  previewByPath: PreviewByPath
+): TransactionSpec => ({
+  effects: previewByPathConf.reconfigure(previewByPathFacet.of(previewByPath)),
+})
@@ -48,6 +48,7 @@ import { toolbarPanel } from './toolbar/toolbar-panel'
 import { breadcrumbPanel } from './breadcrumbs-panel'
 import { geometryChangeEvent } from './geometry-change-event'
 import { docName } from './doc-name'
+import { filePreview } from './file-preview'
 import { fileTreeItemDrop } from './file-tree-item-drop'
 import { mathPreview } from './math-preview'
 import { ranges } from './ranges'
@@ -66,7 +67,7 @@ const moduleExtensions: Array<(options: Record<string, any>) => Extension> =

 export const createExtensions = (options: Record<string, any>): Extension[] => [
  lineNumbers(),
-  highlightSpecialChars(options.visual.visual),
+  highlightSpecialChars(options.showVisual),
  // The built-in extension that manages the history stack,
  // configured to increase the maximum delay between adjacent grouped edits
  history({ newGroupDelay: 250 }),
@@ -85,13 +86,13 @@ export const createExtensions = (options: Record<string, any>): Extension[] => [
  // A built-in extension that enables soft line wrapping.
  EditorView.lineWrapping,
  sourceOnly(
-    options.visual.visual,
+    options.showVisual,
    EditorView.contentAttributes.of({ 'aria-label': 'Source Editor editing' })
  ),
  // A built-in extension that re-indents input if the language defines an indentOnInput field in its language data.
  indentOnInput(),
-  lineWrappingIndentation(options.visual.visual),
-  indentationMarkers(options.visual.visual),
+  lineWrappingIndentation(options.showVisual),
+  indentationMarkers(options.showVisual),
  bracketMatching(),
  bracketSelection(),
  // A built-in extension that enables rectangular selections, created by dragging a new selection while holding down Alt.
@@ -133,6 +134,7 @@ export const createExtensions = (options: Record<string, any>): Extension[] => [
  keybindings(),

  docName(options.docName),
+  filePreview(options.previewByPath),

  // NOTE: `annotations` needs to be before `language`
  annotations(),
@@ -141,7 +143,7 @@ export const createExtensions = (options: Record<string, any>): Extension[] => [
  theme(options.theme),
  realtime(options.currentDoc, options.handleError),
  cursorPosition(options.currentDoc),
-  scrollPosition(options.currentDoc, options.visual),
+  scrollPosition(options.currentDoc, options.showVisual),
  cursorHighlights(),
  autoPair(options.settings),
  editable(),
@@ -157,17 +159,17 @@ export const createExtensions = (options: Record<string, any>): Extension[] => [
    ? historyOT(options.currentDoc.currentDocument)
    : ranges(),
  trackDetachedComments(options.currentDoc),
-  visual(options.docName, options.visual),
+  visual(options.docName, options.showVisual),
  mathPreview(options.settings.mathPreview),
  reviewTooltip(options.settings.floatingMenu),
  contextMenu(),
  toolbarPanel(),
  breadcrumbPanel(),
  verticalOverflow(),
-  highlightActiveLine(options.visual.visual),
+  highlightActiveLine(options.showVisual),
  // The built-in extension that highlights the active line in the gutter.
  highlightActiveLineGutter(),
-  inlineBackground(options.visual.visual),
+  inlineBackground(options.showVisual),
  codemirrorDevTools(),
  // Send exceptions to Sentry
  EditorView.exceptionSink.of(options.handleException),
@@ -29,7 +29,7 @@ export const scrollPosition = (
  }: {
    currentDoc: { doc_id: string }
  },
-  { visual }: { visual: boolean }
+  visual: boolean
 ) => {
  // store lineInfo for use on unload, when the DOM has already been unmounted
  let lineInfo: LineInfo
@@ -3,8 +3,8 @@ import { placeSelectionInsideBlock } from '../selection'
 import { isEqual } from 'lodash'
 import { FigureData } from '../../figure-modal'
 import { debugConsole } from '@/utils/debugging'
-import { PreviewPath } from '../../../../../../../types/preview-path'
 import type { PDFDocumentProxy } from 'pdfjs-dist/types/src/display/api'
+import { previewByPathFacet } from '../../file-preview'
 import { PdfDestroyLock } from '../utils/pdf-destroy-lock'

 // Module level to synchronize across all GraphicsWidgets
@@ -25,7 +25,6 @@ export class GraphicsWidget extends WidgetType {

  constructor(
    public filePath: string,
-    public previewByPath: (path: string) => PreviewPath | null,
    public centered: boolean,
    public figureData: FigureData | null
  ) {
@@ -107,7 +106,7 @@ export class GraphicsWidget extends WidgetType {
  renderGraphic(element: HTMLElement, view: EditorView) {
    element.textContent = '' // ensure the element is empty

-    const preview = this.previewByPath(this.filePath)
+    const preview = view.state.facet(previewByPathFacet)(this.filePath)
    element.dataset.filepath = this.filePath
    element.dataset.width = this.figureData?.width?.toString()

@@ -21,14 +21,8 @@ import { pasteHtml } from './paste-html'
 import { commandTooltip } from '../command-tooltip'
 import { tableGeneratorTheme } from './table-generator'
 import { debugConsole } from '@/utils/debugging'
-import { PreviewPath } from '../../../../../../types/preview-path'
 import { getFileExtension } from '../../utils/file'

-type Options = {
-  visual: boolean
-  previewByPath: (path: string) => PreviewPath | null
-}
-
 // Module-provided visual editors registered via the
 // `sourceEditorVisualExtensions` hook. Module exposes `getExtensions(ext)`,
 // returning the visual-mode extensions for that file extension. A module match takes precedence
@@ -55,17 +49,16 @@ const visualState = StateField.define<boolean>({
  },
 })

-const configureVisualExtensions = (options: Options) =>
-  options.visual ? sharedVisualExtensions() : []
+const configureVisualExtensions = (showVisual: boolean) =>
+  showVisual ? sharedVisualExtensions() : []

-export const visual = (docName: string, options: Options): Extension => {
-  const extensions =
-    visualModuleExtensions(docName) ?? latexVisualExtensions(options)
+export const visual = (docName: string, showVisual: boolean): Extension => {
+  const extensions = visualModuleExtensions(docName) ?? latexVisualExtensions()

  return [
-    visualState.init(() => options.visual),
-    visualConf.of(configureVisualExtensions(options)),
-    visualOnly(options.visual, extensions),
+    visualState.init(() => showVisual),
+    visualConf.of(configureVisualExtensions(showVisual)),
+    visualOnly(showVisual, extensions),
  ]
 }

@@ -73,11 +66,11 @@ export const isVisual = (view: EditorView) => {
  return view.state.field(visualState, false) || false
 }

-export const setVisual = (options: Options): TransactionSpec => {
+export const setVisual = (showVisual: boolean): TransactionSpec => {
  return {
    effects: [
-      toggleVisualEffect.of(options.visual),
-      visualConf.reconfigure(configureVisualExtensions(options)),
+      toggleVisualEffect.of(showVisual),
+      visualConf.reconfigure(configureVisualExtensions(showVisual)),
    ],
  }
 }
@@ -202,9 +195,9 @@ const sharedVisualExtensions = () => [
  EditorView.contentAttributes.of({ 'aria-label': 'Visual Editor editing' }),
 ]

-const latexVisualExtensions = (options: Options): Extension => [
+const latexVisualExtensions = (): Extension => [
  listItemMarker,
-  atomicDecorations(options),
+  atomicDecorations,
  markDecorations, // NOTE: must be after atomicDecorations, so that mark decorations wrap inline widgets
  visualKeymap,
  commandTooltip,
@@ -31,7 +31,8 @@ import { setKeybindings } from '../extensions/keybindings'
 import { Highlight } from '../../../../../types/highlight'
 import { EditorView } from '@codemirror/view'
 import { useErrorBoundary } from 'react-error-boundary'
-import { setVisual } from '../extensions/visual/visual'
+import { isVisual, setVisual } from '../extensions/visual/visual'
+import { setFilePreview } from '../extensions/file-preview'
 import { useFileTreePathContext } from '@/features/file-tree/contexts/file-tree-path'
 import { useUserSettingsContext } from '@/shared/context/user-settings-context'
 import { setDocName } from '@/features/source-editor/extensions/doc-name'
@@ -291,10 +292,9 @@ function useCodeMirrorScope(view: EditorView) {
  const showVisual =
    visual && !!openDocName && isCmVisualEditorAvailable(openDocName)

-  const visualRef = useRef({
-    previewByPath,
-    visual: showVisual,
-  })
+  const showVisualRef = useRef(showVisual)
+
+  const previewByPathRef = useRef(previewByPath)

  // Persist the search query in this hook when the document changes by keeping
  // a reference to the search query in sync with the editor state
@@ -308,21 +308,24 @@ function useCodeMirrorScope(view: EditorView) {

  const { showBoundary } = useErrorBoundary()

-  const handleException = useCallback((exception: any) => {
-    captureException(exception, {
-      tags: {
-        handler: 'cm6-exception',
-        // which editor mode is active ('visual' | 'code')
-        ol_editor_mode: visualRef.current.visual ? 'visual' : 'code',
-        // which editor keybindings are active ('default' | 'vim' | 'emacs')
-        ol_editor_keybindings: settingsRef.current.mode,
-        // whether Writefull is present ('extension' | 'integration' | 'none')
-        ol_extensions_writefull: window.writefull ? 'integration' : 'none',
-        // whether Grammarly is present
-        ol_extensions_grammarly: grammarlyExtensionPresent(),
-      },
-    })
-  }, [])
+  const handleException = useCallback(
+    (exception: any) => {
+      captureException(exception, {
+        tags: {
+          handler: 'cm6-exception',
+          // which editor mode is active ('visual' | 'code')
+          ol_editor_mode: isVisual(view) ? 'visual' : 'code',
+          // which editor keybindings are active ('default' | 'vim' | 'emacs')
+          ol_editor_keybindings: settingsRef.current.mode,
+          // whether Writefull is present ('extension' | 'integration' | 'none')
+          ol_extensions_writefull: window.writefull ? 'integration' : 'none',
+          // whether Grammarly is present
+          ol_extensions_grammarly: grammarlyExtensionPresent(),
+        },
+      })
+    },
+    [view]
+  )

  // create a new state when currentDocument changes

@@ -350,7 +353,8 @@ function useCodeMirrorScope(view: EditorView) {
          settings: settingsRef.current,
          phrases: phrasesRef.current,
          spelling: spellingRef.current,
-          visual: visualRef.current,
+          showVisual: showVisualRef.current,
+          previewByPath: previewByPathRef.current,
          projectFeatures: projectFeaturesRef.current,
          initialSearchQuery: searchQueryRef.current,
          showBoundary,
@@ -376,7 +380,7 @@ function useCodeMirrorScope(view: EditorView) {
        view.dispatch(spec)
      })

-      if (!visualRef.current.visual) {
+      if (!showVisualRef.current) {
        window.setTimeout(() => {
          view.dispatch(restoreScrollPosition())
          view.focus()
@@ -405,9 +409,9 @@ function useCodeMirrorScope(view: EditorView) {
  }, [view, openDocName])

  useEffect(() => {
-    visualRef.current.visual = showVisual
+    showVisualRef.current = showVisual
    window.setTimeout(() => {
-      view.dispatch(setVisual(visualRef.current))
+      view.dispatch(setVisual(showVisual))
      view.dispatch({
        effects: EditorView.scrollIntoView(view.state.selection.main.head),
      })
@@ -417,9 +421,9 @@ function useCodeMirrorScope(view: EditorView) {
  }, [view, showVisual])

  useEffect(() => {
-    visualRef.current.previewByPath = previewByPath
+    previewByPathRef.current = previewByPath
    window.setTimeout(() => {
-      view.dispatch(setVisual(visualRef.current))
+      view.dispatch(setFilePreview(previewByPath))
    })
  }, [view, previewByPath])

@@ -61,7 +61,7 @@ describe('CodeMirror scroll position extension', function () {
    const view = new EditorView({
      state: EditorState.create({
        doc,
-        extensions: [scrollPosition({ currentDoc }, { visual: false })],
+        extensions: [scrollPosition({ currentDoc }, false)],
      }),
    })

@@ -97,7 +97,7 @@ describe('CodeMirror scroll position extension', function () {
    const view = new EditorView({
      state: EditorState.create({
        doc,
-        extensions: [scrollPosition({ currentDoc }, { visual: false })],
+        extensions: [scrollPosition({ currentDoc }, false)],
      }),
    })
    view.dispatch(restoreScrollPosition())