From 1ef7a21de23535f56a34ce75230d64ba6a57f446 Mon Sep 17 00:00:00 2001 From: xarmian Date: Fri, 8 May 2026 12:51:02 -0400 Subject: [PATCH] fix(editor): add NodeView update() hook to MermaidCodeBlock (TASK-1249) (#447) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(editor): add NodeView update() hook to MermaidCodeBlock (TASK-1249) Mermaid diagrams previously froze on the SVG generated when the NodeView was first created. ProseMirror only recreates a NodeView on node identity change; in-place text edits don't trigger that, and the existing factory had no `update()` hook to re-queue a render — so editing the source via the hover-revealed `< >` toggle silently mutated the code while the diagram showed stale output. Resolves BUG-1246. Implementation matches the pattern verified in the TASK-1245 spike (iteration 3, dev sandbox at /dev/yjs-sandbox) but with precise ProseMirror Node typing instead of `any`: - update(updatedNode) returns false on type mismatch or when the language attr flips into/out of `mermaid` — different DOM shape, so ProseMirror must tear down + recreate the NodeView. - Returns true (in-place update accepted) when same-node + same-lang; re-queues queueMermaidRender only when textContent actually changed. - Empty source clears the diagram element and the mermaid-error class. - Toggle state survives because we don't recreate the wrapper. Becomes a hard blocker once Yjs collab lands (PLAN-1248) since remote ops will constantly mutate mermaid source text mid-view. Parent: PLAN-1248. * fix(editor): serialize mermaid clear + drop error class on success per Codex review (round 1) Two issues raised in PR #447 review: P2 — Pending queueMermaidRender() could overwrite a synchronous diagram clear with a stale SVG, racing against a freshly-emptied source. Route the clear through the same renderQueue (queueMermaidClear) so it executes strictly after any in-flight render for the same target. P3 — A valid re-render after an invalid mermaid edit kept the .mermaid-error class on the diagram element. Drop the class in queueMermaidRender's success path now that successful render means the source compiled. Both fixes preserve TASK-1249's NodeView update() contract; no other behavior changed. --- web/src/lib/components/editor/Editor.svelte | 48 +++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/web/src/lib/components/editor/Editor.svelte b/web/src/lib/components/editor/Editor.svelte index 7cd1c52f..1327fc66 100644 --- a/web/src/lib/components/editor/Editor.svelte +++ b/web/src/lib/components/editor/Editor.svelte @@ -3,6 +3,7 @@ import { page } from '$app/state'; import { Editor, mergeAttributes } from '@tiptap/core'; import { Plugin } from '@tiptap/pm/state'; + import type { Node as ProseMirrorNode } from '@tiptap/pm/model'; import StarterKit from '@tiptap/starter-kit'; import TaskList from '@tiptap/extension-task-list'; import TaskItem from '@tiptap/extension-task-item'; @@ -37,6 +38,9 @@ const id = `mmd-${Math.random().toString(36).slice(2, 10)}`; const { svg } = await m.default.render(id, source); target.innerHTML = svg; + // A successful render means the source is now valid — drop any + // error styling left over from a prior failed render. + target.classList.remove('mermaid-error'); } catch { target.textContent = '⚠ Invalid Mermaid syntax'; target.classList.add('mermaid-error'); @@ -44,6 +48,17 @@ }); } + // Chain a diagram-clear through the shared mermaid render queue so it + // executes AFTER any still-pending renders for the same target. Without + // this, an in-flight queueMermaidRender() could overwrite a synchronous + // clear with stale SVG. + function queueMermaidClear(target: HTMLElement) { + renderQueue = renderQueue.then(() => { + target.textContent = ''; + target.classList.remove('mermaid-error'); + }); + } + // Build a hover-to-reveal "Copy" button for a code block. // Reads the live code text from `codeEl` so it copies edits too. function buildCopyButton(codeEl: HTMLElement): HTMLButtonElement { @@ -248,9 +263,42 @@ queueMermaidRender(source, diagram); } + // Closure state for update(): track last-rendered source + the + // language at NodeView creation time. ProseMirror only recreates + // the NodeView on node identity change (replace/retype) — text + // edits inside the same node hit update(), so we re-queue a + // mermaid render whenever the source text changes. See BUG-1246. + let lastSource = source; + const initialLang = lang; + return { dom: wrapper, contentDOM: code, + // Re-render the diagram when the node's text content changes. + // Typed via NodeView['update'] from prosemirror-view: the param + // is a ProseMirror Node (not the DOM Node global). Returning + // `true` accepts the in-place update; `false` forces ProseMirror + // to tear down + recreate this NodeView, which we want when + // the language attribute flips into/out of `mermaid` because + // the DOM shape (wrapper + diagram + toggle) is mermaid-only. + update(updatedNode: ProseMirrorNode) { + if (updatedNode.type.name !== 'codeBlock') return false; + if (updatedNode.attrs.language !== initialLang) return false; + + const newSource = updatedNode.textContent?.trim() ?? ''; + if (newSource !== lastSource) { + lastSource = newSource; + if (newSource) { + queueMermaidRender(newSource, diagram); + } else { + // Empty source: clear any stale SVG / error state. + // Routed through the render queue so a pending + // queueMermaidRender can't overwrite us afterward. + queueMermaidClear(diagram); + } + } + return true; + }, // Critical: tell ProseMirror to ignore DOM mutations outside // the contentDOM (code element). Without this, inserting the // mermaid SVG triggers ProseMirror's MutationObserver → re-parse