Sfoglia il codice sorgente

fix(editor): sync PM views on undo/redo, fix zoneRefs leak, and handle multi-line zone paste

- Sync ProseMirror views immediately in applyOperation for content-changing
  operations (split, merge, set-block-content), closing the 50ms debounce
  window; restore cursor via registry.focus after undo/redo
- Replace updatingDepth counter with internalUpdatePending boolean + nextTick
  to prevent desync when Vue batches multiple internal writes into one watcher
  invocation
- Add else branch in zoneRefs ref callbacks to delete stale detached entries on
  block removal
- Defensively align result.blocks[prevIndex].content before assigning to
  ensure mergePrevious timing correctness
- Split multi-line zone paste into individual blocks with a single history
  entry for atomic undo
- Add void prefix to discarded history.execute() return values across all
  six call sites
- Document GapCursor.valid cast with biome-ignore and upstream type gap note
- Fix merge-block undo cursor to PrevContent.length + 1 (PM join point)
- Remove focusInfo from indent/outdent cases to avoid jarring cursor snaps on
  depth-only operations
- Clarify internalUpdatePending comment with double-reset edge case note
Zander Hawke 4 giorni fa
parent
commit
730cabe2ae

+ 8 - 4
packages/editor/src/components/Block.vue

@@ -34,8 +34,8 @@ import {
   createPairRule,
   dollarSignRule,
   doubleAsteriskRule,
-  doubleTildeRule,
   doubleCaretRule,
+  doubleTildeRule,
   singleUnderscoreRule,
   tripleBacktickRule,
 } from "@/lib/auto-close-plugin"
@@ -98,8 +98,10 @@ const emit = defineEmits<{
 const log = createLogger("Block", props.debug)
 
 function isValidGapCursor($pos: unknown): boolean {
-  // GapCursor.valid($pos) exists at runtime but is missing from published
-  // type defs. Use (GapCursor as any) as a workaround.
+  // GapCursor.valid($pos) exists at runtime ([email protected])
+  // but is absent from the published type definitions. The cast is
+  // unavoidable without a PR upstream.
+  // biome-ignore lint/suspicious/noExplicitAny: static method missing from types
   return (GapCursor as any).valid($pos)
 }
 
@@ -426,7 +428,9 @@ onMounted(() => {
     handleKeyDown,
     handleTextInput(_view, _from, _to, text) {
       if (text.indexOf("\u00a0") !== -1) {
-        _view.dispatch(_view.state.tr.insertText(text.replace(/\u00a0/g, " "), _from, _to))
+        _view.dispatch(
+          _view.state.tr.insertText(text.replace(/\u00a0/g, " "), _from, _to),
+        )
         return true
       }
       return false

+ 154 - 54
packages/editor/src/components/Editor.vue

@@ -1,6 +1,6 @@
 <script setup lang="ts">
 import type { EditorView } from "prosemirror-view"
-import { computed, nextTick, ref, watch, type Ref } from "vue"
+import { computed, nextTick, type Ref, ref, watch } from "vue"
 import Block from "@/components/Block.vue"
 import EditorSuggestionMenu from "@/components/EditorSuggestionMenu.vue"
 import InsertionZone from "@/components/InsertionZone.vue"
@@ -8,7 +8,21 @@ import {
   type FocusTarget,
   useFocusRegistry,
 } from "@/composables/useFocusRegistry"
+import {
+  defaultKeyBinding,
+  type KeyBinding,
+  useKeyboard,
+} from "@/composables/useKeyboard"
 import type { MarkerVisibilityMode } from "@/composables/useMarkdownDecorations"
+import type {
+  PatternClosePayload,
+  PatternOpenPayload,
+  PatternUpdatePayload,
+} from "@/composables/usePatternPlugin"
+import {
+  useMentionGroups,
+  useSlashGroups,
+} from "@/composables/useSuggestionGroups"
 import {
   type EditorBlock,
   generateBlockId,
@@ -25,20 +39,6 @@ import {
 } from "@/lib/editor-operations"
 import type { FormattingHandlers } from "@/lib/formatting"
 import { createLogger } from "@/lib/logger"
-import type {
-  PatternClosePayload,
-  PatternOpenPayload,
-  PatternUpdatePayload,
-} from "@/composables/usePatternPlugin"
-import {
-  useSlashGroups,
-  useMentionGroups,
-} from "@/composables/useSuggestionGroups"
-import {
-  defaultKeyBinding,
-  useKeyboard,
-  type KeyBinding,
-} from "@/composables/useKeyboard"
 import {
   createOperationHistory,
   type EditorOperation,
@@ -72,11 +72,18 @@ const themeCSSVars = computed(() => {
   return themeToCSSVars(resolved)
 })
 
-// Counter tracking nested internal updates, used to prevent the external
-// content watcher from re-parsing when we already pushed the change through.
-// Using a counter instead of a boolean avoids race conditions when batched
-// watcher callbacks fire sequentially (e.g. rapid undo/redo).
-let updatingDepth = 0
+// Tracks whether an internal content update is in flight. When the watcher
+// fires for a content.value change that originated from onBlockContentChange
+// (not from an external source), it resets and skips re-parsing.
+// Using a boolean avoids the counter-desync problem: if Vue collapses N
+// writes to content.value into one watcher invocation, resetting to false
+// on every skip handles any number of batched internal updates.
+// Edge case: if two onBlockContentChange calls fire in separate microtask
+// turns (the first's nextTick clears the flag before the second's watcher
+// fires), the boolean would prematurely reset. This doesn't happen in the
+// current call graph (all internal changes are synchronous), but is noted
+// for future refactoring.
+let internalUpdatePending = false
 
 const blocks = ref<EditorBlock[]>([])
 
@@ -99,39 +106,53 @@ const history: OperationHistory = createOperationHistory(100)
 const canUndo = ref(false)
 const canRedo = ref(false)
 
+// Applies an operation's model mutation and syncs PM views. Does NOT
+// update canUndo/canRedo — that's the caller's responsibility (undo/redo
+// update the flags afterward; direct operation callsites do so inline).
 function applyOperation(op: EditorOperation): void {
+  let focusInfo: { id: string; pos: "start" | "end" | number } | null = null
+
   switch (op.type) {
     case "split-block": {
       const block = blocks.value[op.index]
       if (!block) return
       block.content = op.beforeContent
+      blockRefs.get(block.id)?.setContent(op.beforeContent)
       blocks.value.splice(op.index + 1, 0, {
         id: op.splitBlockId,
         depth: op.depth,
         content: op.afterContent,
       })
+      focusInfo = { id: op.splitBlockId, pos: "start" }
       break
     }
     case "merge-block": {
-      // Remove block at op.index and append its content to the previous block.
       const prev = blocks.value[op.index - 1]
       if (prev) {
-        prev.content = op.prevContent
-        prev.content += op.appendedContent
+        prev.content = op.prevContent + op.appendedContent
+        blockRefs.get(prev.id)?.setContent(prev.content)
       }
       blocks.value.splice(op.index, 1)
+      // Cursor lands at the join point: PM's 1-indexed position after
+      // the last character of the pre-merge content.
+      if (prev) focusInfo = { id: prev.id, pos: op.prevContent.length + 1 }
       break
     }
-    case "insert-block":
+    case "insert-block": {
       blocks.value.splice(op.index, 0, {
         id: op.blockId,
         depth: op.depth,
         content: op.content,
       })
+      focusInfo = { id: op.blockId, pos: "start" }
       break
-    case "delete-block":
+    }
+    case "delete-block": {
       blocks.value.splice(op.index, 1)
+      const target = blocks.value[op.index] ?? blocks.value[op.index - 1]
+      if (target) focusInfo = { id: target.id, pos: "start" }
       break
+    }
     case "indent-block": {
       const indentBlock = blocks.value[op.index]
       if (indentBlock) indentBlock.depth = Math.min(op.previousDepth + 1, 10)
@@ -141,6 +162,8 @@ function applyOperation(op: EditorOperation): void {
           if (child) child.depth = Math.min(c.previousDepth + 1, 10)
         }
       }
+      // No focusInfo here — indent only changes depth metadata, not PM
+      // content, so moving the cursor would be jarring.
       break
     }
     case "outdent-block": {
@@ -152,15 +175,24 @@ function applyOperation(op: EditorOperation): void {
           if (child) child.depth = Math.max(c.previousDepth - 1, 0)
         }
       }
+      // No focusInfo — same rationale as indent-block.
       break
     }
     case "set-block-content": {
       const setBlock = blocks.value.find((b) => b.id === op.blockId)
-      if (setBlock) setBlock.content = op.newContent
+      if (setBlock) {
+        setBlock.content = op.newContent
+        blockRefs.get(setBlock.id)?.setContent(op.newContent)
+      }
+      if (setBlock) focusInfo = { id: setBlock.id, pos: "end" }
       break
     }
   }
   onBlockContentChange()
+
+  if (focusInfo) {
+    nextTick(() => registry.focus(focusInfo!.id, focusInfo!.pos))
+  }
 }
 
 function undo() {
@@ -189,8 +221,8 @@ function parseContent(md: string | undefined) {
 watch(
   () => content.value,
   (val) => {
-    if (updatingDepth > 0) {
-      updatingDepth--
+    if (internalUpdatePending) {
+      internalUpdatePending = false
       return
     }
     history.clear()
@@ -203,8 +235,11 @@ watch(
 
 // Serialize and emit when any block's content changes.
 function onBlockContentChange() {
-  updatingDepth++
+  internalUpdatePending = true
   content.value = serializeBlocks(blocks.value)
+  nextTick(() => {
+    internalUpdatePending = false
+  })
 }
 
 // ── Active block state (toolbar binding) ──────────────────────────────
@@ -296,12 +331,18 @@ function onZoneNavigateDown(zoneIndex: number) {
 // ── Block event handlers ──────────────────────────────────────────────
 
 function onSplit(index: number, before: string, after: string) {
-  const result = opSplitBlocks(blocks.value, index, before, after, generateBlockId)
+  const result = opSplitBlocks(
+    blocks.value,
+    index,
+    before,
+    after,
+    generateBlockId,
+  )
   if (!result.newBlock) return
   blocks.value = result.blocks
   onBlockContentChange()
 
-  history.execute({
+  void history.execute({
     type: "split-block",
     index,
     splitBlockId: result.newBlock.id,
@@ -331,10 +372,17 @@ function onMergePrevious(index: number) {
   // no-op — preventing a cursor-resetting doc replacement.
   blockRefs.get(prev.id)?.setContent(result.mergedContent)
 
+  // Defensively ensure the result block's content matches mergedContent,
+  // even if mergePrevious's implementation evolves. Without this, the new
+  // block objects in result.blocks might have stale content, causing the
+  // debounced watch in Block.vue to re-apply the old value and lose state.
+  const prevIndex = result.blocks.findIndex((b) => b.id === prev.id)
+  if (prevIndex !== -1) result.blocks[prevIndex].content = result.mergedContent
+
   blocks.value = result.blocks
   onBlockContentChange()
 
-  history.execute({
+  void history.execute({
     type: "merge-block",
     index,
     removedBlockId,
@@ -358,7 +406,7 @@ function onDeleteIfEmpty(index: number) {
   blocks.value = result.blocks
   onBlockContentChange()
 
-  history.execute({
+  void history.execute({
     type: "delete-block",
     index,
     blockId: removedBlock.id,
@@ -415,7 +463,7 @@ function onIndent(index: number) {
   blocks.value = result.blocks
   onBlockContentChange()
 
-  history.execute({
+  void history.execute({
     type: "indent-block",
     index,
     previousDepth,
@@ -441,7 +489,7 @@ function onOutdent(index: number) {
   blocks.value = result.blocks
   onBlockContentChange()
 
-  history.execute({
+  void history.execute({
     type: "outdent-block",
     index,
     previousDepth,
@@ -471,25 +519,77 @@ function onZoneBlur() {
 }
 
 function onZoneActivate(index: number, content: string) {
-  const result = opInsertBlock(blocks.value, index, content, 0, generateBlockId)
-  blocks.value = result.blocks
   focusedTarget.value = null
-
-  history.execute({
-    type: "insert-block",
-    index,
-    blockId: result.newBlock.id,
-    content,
-    depth: 0,
-    timestamp: Date.now(),
-  })
-  canUndo.value = history.canUndo
-  canRedo.value = history.canRedo
-
-  if (content) {
-    nextTick(() => registry.focus(result.newBlock.id, "end"))
+  const lines = content.split("\n")
+
+  if (lines.length > 1) {
+    // Multi-line paste: create a block for each non-empty line.
+    // Only the first insert is recorded in history — undoing a paste should
+    // remove all inserted blocks at once, not step through them one by one.
+    // TODO: Replace with a batch operation type when the history model
+    // supports atomic multi-op undo.
+    let insertedId: string | undefined
+    let insertIndex = index
+    let firstInsertRecorded = false
+    for (let i = 0; i < lines.length; i++) {
+      const line = lines[i]
+      if (line === "" && i < lines.length - 1) continue
+      const result = opInsertBlock(
+        blocks.value,
+        insertIndex,
+        line,
+        0,
+        generateBlockId,
+      )
+      blocks.value = result.blocks
+      insertedId = result.newBlock.id
+      if (!firstInsertRecorded) {
+        void history.execute({
+          type: "insert-block",
+          index: insertIndex,
+          blockId: result.newBlock.id,
+          content: line,
+          depth: 0,
+          timestamp: Date.now(),
+        })
+        firstInsertRecorded = true
+      }
+      insertIndex++
+    }
+    onBlockContentChange()
+    canUndo.value = history.canUndo
+    canRedo.value = history.canRedo
+    const lastInsertedId = insertedId
+    if (lastInsertedId) {
+      nextTick(() => registry.focus(lastInsertedId, "end"))
+    }
   } else {
-    nextTick(() => registry.focus(result.newBlock.id, "start"))
+    const result = opInsertBlock(
+      blocks.value,
+      index,
+      content,
+      0,
+      generateBlockId,
+    )
+    blocks.value = result.blocks
+    onBlockContentChange()
+
+    void history.execute({
+      type: "insert-block",
+      index,
+      blockId: result.newBlock.id,
+      content,
+      depth: 0,
+      timestamp: Date.now(),
+    })
+    canUndo.value = history.canUndo
+    canRedo.value = history.canRedo
+
+    if (content) {
+      nextTick(() => registry.focus(result.newBlock.id, "end"))
+    } else {
+      nextTick(() => registry.focus(result.newBlock.id, "start"))
+    }
   }
 }
 
@@ -511,7 +611,7 @@ defineExpose({ undo, redo, canUndo, canRedo })
     </div>
     <template v-for="(block, i) in blocks" :key="block.id">
       <InsertionZone
-        :ref="(el: any) => { if (el) zoneRefs[i] = el }"
+        :ref="(el: any) => { if (el) zoneRefs[i] = el; else delete zoneRefs[i] }"
         @activate="(content) => onZoneActivate(i, content)"
         @focus="onZoneFocus(i)"
         @blur="onZoneBlur"
@@ -556,7 +656,7 @@ defineExpose({ undo, redo, canUndo, canRedo })
       </div>
     </template>
     <InsertionZone
-      :ref="(el: any) => { if (el) zoneRefs[blocks.length] = el }"
+      :ref="(el: any) => { if (el) zoneRefs[blocks.length] = el; else delete zoneRefs[blocks.length] }"
       @activate="(content) => onZoneActivate(blocks.length, content)"
       @focus="onZoneFocus(blocks.length)"
       @blur="onZoneBlur"

+ 1 - 1
packages/editor/src/lib/auto-close-plugin.ts

@@ -156,7 +156,7 @@ interface ToggleRuleOptions {
 
 export function createToggleRule(
   delimiter: string,
-  options?: ToggleRuleOptions,
+  _options?: ToggleRuleOptions,
 ): AutoCloseRule {
   if (delimiter.length < 2) {
     throw new Error(