From 430306193e1f7fa4be8ae07fac6d5ca093858bf8 Mon Sep 17 00:00:00 2001 From: Sergey Kozyrenko Date: Sat, 4 Jul 2026 12:07:30 +0700 Subject: [PATCH] refactor(markdown-editor): apply mechanical review batch (11 findings) - M5: replace hand-rolled unknown-typed markdown types with the official @tiptap/core exports (MarkdownToken/ParseHelpers/RendererHelpers) and drop both casts they forced on the fidelity-critical paths - M3: forward FormControl-injected id/aria-describedby/aria-invalid to the ProseMirror contenteditable (field<->label association + error announce) - M2: skip the redundant whole-document re-parse on mount (compare against the captured initialContent instead of the normalized getMarkdown()) - L1+L10: extract the toolbar + isSafeImageSrc to markdown-editor-toolbar.tsx and memo() it (parent re-renders per keystroke under RHF) - L2: delete dead autoFocus/hasToolbar props (zero consumers) - L3: drop dead match.index narrowing (required number under es2023 lib) - L4: own vite chunk for marked; drop stale tiptap-markdown chunk pattern - L8: TunedStarterKit / TunedTable (consistent Tuned* prefix) - L9: shouldExternalSync -> isExternalChange - L14: isSafeImageSrc data: allowlist narrowed to base64 raster formats (data:image/svg+xml can carry script) - L7: trim narration/restatement comments across module + tests Co-Authored-By: Claude Opus 4.8 --- .../markdown-editor-code-fence.test.ts | 4 +- .../markdown-editor-content-integrity.test.ts | 10 +- .../markdown-editor-extensions.test.ts | 20 +- .../markdown-editor-extensions.ts | 40 +- .../markdown-editor-image-src.test.ts | 9 +- .../markdown-editor-inline-scan.ts | 2 +- .../markdown-editor/markdown-editor-marked.ts | 12 +- .../markdown-editor-test-setup.ts | 1 - .../markdown-editor-toolbar.tsx | 302 +++++++++++++++ .../markdown-editor-variable-syntax.ts | 2 +- .../markdown-editor/markdown-editor.tsx | 363 +++--------------- .../features/knowledges/knowledge-form.tsx | 7 +- frontend/vite.config.ts | 7 +- 13 files changed, 400 insertions(+), 379 deletions(-) create mode 100644 frontend/src/components/shared/markdown-editor/markdown-editor-toolbar.tsx diff --git a/frontend/src/components/shared/markdown-editor/markdown-editor-code-fence.test.ts b/frontend/src/components/shared/markdown-editor/markdown-editor-code-fence.test.ts index 90cb4dd6..7018ab29 100644 --- a/frontend/src/components/shared/markdown-editor/markdown-editor-code-fence.test.ts +++ b/frontend/src/components/shared/markdown-editor/markdown-editor-code-fence.test.ts @@ -14,7 +14,7 @@ const nodeCount = (md: string, type: string): number => { return count; }; -describe('code-block fence lengthening — a block documenting a ``` fence stays one block (M1)', () => { +describe('code-block fence lengthening — a block documenting a ``` fence stays one block', () => { it('widens the outer fence past an inner ``` run so the block does not split on reload', () => { const src = '````\ninner\n```\nnested fence\n```\nafter\n````'; const out = roundTrip(src); @@ -33,7 +33,7 @@ describe('code-block fence lengthening — a block documenting a ``` fence stays // delimiter by rendering it around a fixed placeholder (getMarkOpening/getMarkClosing), so the `code` mark's // real content is never seen at delimiter time and it always emits a SINGLE backtick. A code span whose // content contains a backtick therefore collapses on save and never converges. A NODE (codeBlock, above) DOES -// receive its real content and is content-aware — hence M1 is fixable but this is not. Pinned so the gap stays +// receive its real content and is content-aware — hence the fence widening is fixable but this is not. Pinned so the gap stays // visible and this test flips green if a future @tiptap/markdown makes mark delimiters content-aware. describe('inline code containing a backtick — known lossy on save (upstream mark-delimiter limitation)', () => { it('collapses a backtick-containing code span (does not round-trip)', () => { diff --git a/frontend/src/components/shared/markdown-editor/markdown-editor-content-integrity.test.ts b/frontend/src/components/shared/markdown-editor/markdown-editor-content-integrity.test.ts index 532c1584..e4e448be 100644 --- a/frontend/src/components/shared/markdown-editor/markdown-editor-content-integrity.test.ts +++ b/frontend/src/components/shared/markdown-editor/markdown-editor-content-integrity.test.ts @@ -44,7 +44,7 @@ const WORDS = ['firewall', 'payload', 'exploit', 'nmap', 'recon', 'shell', 'toke // Block contexts that keep inline content literal: paragraph, heading, bullet item, ordered item, // blockquote, inline code. (Nested compositions — ordered>bullet>code, fence-in-fence — are exercised -// separately below; the historical ordered>bullet>code drop is fixed by the indented-fence + fence-widen work.) +// separately below.) const wrap = (context: number, text: string): string => { switch (context) { case 1: @@ -107,10 +107,10 @@ describe('generative content-integrity — atoms survive load↔serialize across expect(roundTrip(source)).toBe(source); }); - // M3: the survives/converges oracles above only place atoms in SINGLE contexts. Nesting primitives inside - // one another (ordered>bullet, blockquote>list, list>code, fence-in-fence) is exactly the class that - // produced the historical ordered>bullet>code drop — assert structure (via structuralCounts, catching a - // dropped block/item) AND convergence AND atom survival across depth-≥2 compositions. + // The survives/converges oracles above only place atoms in SINGLE contexts. Nesting primitives inside + // one another (ordered>bullet, blockquote>list, list>code, fence-in-fence) is exactly the class where a + // whole block can drop silently — assert structure (via structuralCounts, catching a dropped block/item) + // AND convergence AND atom survival across depth-≥2 compositions. it('nested primitive compositions preserve structure, content, and converge', () => { const rng = mulberry32(0x5eeded); const nestAtoms = ['{{.TargetURL}}', '', '__init__', 'C++ then C++', 'os.execute()']; diff --git a/frontend/src/components/shared/markdown-editor/markdown-editor-extensions.test.ts b/frontend/src/components/shared/markdown-editor/markdown-editor-extensions.test.ts index 72b5bc80..bc8afaef 100644 --- a/frontend/src/components/shared/markdown-editor/markdown-editor-extensions.test.ts +++ b/frontend/src/components/shared/markdown-editor/markdown-editor-extensions.test.ts @@ -118,12 +118,12 @@ describe('named HTML entities decode outside code; numeric refs, bare & and raw expect(roundTrip(src)).toBe(expected); }); - it.each([ - 'numeric { and ( and % stay', // numeric refs are not decoded - 'bare AT&T and a & b and 2>&1 survive', // a lone & is left literal - ])('keeps %s byte-identical', (s) => { - expect(roundTrip(s)).toBe(s); - }); + it.each(['numeric { and ( and % stay', 'bare AT&T and a & b and 2>&1 survive'])( + 'keeps %s byte-identical', + (s) => { + expect(roundTrip(s)).toBe(s); + }, + ); it('entities inside inline code are preserved (code content is not decoded)', () => { expect(roundTrip('`<script>`')).toBe('`<script>`'); @@ -174,7 +174,7 @@ describe('inline marks round-trip', () => { }); }); -describe('MarkdownTable — cell pipes escaped + alignment preserved, idempotent', () => { +describe('TunedTable — cell pipes escaped + alignment preserved, idempotent', () => { it.each([ [ 'pipe in a code-span cell', @@ -221,10 +221,8 @@ describe('nesting & sequencing — content preserved and converges (≤2 saves)' }); }); -// A code block nested in a list (ordered > bullet > code) has an indented opening fence, so -// @tiptap/extension-code-block's `startsWith('```')` gate used to drop it — the same root cause as a -// top-level indented fence, fixed by parseTunedCodeBlock (see markdown-editor-extensions.ts). This was -// once a pinned KNOWN-BUG test asserting the loss; the fix now keeps the code. +// A code block nested in a list (ordered > bullet > code) has an indented opening fence — the same root +// cause as a top-level indented fence, covered by parseTunedCodeBlock (see markdown-editor-extensions.ts). describe('ordered > bullet > code — indented nested code survives', () => { it('keeps the deeply-nested code block', () => { const out = roundTrip('1. lvl1\n - lvl2\n\n ```\n deepcode\n ```'); diff --git a/frontend/src/components/shared/markdown-editor/markdown-editor-extensions.ts b/frontend/src/components/shared/markdown-editor/markdown-editor-extensions.ts index 20dc8335..2a8f9aed 100644 --- a/frontend/src/components/shared/markdown-editor/markdown-editor-extensions.ts +++ b/frontend/src/components/shared/markdown-editor/markdown-editor-extensions.ts @@ -1,10 +1,18 @@ +import type { + JSONContent, + MarkdownParseHelpers, + MarkdownParseResult, + MarkdownRendererHelpers, + MarkdownToken, +} from '@tiptap/core'; + import { Image } from '@tiptap/extension-image'; import { TaskItem, TaskList } from '@tiptap/extension-list'; import { TableCell, TableHeader, TableRow } from '@tiptap/extension-table'; import { Placeholder } from '@tiptap/extensions'; import StarterKit from '@tiptap/starter-kit'; -import { createMarkdownLayer, MarkdownTable } from './markdown-editor-marked'; +import { createMarkdownLayer, TunedTable } from './markdown-editor-marked'; import { MarkdownPaste } from './markdown-editor-paste'; import { TagHighlight } from './markdown-editor-tag-highlight'; import { VariableHighlight } from './markdown-editor-variable-highlight'; @@ -12,24 +20,6 @@ import { VariableHighlight } from './markdown-editor-variable-highlight'; const dropUnderscore = (rules: { find: unknown }[]) => rules.filter((rule) => !(rule.find instanceof RegExp && rule.find.source.includes('_'))); -interface MarkdownCodeToken { - codeBlockStyle?: string; - lang?: string; - raw?: string; - text?: string; -} - -interface MarkdownParseHelpers { - createNode: (type: string, attrs: Record, content: unknown[]) => unknown; - createTextNode: (text: string) => unknown; -} - -interface MarkdownRenderHelpers { - renderChildren: (content: unknown) => string; -} - -type MarkdownRenderNode = { attrs?: { language?: null | string }; content?: unknown }; - const longestBacktickRun = (text: string): number => (text.match(/`+/g) ?? []).reduce((max, run) => Math.max(max, run.length), 0); @@ -37,7 +27,7 @@ const longestBacktickRun = (text: string): number => // contains a ``` line (a doc demonstrating fenced markdown — common in knowledge/prompt examples) re-parses as // TWO blocks on the next load: the inner fence closes the outer one. CommonMark requires the fence to be longer // than any backtick run inside — widen it. Otherwise identical to upstream. -const renderTunedCodeBlock = (node: MarkdownRenderNode, helpers: MarkdownRenderHelpers): string => { +const renderTunedCodeBlock = (node: JSONContent, helpers: MarkdownRendererHelpers): string => { const language = node.attrs?.language || ''; if (!node.content) { @@ -55,7 +45,7 @@ const renderTunedCodeBlock = (node: MarkdownRenderNode, helpers: MarkdownRenderH // token whose `raw` starts with that whitespace, the gate rejects it, and the block is dropped on load. When // a document mixes fences at different indents the mis-detection cascades and everything after the first // dropped fence vanishes too. Trim the leading indent before the gate; otherwise identical to upstream. -const parseTunedCodeBlock = (token: MarkdownCodeToken, helpers: MarkdownParseHelpers): unknown => { +const parseTunedCodeBlock = (token: MarkdownToken, helpers: MarkdownParseHelpers): MarkdownParseResult => { const fence = token.raw?.trimStart() ?? ''; if (!fence.startsWith('```') && !fence.startsWith('~~~') && token.codeBlockStyle !== 'indented') { @@ -74,7 +64,7 @@ const parseTunedCodeBlock = (token: MarkdownCodeToken, helpers: MarkdownParseHel // `__init__`/`_word_` would emphasize (→ `**init**`/`*word*`) while the same text loaded stays literal, // breaking identifiers. Drop only the underscore rules (their `find` regex mentions `_`; the `*` rules stay) // so typing matches load. codeBlock gets the indented-fence fix above. (Underline off below.) -const StarterKitTuned = StarterKit.extend({ +const TunedStarterKit = StarterKit.extend({ addExtensions() { return (this.parent?.() ?? []).map((extension) => { if (extension.name === 'bold' || extension.name === 'italic') { @@ -92,7 +82,7 @@ const StarterKitTuned = StarterKit.extend({ return extension.extend({ parseMarkdown: parseTunedCodeBlock, renderMarkdown: renderTunedCodeBlock, - } as Parameters[0]); + }); } return extension; @@ -109,12 +99,12 @@ const StarterKitTuned = StarterKit.extend({ // symmetric with the marked layer (which no longer neutralises autolink/url). Do NOT set false: it // diverges typing from load and re-freezes bare URLs as text. export const createMarkdownExtensions = (placeholder?: string) => [ - StarterKitTuned.configure({ + TunedStarterKit.configure({ codeBlock: { HTMLAttributes: { class: 'hljs' } }, link: { autolink: true, linkOnPaste: true }, underline: false, }), - MarkdownTable.configure({ resizable: true }), + TunedTable.configure({ resizable: true }), TableRow, TableHeader, TableCell, diff --git a/frontend/src/components/shared/markdown-editor/markdown-editor-image-src.test.ts b/frontend/src/components/shared/markdown-editor/markdown-editor-image-src.test.ts index 8e1fc1ce..bac79ca3 100644 --- a/frontend/src/components/shared/markdown-editor/markdown-editor-image-src.test.ts +++ b/frontend/src/components/shared/markdown-editor/markdown-editor-image-src.test.ts @@ -1,17 +1,18 @@ import { describe, expect, it } from 'vitest'; -import { isSafeImageSrc } from './markdown-editor'; +import { isSafeImageSrc } from './markdown-editor-toolbar'; // The Image extension stores whatever src it is handed; isSafeImageSrc is the allowlist that blocks dangerous // PROTOCOLS (javascript:, data:text/html, vbscript:, file:) before a src is saved. A bare relative string // resolves against the page origin (http/https) and is intentionally allowed — the guard validates the -// protocol, not that the URL is absolute. +// protocol, not that the URL is absolute. data: URLs are limited to base64 raster formats: svg+xml is the one +// image type that can carry script. describe('isSafeImageSrc — image-src protocol allowlist', () => { it.each([ 'http://example.com/a.png', 'https://example.com/a.png', 'data:image/png;base64,AAAA', - 'data:image/svg+xml;utf8,', + 'data:image/webp;base64,AAAA', ])('allows %s', (url) => { expect(isSafeImageSrc(url)).toBe(true); }); @@ -20,6 +21,8 @@ describe('isSafeImageSrc — image-src protocol allowlist', () => { 'javascript:alert(1)', 'data:text/html,', 'data:application/pdf;base64,AA', + 'data:image/svg+xml;utf8,', + 'data:image/svg+xml;base64,AAAA', 'vbscript:msgbox(1)', 'file:///etc/passwd', 'http://', // malformed → URL constructor throws → rejected via the catch diff --git a/frontend/src/components/shared/markdown-editor/markdown-editor-inline-scan.ts b/frontend/src/components/shared/markdown-editor/markdown-editor-inline-scan.ts index f7f71aaa..541744da 100644 --- a/frontend/src/components/shared/markdown-editor/markdown-editor-inline-scan.ts +++ b/frontend/src/components/shared/markdown-editor/markdown-editor-inline-scan.ts @@ -37,7 +37,7 @@ export const collectInlineMatches = (doc: PMNode, regex: RegExp): InlineMatch[] }); for (const match of text.matchAll(regex)) { - const start = match.index ?? 0; + const start = match.index; const from = positions[start]; const last = positions[start + match[0].length - 1]; diff --git a/frontend/src/components/shared/markdown-editor/markdown-editor-marked.ts b/frontend/src/components/shared/markdown-editor/markdown-editor-marked.ts index 458531eb..4eb8b9e7 100644 --- a/frontend/src/components/shared/markdown-editor/markdown-editor-marked.ts +++ b/frontend/src/components/shared/markdown-editor/markdown-editor-marked.ts @@ -1,4 +1,4 @@ -import type { JSONContent } from '@tiptap/core'; +import type { JSONContent, MarkdownRendererHelpers } from '@tiptap/core'; import { Extension } from '@tiptap/core'; import { renderTableToMarkdown, Table } from '@tiptap/extension-table'; @@ -71,16 +71,14 @@ const TunedMarkdownText = Extension.create({ // that one call to escape pipes fixes the save side, on the official alignment-aware renderer. // This does NOT (and cannot) help the LOAD side: a raw `|` inside inline code arriving in external markdown is // split by marked's GFM table tokenizer BEFORE the inline-code tokenizer runs — a marked limitation. -type RenderHelpers = { renderChildren: (nodes: JSONContent | JSONContent[], separator?: string) => string }; - -export const MarkdownTable = Table.extend({ - renderMarkdown(node: JSONContent, helpers: RenderHelpers) { - const pipeEscaping: RenderHelpers = { +export const TunedTable = Table.extend({ + renderMarkdown(node: JSONContent, helpers: MarkdownRendererHelpers) { + const pipeEscaping: MarkdownRendererHelpers = { ...helpers, renderChildren: (nodes, separator) => helpers.renderChildren(nodes, separator).replace(/\|/g, '\\|'), }; - return renderTableToMarkdown(node, pipeEscaping as Parameters[1]); + return renderTableToMarkdown(node, pipeEscaping); }, }); diff --git a/frontend/src/components/shared/markdown-editor/markdown-editor-test-setup.ts b/frontend/src/components/shared/markdown-editor/markdown-editor-test-setup.ts index 34e8e837..9deb2ed0 100644 --- a/frontend/src/components/shared/markdown-editor/markdown-editor-test-setup.ts +++ b/frontend/src/components/shared/markdown-editor/markdown-editor-test-setup.ts @@ -12,7 +12,6 @@ export const setupEditorJsdom = (): void => { ({ item: () => null, length: 0, [Symbol.iterator]: [][Symbol.iterator] }) as unknown as DOMRectList; }; -// The folder's standard load↔save round-trip: parse markdown into a headless editor and re-serialize. export const roundTrip = (markdown: string): string => { const editor = new Editor({ content: markdown, contentType: 'markdown', extensions: createMarkdownExtensions() }); const out = editor.getMarkdown(); diff --git a/frontend/src/components/shared/markdown-editor/markdown-editor-toolbar.tsx b/frontend/src/components/shared/markdown-editor/markdown-editor-toolbar.tsx new file mode 100644 index 00000000..47d2e613 --- /dev/null +++ b/frontend/src/components/shared/markdown-editor/markdown-editor-toolbar.tsx @@ -0,0 +1,302 @@ +import type { Editor } from '@tiptap/react'; + +import { useEditorState } from '@tiptap/react'; +import { + Bold, + Code, + Code2, + Heading1, + Heading2, + Heading3, + ImagePlus, + Italic, + Link as LinkIcon, + List, + ListOrdered, + ListTodo, + Minus, + Quote, + Redo, + Strikethrough, + Table, + Undo, +} from 'lucide-react'; +import { memo, useCallback } from 'react'; + +import { Separator } from '@/components/ui/separator'; +import { Toggle } from '@/components/ui/toggle'; +import { cn } from '@/lib/utils'; + +interface MarkdownEditorToolbarProps { + disabled?: boolean; + editor: Editor; +} + +// The Image extension doesn't validate the src protocol, so the toolbar Insert-image button rejects +// non-http(s)/non-raster-data URLs (javascript:, data:text/html, data:image/svg+xml — SVG can carry script). +// Guards ONLY that button — images entering via markdown load/paste bypass it (inert in an ; the +// read-only viewer sanitizes protocols on render). +export const isSafeImageSrc = (url: string): boolean => { + try { + const { protocol } = new URL(url, window.location.href); + + return ( + protocol === 'http:' || protocol === 'https:' || /^data:image\/(png|jpe?g|gif|webp|bmp);base64,/i.test(url) + ); + } catch { + return false; + } +}; + +// memo: every keystroke re-renders the RHF-controlled parent; without it all ~20 Toggle subtrees re-render +// per keystroke for referentially-stable props. +export const MarkdownEditorToolbar = memo(function MarkdownEditorToolbar({ + disabled, + editor, +}: MarkdownEditorToolbarProps) { + const handleSetLink = useCallback(() => { + const previousUrl = editor.getAttributes('link').href as string | undefined; + const url = window.prompt('URL', previousUrl ?? ''); + + if (url === null) { + return; + } + + if (url === '') { + editor.chain().focus().extendMarkRange('link').unsetLink().run(); + + return; + } + + editor.chain().focus().extendMarkRange('link').setLink({ href: url }).run(); + }, [editor]); + + // tiptap v3's useEditor does NOT re-render on every transaction, so reading editor.isActive/can() inline + // would leave the toolbar stale on selection-only moves (click into a bold word → Bold stays unlit). + // useEditorState re-runs this selector per transaction and re-renders only when a button's state flips. + const state = useEditorState({ + editor, + selector: ({ editor }) => ({ + canRedo: editor.can().redo(), + canUndo: editor.can().undo(), + isBlockquote: editor.isActive('blockquote'), + isBold: editor.isActive('bold'), + isBulletList: editor.isActive('bulletList'), + isCode: editor.isActive('code'), + isCodeBlock: editor.isActive('codeBlock'), + isH1: editor.isActive('heading', { level: 1 }), + isH2: editor.isActive('heading', { level: 2 }), + isH3: editor.isActive('heading', { level: 3 }), + isItalic: editor.isActive('italic'), + isLink: editor.isActive('link'), + isOrderedList: editor.isActive('orderedList'), + isStrike: editor.isActive('strike'), + isTable: editor.isActive('table'), + isTaskList: editor.isActive('taskList'), + }), + }); + + return ( +
+ editor.chain().focus().toggleBold().run()} + pressed={state.isBold} + size="sm" + title="Bold (Ctrl+B)" + > + + + editor.chain().focus().toggleItalic().run()} + pressed={state.isItalic} + size="sm" + title="Italic (Ctrl+I)" + > + + + editor.chain().focus().toggleStrike().run()} + pressed={state.isStrike} + size="sm" + title="Strikethrough" + > + + + editor.chain().focus().toggleCode().run()} + pressed={state.isCode} + size="sm" + title="Inline code" + > + + + + + + editor.chain().focus().toggleHeading({ level: 1 }).run()} + pressed={state.isH1} + size="sm" + title="Heading 1" + > + + + editor.chain().focus().toggleHeading({ level: 2 }).run()} + pressed={state.isH2} + size="sm" + title="Heading 2" + > + + + editor.chain().focus().toggleHeading({ level: 3 }).run()} + pressed={state.isH3} + size="sm" + title="Heading 3" + > + + + + + + editor.chain().focus().toggleBulletList().run()} + pressed={state.isBulletList} + size="sm" + title="Bullet list" + > + + + editor.chain().focus().toggleOrderedList().run()} + pressed={state.isOrderedList} + size="sm" + title="Ordered list" + > + + + editor.chain().focus().toggleTaskList().run()} + pressed={state.isTaskList} + size="sm" + title="Task list" + > + + + + + + editor.chain().focus().toggleBlockquote().run()} + pressed={state.isBlockquote} + size="sm" + title="Blockquote" + > + + + editor.chain().focus().toggleCodeBlock().run()} + pressed={state.isCodeBlock} + size="sm" + title="Code block" + > + + + + + + { + const url = window.prompt('Image URL'); + + if (url && isSafeImageSrc(url)) { + editor.chain().focus().setImage({ src: url }).run(); + } + }} + pressed={false} + size="sm" + title="Insert image" + > + + + editor.chain().focus().setHorizontalRule().run()} + pressed={false} + size="sm" + title="Horizontal rule" + > + + + + editor.chain().focus().insertTable({ cols: 3, rows: 3, withHeaderRow: true }).run() + } + pressed={state.isTable} + size="sm" + title="Insert table" + > + + + +
+ editor.chain().focus().undo().run()} + pressed={false} + size="sm" + title="Undo (Ctrl+Z)" + > + + + editor.chain().focus().redo().run()} + pressed={false} + size="sm" + title="Redo (Ctrl+Shift+Z)" + > + + +
+ + ); +}); diff --git a/frontend/src/components/shared/markdown-editor/markdown-editor-variable-syntax.ts b/frontend/src/components/shared/markdown-editor/markdown-editor-variable-syntax.ts index ce181d0e..d5236379 100644 --- a/frontend/src/components/shared/markdown-editor/markdown-editor-variable-syntax.ts +++ b/frontend/src/components/shared/markdown-editor/markdown-editor-variable-syntax.ts @@ -19,7 +19,7 @@ export const findVariableUseRanges = (value: string, variable: string): { index: const ranges: { index: number; length: number }[] = []; for (const match of value.matchAll(VARIABLE_RE)) { - if (match.index !== undefined && probe.test(match[0])) { + if (probe.test(match[0])) { ranges.push({ index: match.index, length: match[0].length }); } } diff --git a/frontend/src/components/shared/markdown-editor/markdown-editor.tsx b/frontend/src/components/shared/markdown-editor/markdown-editor.tsx index 114528fb..d84e6650 100644 --- a/frontend/src/components/shared/markdown-editor/markdown-editor.tsx +++ b/frontend/src/components/shared/markdown-editor/markdown-editor.tsx @@ -1,35 +1,15 @@ import type { Editor } from '@tiptap/react'; +import type { AriaAttributes, Ref } from 'react'; import { history } from '@tiptap/pm/history'; import { EditorState, TextSelection } from '@tiptap/pm/state'; -import { EditorContent, useEditor, useEditorState } from '@tiptap/react'; -import { - Bold, - Code, - Code2, - Heading1, - Heading2, - Heading3, - ImagePlus, - Italic, - Link as LinkIcon, - List, - ListOrdered, - ListTodo, - Minus, - Quote, - Redo, - Strikethrough, - Table, - Undo, -} from 'lucide-react'; -import { type Ref, useCallback, useEffect, useImperativeHandle, useMemo, useRef, useState } from 'react'; +import { EditorContent, useEditor } from '@tiptap/react'; +import { useEffect, useImperativeHandle, useMemo, useRef, useState } from 'react'; -import { Separator } from '@/components/ui/separator'; -import { Toggle } from '@/components/ui/toggle'; import { cn } from '@/lib/utils'; import { createMarkdownExtensions } from './markdown-editor-extensions'; +import { MarkdownEditorToolbar } from './markdown-editor-toolbar'; import { findVariableOccurrences } from './markdown-editor-variable-highlight'; export interface MarkdownEditorHandle { @@ -38,11 +18,12 @@ export interface MarkdownEditorHandle { } interface MarkdownEditorProps { - autoFocus?: boolean; + 'aria-describedby'?: string; + 'aria-invalid'?: AriaAttributes['aria-invalid']; className?: string; contentClassName?: string; disabled?: boolean; - hasToolbar?: boolean; + id?: string; onBlur?: () => void; onChange: (value: string) => void; placeholder?: string; @@ -87,21 +68,16 @@ export const resetUndoHistory = (editor: Editor): void => { view.dispatch(view.state.tr.setMeta('addToHistory', false)); }; -interface MarkdownEditorToolbarProps { - disabled?: boolean; - editor: Editor; -} - -// Shared by the loading shell and the mounted editor so the two wrappers cannot drift. const WRAPPER_CLASS = 'border-input dark:bg-input/30 group/markdown-editor flex w-full flex-col overflow-hidden rounded-md border shadow-2xs outline-hidden transition-[color,box-shadow]'; function MarkdownEditor({ - autoFocus, + 'aria-describedby': ariaDescribedby, + 'aria-invalid': ariaInvalid, className, contentClassName, disabled, - hasToolbar = true, + id, onBlur, onChange, placeholder = 'Write something…', @@ -218,11 +194,15 @@ function MarkdownEditor({ return; } - const current = editor.getMarkdown(); - const shouldExternalSync = current !== value; + // On mount the construction-time parse already reflects `initialContent`, so compare against THAT, not + // getMarkdown(): serialization normalizes most real documents, so `getMarkdown() !== value` on mount + // would re-parse the identical doc — doubling mount cost for exactly the large documents where parse + // dominates. After mount, getMarkdown() is the right divergence check for external form.reset syncs. + const isExternalChange = hasResetInitialHistoryRef.current + ? editor.getMarkdown() !== value + : value !== initialContent; - if (shouldExternalSync) { - // emitUpdate:false — an external sync (form.reset) is not a user edit, so it must not fire onChange. + if (isExternalChange) { editor.commands.setContent(value, { contentType: 'markdown', emitUpdate: false }); } @@ -232,16 +212,15 @@ function MarkdownEditor({ // Clear the undo stack: // - on initial mount, to discard the construction-time - // transactions dispatched by extensions like `trailingNode` - // plus the initial `setContent` we just ran above; + // transactions dispatched by extensions like `trailingNode`; // - on every external `setContent` (e.g. parent calls // form.reset(serverDocument)), because that transaction is // not a user edit either. - if (!hasResetInitialHistoryRef.current || shouldExternalSync) { + if (!hasResetInitialHistoryRef.current || isExternalChange) { hasResetInitialHistoryRef.current = true; resetUndoHistory(editor); } - }, [editor, value]); + }, [editor, value, initialContent]); useEffect(() => { if (!editor || editor.isEditable === !disabled) { @@ -251,16 +230,29 @@ function MarkdownEditor({ editor.setEditable(!disabled); }, [editor, disabled]); + // FormControl (Radix Slot) injects these; forward them to the ProseMirror contenteditable. NOT via + // reactive editorProps: setOptions(editorProps) wholesale-replaces `attributes` (dropping tiptap's + // default role="textbox") and diffs by object identity. PM only removes attributes it set itself, so + // writing ours directly on view.dom survives its updates. useEffect(() => { - if (autoFocus && editor) { - editor.commands.focus('end'); + if (!editor) { + return; } - // `autoFocus` is intentionally omitted from the deps — it's a - // mount-time prop, equivalent to the native `` - // attribute. We don't want a parent flipping `autoFocus` later - // to steal focus back into the editor. - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [editor]); + + const { dom } = editor.view; + + for (const [name, attribute] of Object.entries({ + 'aria-describedby': ariaDescribedby, + 'aria-invalid': ariaInvalid, + id, + })) { + if (attribute === undefined) { + dom.removeAttribute(name); + } else { + dom.setAttribute(name, String(attribute)); + } + } + }, [editor, ariaDescribedby, ariaInvalid, id]); if (!editor) { return ( @@ -282,12 +274,10 @@ function MarkdownEditor({ )} data-slot="markdown-editor" > - {hasToolbar ? ( - - ) : null} + ; the read-only viewer sanitizes protocols on render). -export const isSafeImageSrc = (url: string): boolean => { - try { - const { protocol } = new URL(url, window.location.href); - - return protocol === 'http:' || protocol === 'https:' || /^data:image\//i.test(url); - } catch { - return false; - } -}; - -function MarkdownEditorToolbar({ disabled, editor }: MarkdownEditorToolbarProps) { - const handleSetLink = useCallback(() => { - const previousUrl = editor.getAttributes('link').href as string | undefined; - const url = window.prompt('URL', previousUrl ?? ''); - - if (url === null) { - return; - } - - if (url === '') { - editor.chain().focus().extendMarkRange('link').unsetLink().run(); - - return; - } - - editor.chain().focus().extendMarkRange('link').setLink({ href: url }).run(); - }, [editor]); - - // tiptap v3's useEditor does NOT re-render on every transaction, so reading editor.isActive/can() inline - // would leave the toolbar stale on selection-only moves (click into a bold word → Bold stays unlit). - // useEditorState re-runs this selector per transaction and re-renders only when a button's state flips. - const state = useEditorState({ - editor, - selector: ({ editor }) => ({ - canRedo: editor.can().redo(), - canUndo: editor.can().undo(), - isBlockquote: editor.isActive('blockquote'), - isBold: editor.isActive('bold'), - isBulletList: editor.isActive('bulletList'), - isCode: editor.isActive('code'), - isCodeBlock: editor.isActive('codeBlock'), - isH1: editor.isActive('heading', { level: 1 }), - isH2: editor.isActive('heading', { level: 2 }), - isH3: editor.isActive('heading', { level: 3 }), - isItalic: editor.isActive('italic'), - isLink: editor.isActive('link'), - isOrderedList: editor.isActive('orderedList'), - isStrike: editor.isActive('strike'), - isTable: editor.isActive('table'), - isTaskList: editor.isActive('taskList'), - }), - }); - - return ( -
- editor.chain().focus().toggleBold().run()} - pressed={state.isBold} - size="sm" - title="Bold (Ctrl+B)" - > - - - editor.chain().focus().toggleItalic().run()} - pressed={state.isItalic} - size="sm" - title="Italic (Ctrl+I)" - > - - - editor.chain().focus().toggleStrike().run()} - pressed={state.isStrike} - size="sm" - title="Strikethrough" - > - - - editor.chain().focus().toggleCode().run()} - pressed={state.isCode} - size="sm" - title="Inline code" - > - - - - - - editor.chain().focus().toggleHeading({ level: 1 }).run()} - pressed={state.isH1} - size="sm" - title="Heading 1" - > - - - editor.chain().focus().toggleHeading({ level: 2 }).run()} - pressed={state.isH2} - size="sm" - title="Heading 2" - > - - - editor.chain().focus().toggleHeading({ level: 3 }).run()} - pressed={state.isH3} - size="sm" - title="Heading 3" - > - - - - - - editor.chain().focus().toggleBulletList().run()} - pressed={state.isBulletList} - size="sm" - title="Bullet list" - > - - - editor.chain().focus().toggleOrderedList().run()} - pressed={state.isOrderedList} - size="sm" - title="Ordered list" - > - - - editor.chain().focus().toggleTaskList().run()} - pressed={state.isTaskList} - size="sm" - title="Task list" - > - - - - - - editor.chain().focus().toggleBlockquote().run()} - pressed={state.isBlockquote} - size="sm" - title="Blockquote" - > - - - editor.chain().focus().toggleCodeBlock().run()} - pressed={state.isCodeBlock} - size="sm" - title="Code block" - > - - - - - - { - const url = window.prompt('Image URL'); - - if (url && isSafeImageSrc(url)) { - editor.chain().focus().setImage({ src: url }).run(); - } - }} - pressed={false} - size="sm" - title="Insert image" - > - - - editor.chain().focus().setHorizontalRule().run()} - pressed={false} - size="sm" - title="Horizontal rule" - > - - - - editor.chain().focus().insertTable({ cols: 3, rows: 3, withHeaderRow: true }).run() - } - pressed={state.isTable} - size="sm" - title="Insert table" - > -
- - -
- editor.chain().focus().undo().run()} - pressed={false} - size="sm" - title="Undo (Ctrl+Z)" - > - - - editor.chain().focus().redo().run()} - pressed={false} - size="sm" - title="Redo (Ctrl+Shift+Z)" - > - - -
- - ); -} - export { MarkdownEditor }; diff --git a/frontend/src/features/knowledges/knowledge-form.tsx b/frontend/src/features/knowledges/knowledge-form.tsx index da7af2c3..73beba0f 100644 --- a/frontend/src/features/knowledges/knowledge-form.tsx +++ b/frontend/src/features/knowledges/knowledge-form.tsx @@ -201,11 +201,8 @@ export function KnowledgeForm({ initialValues, isNew, knowledge, onSubmit }: Kno const form = useForm({ defaultValues: initialValues, - // `onTouched` validates a field on its first blur and on every change - // afterwards. With `onChange` we'd run the entire Zod schema on every - // keystroke (including every emit from the multi-kilobyte `content` - // markdown editor) — same UX after the first interaction, no waste - // on initial mount or untouched fields. + // Not `onChange`: that would run the entire Zod schema on every keystroke, including every emit + // from the multi-kilobyte `content` markdown editor. mode: 'onTouched', resetOptions: { // When `values` changes (e.g. a GraphQL subscription pushes an diff --git a/frontend/vite.config.ts b/frontend/vite.config.ts index 6422c605..81142ae8 100644 --- a/frontend/vite.config.ts +++ b/frontend/vite.config.ts @@ -86,9 +86,14 @@ export default defineConfig(({ mode }) => { if (/[\\/](monaco-editor|@monaco-editor[\\/]react)[\\/]/.test(id)) { return 'monaco'; } - if (/[\\/](@tiptap|tiptap-markdown|prosemirror-[a-z-]+)[\\/]/.test(id)) { + if (/[\\/](@tiptap|prosemirror-[a-z-]+)[\\/]/.test(id)) { return 'tiptap'; } + // Without an own chunk, marked gets folded into the 527KB tiptap chunk, and report-pdf's + // static `import { marked }` would download all of it for a ~40KB library. + if (/[\\/]node_modules[\\/]marked[\\/]/.test(id)) { + return 'marked'; + } if (/[\\/](react|react-dom|react-router-dom)[\\/]/.test(id)) { return 'react-vendor'; }