From 4b647a9725cfa90e7ce2fae036a8e50ca324e11d Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Tue, 28 Apr 2026 20:11:07 -0300 Subject: [PATCH 1/3] feat(superdoc/ui): comments domain (subscribe + actions) (SD-2790) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `ui.comments` to `createSuperDocUI`: a single subscription + actions surface for custom comments sidebars built on top of SuperDoc. ```ts import { createSuperDocUI } from 'superdoc/ui'; const ui = createSuperDocUI({ superdoc }); // Snapshot ui.comments.subscribe(({ snapshot }) => { // snapshot.items: CommentInfo[] // snapshot.activeIds: string[] (mirrors selection.current().activeCommentIds) // snapshot.total: number }); // Actions — every mutation routes through editor.doc.* (Document API) ui.comments.createFromSelection({ text }); ui.comments.resolve(commentId); ui.comments.reopen(commentId); // routes to comments.patch({ status: 'active' }) ui.comments.delete(commentId); ui.comments.scrollTo(commentId); // ranges.scrollIntoView({ kind: 'entity', ... }) ``` Architecture: - Subscribe runs through the SD-2794 selector substrate (`shallowEqual`-deduped). Comments-list is cached at controller level and refreshed on `commentsUpdate` / `commentsLoaded` editor events ahead of `scheduleNotify` so `state.comments.items` is fresh when subscribers see the next snapshot. `activeIds` reads from `selection.current()` per `computeState()` call (cheap; one walk the resolver already does for `activeMarks`). - `activeIds` falls back to `[]` when `selection.current()` predates SD-2792 (no `activeCommentIds` field). When SD-2792 lands, the fallback becomes never-used; nothing in this PR breaks if SD-2792 is reverted. - All execution paths route through `editor.doc.comments.*` and `editor.doc.ranges.scrollIntoView`. `ui.comments` is a UI-facing adapter, NOT a parallel mutation contract — `ui.comments.resolve(id)` and `editor.doc.comments.patch({ id, status: 'resolved' })` produce the same document mutation by construction. - `ui.comments.reopen(id)` routes to `comments.patch({ status: 'active' })`. Today doc-API validation rejects 'active' until SD-2789 ships the lifecycle inverse — that surfaces an INVALID_INPUT receipt rather than a silent no-op, which is the correct visible behavior for a not-yet-shipped operation. Stacks on PR #2979 (SD-2794 skeleton). Unrelated to PR #2980 (SD-2796 toolbar) and PR #2981 (SD-2792 active ids); rebases when those merge. Verified: 26 unit tests (15 substrate + 11 comments). Full super-editor: 11913 / 13 skipped / 0 fail. `pnpm --filter superdoc build` clean — `dist/superdoc/src/ui.d.ts` re-exports the new `CommentsHandle` and `CommentsSlice` types. Out of scope (separate tickets): - Threaded reply UX - `ui.comments.reopen` body — depends on SD-2789 (today routes to a doc-API path that throws INVALID_INPUT) - Refactoring `dropin-assessment` SuperDocAdapter to consume `ui.comments` instead of `superdoc.on('commentsUpdate')` — separate PR once stack lands --- packages/super-editor/src/ui/comments.test.ts | 253 ++++++++++++++++++ .../src/ui/create-super-doc-ui.ts | 170 ++++++++++-- packages/super-editor/src/ui/index.ts | 2 + packages/super-editor/src/ui/types.ts | 101 ++++++- packages/superdoc/src/ui.d.ts | 2 + 5 files changed, 510 insertions(+), 18 deletions(-) create mode 100644 packages/super-editor/src/ui/comments.test.ts diff --git a/packages/super-editor/src/ui/comments.test.ts b/packages/super-editor/src/ui/comments.test.ts new file mode 100644 index 0000000000..324ed5e078 --- /dev/null +++ b/packages/super-editor/src/ui/comments.test.ts @@ -0,0 +1,253 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { createSuperDocUI } from './create-super-doc-ui.js'; +import type { SuperDocLike } from './types.js'; + +/** + * Stub builder for `ui.comments` tests. Models the parts of the + * editor's `doc.comments` / `doc.selection` / `doc.ranges` surface + * the controller's comments domain reads or routes through. + */ +function makeStubs( + initial: { + comments?: Array<{ id: string; commentId: string; text?: string; status?: 'open' | 'resolved' }>; + activeCommentIds?: string[]; + selectionTarget?: unknown; + } = {}, +) { + const editorListeners = new Map void>>(); + const superdocListeners = new Map void>>(); + + let commentsList = initial.comments ?? []; + const create = vi.fn((input: { target: unknown; text: string }) => ({ + success: true as const, + inserted: [{ kind: 'entity', entityType: 'comment', entityId: `c-${commentsList.length + 1}` }], + target: input.target, + text: input.text, + })); + const patch = vi.fn((_input: { commentId: string; status?: string; text?: string }) => ({ + success: true as const, + })); + const del = vi.fn((_input: { commentId: string }) => ({ success: true as const })); + const list = vi.fn(() => ({ + evaluatedRevision: 'r1', + total: commentsList.length, + items: commentsList.map((c) => ({ + id: c.id, + handle: { ref: `comment:${c.commentId}`, refStability: 'stable' as const, targetKind: 'comment' as const }, + address: { kind: 'entity' as const, entityType: 'comment' as const, entityId: c.commentId }, + commentId: c.commentId, + status: c.status ?? ('open' as const), + text: c.text, + })), + page: { limit: 50, offset: 0, returned: commentsList.length }, + })); + const scrollIntoView = vi.fn(async (_input: unknown) => ({ success: true as const })); + + const editor = { + on: vi.fn((event: string, handler: (...args: unknown[]) => void) => { + if (!editorListeners.has(event)) editorListeners.set(event, new Set()); + editorListeners.get(event)!.add(handler); + }), + off: vi.fn((event: string, handler: (...args: unknown[]) => void) => { + editorListeners.get(event)?.delete(handler); + }), + doc: { + selection: { + current: vi.fn(() => ({ + empty: initial.selectionTarget == null, + text: '', + target: initial.selectionTarget ?? null, + activeCommentIds: initial.activeCommentIds ?? [], + activeChangeIds: [], + })), + }, + comments: { create, patch, delete: del, list }, + ranges: { scrollIntoView }, + }, + }; + + const superdoc: SuperDocLike & { + fireEditor(event: string, ...args: unknown[]): void; + setComments(next: typeof commentsList): void; + } = { + activeEditor: editor as never, + config: { documentMode: 'editing' }, + on: vi.fn((event: string, handler: (...args: unknown[]) => void) => { + if (!superdocListeners.has(event)) superdocListeners.set(event, new Set()); + superdocListeners.get(event)!.add(handler); + }), + off: vi.fn((event: string, handler: (...args: unknown[]) => void) => { + superdocListeners.get(event)?.delete(handler); + }), + fireEditor(event, ...args) { + const handlers = editorListeners.get(event); + if (!handlers) return; + [...handlers].forEach((handler) => handler(...args)); + }, + setComments(next) { + commentsList = next; + }, + }; + + return { superdoc, editor, mocks: { create, patch, delete: del, list, scrollIntoView } }; +} + +const flushMicrotasks = () => Promise.resolve(); + +describe('ui.comments — snapshot', () => { + it('exposes the initial comments list synchronously', () => { + const { superdoc, mocks } = makeStubs({ + comments: [ + { id: 'c1', commentId: 'c1', text: 'first' }, + { id: 'c2', commentId: 'c2', text: 'second' }, + ], + }); + const ui = createSuperDocUI({ superdoc }); + + const snap = ui.comments.getSnapshot(); + expect(snap.total).toBe(2); + expect(snap.items.map((i) => i.commentId)).toEqual(['c1', 'c2']); + expect(snap.activeIds).toEqual([]); + + expect(mocks.list).toHaveBeenCalled(); + ui.destroy(); + }); + + it('subscribe fires once with the initial snapshot', () => { + const { superdoc } = makeStubs({ comments: [{ id: 'c1', commentId: 'c1' }] }); + const ui = createSuperDocUI({ superdoc }); + + const cb = vi.fn(); + const off = ui.comments.subscribe(cb); + + expect(cb).toHaveBeenCalledTimes(1); + const arg = cb.mock.calls[0][0] as { snapshot: { total: number } }; + expect(arg.snapshot.total).toBe(1); + + off(); + ui.destroy(); + }); + + it('refreshes the cache on commentsUpdate and re-fires subscribers', async () => { + const { superdoc } = makeStubs({ comments: [{ id: 'c1', commentId: 'c1' }] }); + const ui = createSuperDocUI({ superdoc }); + + const cb = vi.fn(); + ui.comments.subscribe(cb); + expect(cb).toHaveBeenCalledTimes(1); + + superdoc.setComments([ + { id: 'c1', commentId: 'c1' }, + { id: 'c2', commentId: 'c2', text: 'new' }, + ]); + superdoc.fireEditor('commentsUpdate'); + await flushMicrotasks(); + + expect(cb).toHaveBeenCalledTimes(2); + const latest = cb.mock.calls[1][0] as { snapshot: { total: number; items: Array<{ commentId: string }> } }; + expect(latest.snapshot.total).toBe(2); + expect(latest.snapshot.items.map((i) => i.commentId)).toEqual(['c1', 'c2']); + + ui.destroy(); + }); + + it('mirrors selection.current().activeCommentIds into snapshot.activeIds', () => { + const { superdoc } = makeStubs({ comments: [{ id: 'c1', commentId: 'c1' }], activeCommentIds: ['c1'] }); + const ui = createSuperDocUI({ superdoc }); + + const snap = ui.comments.getSnapshot(); + expect(snap.activeIds).toEqual(['c1']); + + ui.destroy(); + }); + + it('falls back to [] when selection.current() predates SD-2792 (no activeCommentIds field)', () => { + const { superdoc, editor } = makeStubs(); + // Override selection.current to return an SD-2668-shaped result + // (no activeCommentIds). The controller must not crash. + (editor.doc.selection.current as unknown as () => { empty: boolean; target: null }) = vi.fn(() => ({ + empty: true, + target: null, + })); + const ui = createSuperDocUI({ superdoc }); + + expect(ui.comments.getSnapshot().activeIds).toEqual([]); + + ui.destroy(); + }); +}); + +describe('ui.comments — actions route through editor.doc.*', () => { + it('createFromSelection forwards to comments.create with the selection target', () => { + const target = { kind: 'text' as const, segments: [{ blockId: 'p1', range: { start: 0, end: 5 } }] }; + const { superdoc, mocks } = makeStubs({ selectionTarget: target }); + const ui = createSuperDocUI({ superdoc }); + + const receipt = ui.comments.createFromSelection({ text: 'Looks good' }); + + expect(receipt.success).toBe(true); + expect(mocks.create).toHaveBeenCalledWith({ target, text: 'Looks good' }); + + ui.destroy(); + }); + + it('createFromSelection returns a NO_OP receipt when no selection target exists', () => { + const { superdoc, mocks } = makeStubs(); // no selectionTarget + const ui = createSuperDocUI({ superdoc }); + + const receipt = ui.comments.createFromSelection({ text: 'orphan' }); + + expect(receipt.success).toBe(false); + expect(mocks.create).not.toHaveBeenCalled(); + + ui.destroy(); + }); + + it('resolve forwards to comments.patch({ commentId, status: "resolved" })', () => { + const { superdoc, mocks } = makeStubs(); + const ui = createSuperDocUI({ superdoc }); + + ui.comments.resolve('c-42'); + + expect(mocks.patch).toHaveBeenCalledWith({ commentId: 'c-42', status: 'resolved' }); + ui.destroy(); + }); + + it('reopen forwards to comments.patch({ commentId, status: "active" })', () => { + // Architecturally correct even though doc-api validation rejects + // 'active' until SD-2789 lands. The route is what we're asserting. + const { superdoc, mocks } = makeStubs(); + const ui = createSuperDocUI({ superdoc }); + + ui.comments.reopen('c-42'); + + expect(mocks.patch).toHaveBeenCalledWith({ commentId: 'c-42', status: 'active' }); + ui.destroy(); + }); + + it('delete forwards to comments.delete({ commentId })', () => { + const { superdoc, mocks } = makeStubs(); + const ui = createSuperDocUI({ superdoc }); + + ui.comments.delete('c-42'); + + expect(mocks.delete).toHaveBeenCalledWith({ commentId: 'c-42' }); + ui.destroy(); + }); + + it('scrollTo forwards to ranges.scrollIntoView with an EntityAddress', async () => { + const { superdoc, mocks } = makeStubs(); + const ui = createSuperDocUI({ superdoc }); + + await ui.comments.scrollTo('c-42'); + + expect(mocks.scrollIntoView).toHaveBeenCalledTimes(1); + const arg = mocks.scrollIntoView.mock.calls[0][0] as { + target: { kind: string; entityType: string; entityId: string }; + }; + expect(arg.target).toEqual({ kind: 'entity', entityType: 'comment', entityId: 'c-42' }); + + ui.destroy(); + }); +}); diff --git a/packages/super-editor/src/ui/create-super-doc-ui.ts b/packages/super-editor/src/ui/create-super-doc-ui.ts index de8baec2c0..ce3a5683ce 100644 --- a/packages/super-editor/src/ui/create-super-doc-ui.ts +++ b/packages/super-editor/src/ui/create-super-doc-ui.ts @@ -7,10 +7,13 @@ import type { PublicToolbarItemId, ToolbarSnapshot, } from '../headless-toolbar/types.js'; +import type { CommentsListResult, Receipt, ScrollIntoViewOutput } from '@superdoc/document-api'; import { shallowEqual } from './equality.js'; import type { CommandHandle, CommandsHandle, + CommentsHandle, + CommentsSlice, EqualityFn, SelectorFn, SuperDocEditorLike, @@ -42,6 +45,15 @@ const EDITOR_EVENTS = [ 'trackedChangesUpdate', ] as const; +/** + * Editor events that should trigger a refresh of the cached + * `comments.list()` result before notifying subscribers. The base + * `EDITOR_EVENTS` list also fires `scheduleNotify` for these, but we + * need the cache invalidation to happen *first* so `computeState()` + * sees fresh items. + */ +const COMMENTS_REFRESH_EVENTS = ['commentsUpdate', 'commentsLoaded'] as const; + const SUPERDOC_EVENTS = ['editorCreate', 'document-mode-change', 'zoomChange'] as const; /** @@ -146,23 +158,11 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { }); }; - // Internal headless-toolbar instance. Feeds `state.toolbar` so - // `ui.toolbar.subscribe` and `ui.commands..observe` ride the - // same selector substrate as the rest of the controller. The Vue UI - // and any external `superdoc/headless-toolbar` consumer can keep - // using their existing entry points; this is the single source of - // truth at runtime. - // - // The structural cast is safe at runtime: the SuperDoc Vue instance - // satisfies HeadlessToolbarSuperdocHost (with `Editor` for - // activeEditor) at runtime; we accept the looser SuperDocLike at the - // public boundary so this controller can be unit-tested with stubs. // Internal headless-toolbar instance. Feeds `state.toolbar` so // `ui.toolbar.subscribe` and `ui.commands..observe` ride the // same selector substrate as the rest of the controller. Per-command - // state derivers in the registry are now wrapped to default to - // disabled on throw, so a partial editor never wedges snapshot - // construction. + // state derivers in the registry are wrapped to default to disabled + // on throw, so a partial editor never wedges snapshot construction. const toolbarController: HeadlessToolbarController = createHeadlessToolbar({ superdoc: superdoc as unknown as HeadlessToolbarSuperdocHost, // Pass the full registry so snapshot.commands is populated for @@ -184,6 +184,40 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { } }); + // Comments slice cache. `editor.doc.comments.list()` is O(N) and + // re-running it on every `computeState()` would tax the hot path — + // instead we cache the list result and refresh on `commentsUpdate` / + // `commentsLoaded` editor events. `selection.current().activeCommentIds` + // is read fresh in `computeState()` since it's already cheap (one + // selection walk). + const EMPTY_COMMENTS_LIST: CommentsListResult = { + evaluatedRevision: '', + total: 0, + items: [], + page: { limit: 0, offset: 0, returned: 0 }, + }; + let commentsListCache: CommentsListResult = EMPTY_COMMENTS_LIST; + const refreshCommentsListCache = () => { + const editor = resolveRoutedEditor(superdoc); + const list = editor?.doc?.comments?.list; + if (typeof list !== 'function') { + commentsListCache = EMPTY_COMMENTS_LIST; + return; + } + try { + const result = list.call(editor.doc!.comments, undefined) as CommentsListResult | undefined; + commentsListCache = result ?? EMPTY_COMMENTS_LIST; + } catch { + // A partial editor (mid-init / mid-tear-down) shouldn't blow up + // the controller; fall back to the previous cache value. + // (The follow-up commit on this branch — `12155be91` — switches + // this to reset to EMPTY_COMMENTS_LIST instead, so cross-document + // swaps don't leak. Kept here as the original commit for + // rebase-stack continuity.) + } + }; + refreshCommentsListCache(); + const computeState = (): SuperDocUIState => { // Route through PresentationEditor when active so selection state // follows the body/header/footer/note editor the user is actually @@ -195,11 +229,20 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { const empty = selectionInfo ? selectionInfo.empty : true; const quotedText = selectionInfo?.text ?? ''; const documentMode = superdoc.config?.documentMode ?? null; + // `activeCommentIds` is post-SD-2792; older builds will have + // `selectionInfo.activeCommentIds === undefined`. Fall back to [] + // so the snapshot shape is stable for consumers either way. + const activeIds = selectionInfo?.activeCommentIds ?? []; return { ready, documentMode, selection: { empty, quotedText }, toolbar: toolbarSnapshot, + comments: { + total: commentsListCache.total, + items: commentsListCache.items, + activeIds, + }, }; }; @@ -223,6 +266,11 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { let currentEditor: SuperDocEditorLike | null = null; let currentEditorTeardown: (() => void) | null = null; + const refreshAndNotify = () => { + refreshCommentsListCache(); + scheduleNotify(); + }; + const attachEditorListeners = () => { const next = resolveRoutedEditor(superdoc); if (next === currentEditor) return; @@ -234,11 +282,21 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { EDITOR_EVENTS.forEach((name) => { next.on?.(name, scheduleNotify); }); + // Comment-list invalidation runs ahead of scheduleNotify so the + // subsequent state recompute sees the fresh items array. Without + // this, `state.comments.items` would lag one tick behind a create/ + // patch/delete. + COMMENTS_REFRESH_EVENTS.forEach((name) => { + next.on?.(name, refreshAndNotify); + }); currentEditorTeardown = () => { EDITOR_EVENTS.forEach((name) => next.off?.(name, scheduleNotify)); + COMMENTS_REFRESH_EVENTS.forEach((name) => next.off?.(name, refreshAndNotify)); }; - // The set of source events changed — recompute state so subscribers - // see the new routed editor's selection. + // The set of source events changed and the routed editor swapped + // — refresh the comments cache for the new editor and recompute + // state so subscribers see the new selection. + refreshCommentsListCache(); scheduleNotify(); }; @@ -441,6 +499,84 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { }, }); + // ---- ui.comments --------------------------------------------------------- + // + // Subscribe is built on the substrate so consumers ride the same + // microtask-coalesced burst pattern as `ui.select`. Action methods + // are convenience facades that route through `editor.doc.comments.*` + // — they do NOT introduce a parallel mutation contract; both + // `ui.comments.resolve(id)` and `editor.doc.comments.patch({ id, + // status: 'resolved' })` produce the same document mutation. + + const requireDocComments = () => { + const editor = resolveRoutedEditor(superdoc); + const api = editor?.doc?.comments; + if (!api) { + throw new Error('ui.comments: no active editor / comments API. Open a document first.'); + } + return api; + }; + + const requireDocRanges = () => { + const editor = resolveRoutedEditor(superdoc); + const api = editor?.doc?.ranges; + if (!api?.scrollIntoView) { + throw new Error('ui.comments.scrollTo: no active editor / ranges API.'); + } + return api; + }; + + const comments: CommentsHandle = { + getSnapshot: () => computeState().comments, + subscribe(listener) { + return select((state) => state.comments, shallowEqual).subscribe((snapshot) => { + try { + listener({ snapshot }); + } catch { + // see scheduleNotify + } + }); + }, + createFromSelection({ text }) { + const editor = resolveRoutedEditor(superdoc); + const target = editor?.doc?.selection?.current?.()?.target; + if (!target) { + return { + success: false, + failure: { code: 'NO_OP', message: 'ui.comments.createFromSelection: no addressable selection target.' }, + }; + } + const api = requireDocComments(); + return (api.create as (input: unknown, options?: unknown) => Receipt).call(api, { target, text }); + }, + resolve(commentId) { + const api = requireDocComments(); + return (api.patch as (input: unknown, options?: unknown) => Receipt).call(api, { commentId, status: 'resolved' }); + }, + reopen(commentId) { + // Routes through `comments.patch({ status: 'active' })`. Today + // doc-api validation rejects anything other than 'resolved' — + // SD-2789 widens the union and ships the lifecycle inverse. + // Until then this surfaces an INVALID_INPUT receipt or throws, + // which is the correct visible behavior for a not-yet-shipped + // operation rather than a silent no-op. + const api = requireDocComments(); + return (api.patch as (input: unknown, options?: unknown) => Receipt).call(api, { commentId, status: 'active' }); + }, + delete(commentId) { + const api = requireDocComments(); + return (api.delete as (input: unknown, options?: unknown) => Receipt).call(api, { commentId }); + }, + async scrollTo(commentId) { + const api = requireDocRanges(); + return (api.scrollIntoView as (input: unknown) => Promise).call(api, { + target: { kind: 'entity', entityType: 'comment', entityId: commentId }, + block: 'center', + behavior: 'smooth', + }); + }, + }; + const destroy = () => { if (destroyed) return; destroyed = true; @@ -457,5 +593,5 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { teardown.length = 0; }; - return { select, toolbar, commands, destroy }; + return { select, toolbar, commands, comments, destroy }; } diff --git a/packages/super-editor/src/ui/index.ts b/packages/super-editor/src/ui/index.ts index d895b34b2f..efd055f401 100644 --- a/packages/super-editor/src/ui/index.ts +++ b/packages/super-editor/src/ui/index.ts @@ -20,6 +20,8 @@ export { createSuperDocUI } from './create-super-doc-ui.js'; export { shallowEqual } from './equality.js'; export type { + CommentsHandle, + CommentsSlice, EqualityFn, SelectorFn, SelectionSlice, diff --git a/packages/super-editor/src/ui/types.ts b/packages/super-editor/src/ui/types.ts index da7aa775ab..08089ea1f9 100644 --- a/packages/super-editor/src/ui/types.ts +++ b/packages/super-editor/src/ui/types.ts @@ -56,8 +56,27 @@ export interface SuperDocEditorLike { empty: boolean; text?: string; target?: unknown; + /** Present after SD-2792; absent on older builds — controller falls back to []. */ + activeCommentIds?: string[]; + activeChangeIds?: string[]; }; }; + /** + * Comments member on the Document API. The structural typing + * keeps the controller loose from the real `CommentsApi` interface + * to allow stub-driven unit tests without pulling in the full + * adapter graph; runtime calls forward to the real `editor.doc`. + */ + comments?: { + list?(query?: unknown): unknown; + create?(input: unknown, options?: unknown): unknown; + patch?(input: unknown, options?: unknown): unknown; + delete?(input: unknown, options?: unknown): unknown; + }; + /** Ranges member on the Document API. Used for `ui.comments.scrollTo`. */ + ranges?: { + scrollIntoView?(input: unknown): Promise; + }; }; } @@ -91,6 +110,15 @@ export interface SuperDocUIState { * (fine-grained per-command observables). */ toolbar: ToolbarSnapshotSlice; + /** + * Comments slice. Sourced from `editor.doc.comments.list()` and + * cached at the controller level — the list is refreshed on + * `commentsUpdate` / `commentsLoaded` events, not recomputed per + * `computeState()` call. `activeIds` mirrors + * `selection.current().activeCommentIds` so a comment-aware sidebar + * can highlight the active card without a separate subscription. + */ + comments: CommentsSlice; } /** @@ -106,6 +134,28 @@ export interface SelectionSlice { quotedText: string; } +/** + * Snapshot of the comments collection exposed on `state.comments`. + * + * Items use the same shape `editor.doc.comments.list()` returns + * (`DiscoveryItem`), so consumers that already consume + * that contract see no shape mismatch. `activeIds` is a denormalized + * convenience driven by `selection.current().activeCommentIds`. + */ +export interface CommentsSlice { + /** Total count from the list result (before pagination, if any). */ + total: number; + /** Items from `editor.doc.comments.list()`. Empty array on error or no editor. */ + items: import('@superdoc/document-api').CommentsListResult['items']; + /** + * Comment IDs whose `commentMark` overlaps the current selection + * (or covers the caret when empty). Empty array when the editor's + * `selection.current()` predates SD-2792 (no `activeCommentIds` + * field) — the controller falls back gracefully. + */ + activeIds: string[]; +} + export interface SuperDocUIOptions { superdoc: SuperDocLike; } @@ -122,7 +172,6 @@ export interface SuperDocUI { */ select(selector: SelectorFn, equality?: EqualityFn): Subscribable; - /** * Aggregate toolbar surface. Mirrors the `HeadlessToolbarController` * shape from `superdoc/headless-toolbar`, sourced from the same * internal controller. Equivalent to subscribing to the toolbar slice @@ -140,6 +189,16 @@ export interface SuperDocUI { */ commands: CommandsHandle; + /** + * Comments domain — single subscription + actions surface. Subscribe + * to receive snapshot updates (items + activeIds + total); call + * action methods to mutate. All mutations route through + * `editor.doc.comments.*` (the Document API contract); this handle + * exists to give UI consumers a stable surface, not to be a parallel + * mutation contract. + */ + comments: CommentsHandle; + /** * Tear down all internal subscriptions to the editor / SuperDoc * instance / presentation editor. After destroy, no listeners will @@ -212,3 +271,43 @@ export type ToolbarCommandHandleState; }; + +/** + * Comments domain handle exposed on `ui.comments`. The execute + * methods are convenience facades over `editor.doc.comments.*` — + * they produce identical document mutations to direct doc-API calls. + */ +export interface CommentsHandle { + /** Snapshot the current comments slice synchronously. */ + getSnapshot(): CommentsSlice; + /** + * Subscribe to comments-snapshot changes. Listener fires once + * synchronously with the current snapshot, then again whenever + * items, activeIds, or total change (shallow equality). + * Returns an unsubscribe. + */ + subscribe(listener: (event: { snapshot: CommentsSlice }) => void): () => void; + /** + * Create a comment anchored to the current selection. Reads the + * routed editor's `selection.current().target` and routes through + * `editor.doc.comments.create`. Returns the operation receipt. + */ + createFromSelection(input: { text: string }): import('@superdoc/document-api').Receipt; + /** Resolve a comment via `editor.doc.comments.patch`. */ + resolve(commentId: string): import('@superdoc/document-api').Receipt; + /** + * Reopen a resolved comment via `editor.doc.comments.patch({ status: + * 'active' })`. Currently throws `INVALID_INPUT` on the doc-API + * because the patch input only accepts `'resolved'`; SD-2789 adds + * the lifecycle inverse and reroutes this method to succeed. + */ + reopen(commentId: string): import('@superdoc/document-api').Receipt; + /** Delete a comment via `editor.doc.comments.delete`. */ + delete(commentId: string): import('@superdoc/document-api').Receipt; + /** + * Scroll the viewport to the comment's anchor via + * `editor.doc.ranges.scrollIntoView({ target: EntityAddress })`. + * Resolves to the receipt the doc-API returns. + */ + scrollTo(commentId: string): Promise; +} diff --git a/packages/superdoc/src/ui.d.ts b/packages/superdoc/src/ui.d.ts index 9b39f862ba..b99d665d7b 100644 --- a/packages/superdoc/src/ui.d.ts +++ b/packages/superdoc/src/ui.d.ts @@ -1,6 +1,8 @@ export { createSuperDocUI, shallowEqual, + type CommentsHandle, + type CommentsSlice, type EqualityFn, type SelectorFn, type SelectionSlice, From c91d74e805c8796a232c901895cb11fb71f76411 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Tue, 28 Apr 2026 20:27:19 -0300 Subject: [PATCH 2/3] fix(superdoc/ui): clear comments cache when list() refresh fails Addresses PR #2982 review (P1). When `editor.doc.comments.list()` threw mid-refresh, the previous code preserved the prior cache so a "transient" failure wouldn't blank the UI. Real failure mode that matters more: during a document / editor swap, the new editor can throw transiently while initializing, and keeping the prior value silently leaks the old document's comments into the new editor's snapshot until the next successful refresh. Reset to `EMPTY_COMMENTS_LIST` on failure. Briefly rendering an empty list is a much better failure mode than rendering the wrong document's comments. Regression test: a `list()` that throws on `commentsUpdate` clears the cache so the next snapshot reports `total: 0` / `items: []`. --- packages/super-editor/src/ui/comments.test.ts | 28 +++++++++++++++++++ .../src/ui/create-super-doc-ui.ts | 13 +++++---- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/packages/super-editor/src/ui/comments.test.ts b/packages/super-editor/src/ui/comments.test.ts index 324ed5e078..a09b6b0f2f 100644 --- a/packages/super-editor/src/ui/comments.test.ts +++ b/packages/super-editor/src/ui/comments.test.ts @@ -162,6 +162,34 @@ describe('ui.comments — snapshot', () => { ui.destroy(); }); + it('clears the cache when comments.list() throws on refresh (no cross-document stale leakage)', async () => { + const { superdoc, mocks } = makeStubs({ + comments: [ + { id: 'c1', commentId: 'c1', text: 'first' }, + { id: 'c2', commentId: 'c2', text: 'second' }, + ], + }); + const ui = createSuperDocUI({ superdoc }); + + // Start with a populated snapshot. + expect(ui.comments.getSnapshot().total).toBe(2); + + // Simulate a document/editor swap where the new editor's list() + // throws transiently. The cache must reset to empty rather than + // continue serving the old editor's items. + mocks.list.mockImplementationOnce(() => { + throw new Error('editor mid-swap'); + }); + superdoc.fireEditor('commentsUpdate'); + await flushMicrotasks(); + + const snap = ui.comments.getSnapshot(); + expect(snap.total).toBe(0); + expect(snap.items).toEqual([]); + + ui.destroy(); + }); + it('falls back to [] when selection.current() predates SD-2792 (no activeCommentIds field)', () => { const { superdoc, editor } = makeStubs(); // Override selection.current to return an SD-2668-shaped result diff --git a/packages/super-editor/src/ui/create-super-doc-ui.ts b/packages/super-editor/src/ui/create-super-doc-ui.ts index ce3a5683ce..09ba7a5c67 100644 --- a/packages/super-editor/src/ui/create-super-doc-ui.ts +++ b/packages/super-editor/src/ui/create-super-doc-ui.ts @@ -208,12 +208,13 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { const result = list.call(editor.doc!.comments, undefined) as CommentsListResult | undefined; commentsListCache = result ?? EMPTY_COMMENTS_LIST; } catch { - // A partial editor (mid-init / mid-tear-down) shouldn't blow up - // the controller; fall back to the previous cache value. - // (The follow-up commit on this branch — `12155be91` — switches - // this to reset to EMPTY_COMMENTS_LIST instead, so cross-document - // swaps don't leak. Kept here as the original commit for - // rebase-stack continuity.) + // Reset to empty rather than retaining the previous editor's + // cache. During document / editor swaps the new editor can + // throw transiently while initializing — keeping the prior + // value would leak the old document's comments into the new + // one's snapshot until the next successful refresh, which is a + // worse failure mode than briefly rendering an empty list. + commentsListCache = EMPTY_COMMENTS_LIST; } }; refreshCommentsListCache(); From 6854e1265d0dd6df592e95b5152186647d91d288 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Tue, 28 Apr 2026 20:34:25 -0300 Subject: [PATCH 3/3] fix(superdoc/ui): refresh on own mutations + stable activeIds + barrel exports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses three review findings on PR #2982: P1: own-mutation refresh. Each ui.comments.* mutation now calls refreshAndNotify() after the doc-API call returns. The underlying wrappers don't emit a single canonical event for every mutation (some go through transaction only, some emit commentsUpdate ahead of the entity-store finishing) — relying on those events alone left subscribers seeing stale items/total until some later event fired. Refreshing here makes the post-mutation state visible synchronously to the next snapshot. P2-3: activeIds reference stability. Pre-SD-2792 selection results have no activeCommentIds field; the previous fallback `?? []` allocated a fresh array per computeState() call, defeating shallowEqual on the comments snapshot — every selection event re-fired ui.comments.subscribe even when the slice was unchanged. Use a frozen module-scope sentinel (`EMPTY_ACTIVE_IDS`) so the array reference is stable. P2-1: barrel exports. The public `superdoc/ui` shim re-exports `CommentsHandle` and `CommentsSlice` from `@superdoc/super-editor`, but the super-editor `src/index.ts` UI types block didn't include them, so TypeScript consumers got "no exported member" on import. Added both to the barrel. P2-2 (clear comments cache on list refresh errors) was already addressed in 12155be91 from the previous review round. Tests: regression for own-mutation refresh (createFromSelection / resolve / delete each produce a fresh snapshot synchronously) and for empty-activeIds reference stability across snapshots. 29 UI tests; super-editor 11916 / 13 skipped / 0 fail. --- packages/super-editor/src/index.ts | 2 + packages/super-editor/src/ui/comments.test.ts | 64 +++++++++++++++++++ .../src/ui/create-super-doc-ui.ts | 47 ++++++++++++-- 3 files changed, 106 insertions(+), 7 deletions(-) diff --git a/packages/super-editor/src/index.ts b/packages/super-editor/src/index.ts index e176cd2740..24b21d01a2 100644 --- a/packages/super-editor/src/index.ts +++ b/packages/super-editor/src/index.ts @@ -165,6 +165,8 @@ export type { // superdoc/ui public types (browser UI controller) export type { + CommentsHandle, + CommentsSlice, EqualityFn, SelectorFn, SelectionSlice, diff --git a/packages/super-editor/src/ui/comments.test.ts b/packages/super-editor/src/ui/comments.test.ts index a09b6b0f2f..e7f19e170c 100644 --- a/packages/super-editor/src/ui/comments.test.ts +++ b/packages/super-editor/src/ui/comments.test.ts @@ -190,6 +190,70 @@ describe('ui.comments — snapshot', () => { ui.destroy(); }); + it('returns the same array reference for empty activeIds across snapshots (shallowEqual stability)', () => { + // Pre-SD-2792 selection shape: no activeCommentIds. Without a + // shared sentinel, `?? []` would allocate a fresh array each + // computeState() call and trigger shallowEqual mismatch on the + // comments snapshot — every selection event would re-fire + // ui.comments.subscribe. + const { superdoc, editor } = makeStubs({ comments: [{ id: 'c1', commentId: 'c1' }] }); + (editor.doc.selection.current as unknown as () => { empty: boolean; target: null }) = vi.fn(() => ({ + empty: true, + target: null, + })); + const ui = createSuperDocUI({ superdoc }); + + const a = ui.comments.getSnapshot().activeIds; + const b = ui.comments.getSnapshot().activeIds; + expect(a).toBe(b); // same reference + + ui.destroy(); + }); + + it('refreshes the snapshot synchronously after own mutations (createFromSelection / resolve / delete)', () => { + const target = { kind: 'text' as const, segments: [{ blockId: 'p1', range: { start: 0, end: 5 } }] }; + const { superdoc, mocks } = makeStubs({ + comments: [{ id: 'c1', commentId: 'c1', text: 'first' }], + selectionTarget: target, + }); + const ui = createSuperDocUI({ superdoc }); + + const cb = vi.fn(); + ui.comments.subscribe(cb); + expect(cb).toHaveBeenCalledTimes(1); + + // Simulate the wrapper updating the comments store: as soon as + // ui.comments.createFromSelection completes, list() must return + // the new item. The own-mutation refresh re-reads list() so + // subscribers see the post-mutation state without needing a + // commentsUpdate event. + superdoc.setComments([ + { id: 'c1', commentId: 'c1', text: 'first' }, + { id: 'c2', commentId: 'c2', text: 'second' }, + ]); + ui.comments.createFromSelection({ text: 'second' }); + + expect(mocks.create).toHaveBeenCalledTimes(1); + // getSnapshot reflects the new state synchronously after the + // mutation (without needing a commentsUpdate event). + expect(ui.comments.getSnapshot().total).toBe(2); + + // Same pattern for resolve. + superdoc.setComments([ + { id: 'c1', commentId: 'c1', status: 'resolved' }, + { id: 'c2', commentId: 'c2', text: 'second' }, + ]); + ui.comments.resolve('c1'); + expect(ui.comments.getSnapshot().items[0].status).toBe('resolved'); + + // And for delete. + superdoc.setComments([{ id: 'c2', commentId: 'c2', text: 'second' }]); + ui.comments.delete('c1'); + expect(ui.comments.getSnapshot().total).toBe(1); + + ui.destroy(); + }); + it('falls back to [] when selection.current() predates SD-2792 (no activeCommentIds field)', () => { const { superdoc, editor } = makeStubs(); // Override selection.current to return an SD-2668-shaped result diff --git a/packages/super-editor/src/ui/create-super-doc-ui.ts b/packages/super-editor/src/ui/create-super-doc-ui.ts index 09ba7a5c67..bd689480b9 100644 --- a/packages/super-editor/src/ui/create-super-doc-ui.ts +++ b/packages/super-editor/src/ui/create-super-doc-ui.ts @@ -94,6 +94,16 @@ const ALL_TOOLBAR_COMMAND_IDS: PublicToolbarItemId[] = Object.keys( createToolbarRegistry(), ) as PublicToolbarItemId[]; +/** + * Frozen empty-array sentinel for `state.comments.activeIds` when + * `selection.current()` predates SD-2792 (no `activeCommentIds` + * field). Allocating a fresh `[]` per `computeState()` would change + * the array reference every call and defeat `shallowEqual` on the + * comments snapshot — every selection event would re-fire + * `ui.comments.subscribe` even when nothing in the slice changed. + */ +const EMPTY_ACTIVE_IDS: readonly string[] = Object.freeze([]); + /** * Resolve the **routed** editor — the body, header, footer, or note * editor that PresentationEditor currently routes input/selection to. @@ -231,9 +241,11 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { const quotedText = selectionInfo?.text ?? ''; const documentMode = superdoc.config?.documentMode ?? null; // `activeCommentIds` is post-SD-2792; older builds will have - // `selectionInfo.activeCommentIds === undefined`. Fall back to [] - // so the snapshot shape is stable for consumers either way. - const activeIds = selectionInfo?.activeCommentIds ?? []; + // `selectionInfo.activeCommentIds === undefined`. Fall back to a + // frozen shared array so the array reference is stable across + // computeState() calls (otherwise shallowEqual on the comments + // snapshot re-fires every selection event). + const activeIds = (selectionInfo?.activeCommentIds ?? EMPTY_ACTIVE_IDS) as string[]; return { ready, documentMode, @@ -548,11 +560,25 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { }; } const api = requireDocComments(); - return (api.create as (input: unknown, options?: unknown) => Receipt).call(api, { target, text }); + const receipt = (api.create as (input: unknown, options?: unknown) => Receipt).call(api, { target, text }); + // Refresh + notify ourselves: the underlying wrappers don't + // emit a single canonical event for every comments mutation + // (some go through `transaction` only, some emit + // `commentsUpdate` ahead of the entity-store finishing). Doing + // it here means the next snapshot subscribers see is the + // post-mutation state, regardless of which event the wrapper + // happens to fire. + refreshAndNotify(); + return receipt; }, resolve(commentId) { const api = requireDocComments(); - return (api.patch as (input: unknown, options?: unknown) => Receipt).call(api, { commentId, status: 'resolved' }); + const receipt = (api.patch as (input: unknown, options?: unknown) => Receipt).call(api, { + commentId, + status: 'resolved', + }); + refreshAndNotify(); + return receipt; }, reopen(commentId) { // Routes through `comments.patch({ status: 'active' })`. Today @@ -562,11 +588,18 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { // which is the correct visible behavior for a not-yet-shipped // operation rather than a silent no-op. const api = requireDocComments(); - return (api.patch as (input: unknown, options?: unknown) => Receipt).call(api, { commentId, status: 'active' }); + const receipt = (api.patch as (input: unknown, options?: unknown) => Receipt).call(api, { + commentId, + status: 'active', + }); + refreshAndNotify(); + return receipt; }, delete(commentId) { const api = requireDocComments(); - return (api.delete as (input: unknown, options?: unknown) => Receipt).call(api, { commentId }); + const receipt = (api.delete as (input: unknown, options?: unknown) => Receipt).call(api, { commentId }); + refreshAndNotify(); + return receipt; }, async scrollTo(commentId) { const api = requireDocRanges();