Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions packages/document-api/scripts/check-contract-parity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,18 @@ import { buildDispatchTable } from '../src/invoke/invoke.js';
* implementation, so it is intentionally excluded from the RPC
* dispatch surface and the CLI command catalog. Direct calls through
* `editor.doc.ranges.scrollIntoView()` are still supported.
* - `selection.onChange` is a subscription primitive (push-based, no
* request/response shape) rather than a request-response operation,
* so it is not represented in `OPERATION_DEFINITIONS` / schemas /
* dispatch. Direct calls through `editor.doc.selection.onChange()`
* - `selection.onChange`, `comments.onChange` are subscription primitives
* (push-based, no request/response shape) rather than request-response
* operations, so they are not represented in `OPERATION_DEFINITIONS` /
* schemas / dispatch. Direct calls through
* `editor.doc.selection.onChange()` and `editor.doc.comments.onChange()`
* are still supported.
*/
const META_MEMBER_PATHS = [
'invoke',
'ranges.scrollIntoView',
'selection.onChange',
'comments.onChange',
...REFERENCE_OPERATION_ALIASES.map((alias) => alias.memberPath),
];

Expand Down
1 change: 1 addition & 0 deletions packages/document-api/src/comments/comments.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const stubAdapter = () =>
goTo: mock(() => ({ success: true })),
get: mock(() => ({ commentId: 'c1', status: 'open' })),
list: mock(() => ({ items: [], total: 0 })),
onChange: mock(() => () => {}),
}) as any;

describe('executeCommentsCreate validation', () => {
Expand Down
22 changes: 22 additions & 0 deletions packages/document-api/src/comments/comments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,13 @@ export interface CommentsDeleteInput {
commentId: string;
}

/**
* Listener invoked when the comments collection changes (create, patch,
* delete, document reload). Receives no payload — call `list()` to read
* fresh state.
*/
export type CommentsChangeListener = () => void;

/**
* Engine-specific adapter that the comments API delegates to.
*/
Expand All @@ -144,6 +151,12 @@ export interface CommentsAdapter {
get(input: GetCommentInput): CommentInfo;
/** List comments matching the given query. */
list(query?: CommentsListQuery): CommentsListResult;
/**
* Subscribe to comments-collection changes. Returns an unsubscribe
* function. Implementations should debounce to at most once per tick
* so multi-step transactions and document load don't storm consumers.
*/
onChange(listener: CommentsChangeListener): () => void;
}

/**
Expand All @@ -161,6 +174,15 @@ export interface CommentsApi {
delete(input: CommentsDeleteInput, options?: RevisionGuardOptions): Receipt;
get(input: GetCommentInput): CommentInfo;
list(query?: CommentsListQuery): CommentsListResult;
/**
* Subscribe to comments-collection changes. Listener fires whenever
* a comment is created, patched, deleted, or the document reloads.
*
* Listener receives no payload — call `comments.list()` inside the
* callback to read fresh state. Returns an unsubscribe function;
* call it in cleanup (React `useEffect` return, Vue `onUnmounted`).
*/
onChange(listener: CommentsChangeListener): () => void;
}

const CREATE_COMMENT_ALLOWED_KEYS = new Set(['target', 'text', 'parentCommentId']);
Expand Down
39 changes: 39 additions & 0 deletions packages/document-api/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ function makeCommentsAdapter(): CommentsAdapter {
status: 'open' as const,
})),
list: mock(() => ({ evaluatedRevision: 'r1', total: 0, items: [], page: { limit: 0, offset: 0, returned: 0 } })),
onChange: mock(() => () => {}),
};
}

Expand Down Expand Up @@ -2187,6 +2188,44 @@ describe('createDocumentApi', () => {
});
});

describe('comments.onChange wiring', () => {
function makeApiWithCommentsOnChange() {
const commentsAdpt = makeCommentsAdapter();
const api = createDocumentApi({
find: makeFindAdapter(FIND_RESULT),
get: makeGetAdapter(),
getNode: makeGetNodeAdapter(PARAGRAPH_NODE_RESULT),
getText: makeGetTextAdapter(),
info: makeInfoAdapter(),
capabilities: makeCapabilitiesAdapter(),
comments: commentsAdpt,
write: makeWriteAdapter(),
selectionMutation: makeSelectionMutationAdapter(),
trackChanges: makeTrackChangesAdapter(),
create: makeCreateAdapter(),
lists: makeListsAdapter(),
});
return { api, commentsAdpt };
}

it('forwards listener registration to the comments adapter and returns its unsubscribe', () => {
const { api, commentsAdpt } = makeApiWithCommentsOnChange();
const unsubFromAdapter = mock(() => {});
// Override the default stub so we can assert the returned unsubscribe identity.
commentsAdpt.onChange = mock(() => unsubFromAdapter);

const listener = () => {};
const unsub = api.comments.onChange(listener);

expect(commentsAdpt.onChange).toHaveBeenCalledTimes(1);
// The listener passed through to the adapter should be the same identity
// — not a wrapped function — so adapter implementations can dedupe.
expect((commentsAdpt.onChange as any).mock.calls[0][0]).toBe(listener);
// The unsubscribe handle returned to the caller should be the adapter's.
expect(unsub).toBe(unsubFromAdapter);
});
});

describe('comments.patch target validation', () => {
function makeApi() {
return createDocumentApi({
Expand Down
4 changes: 4 additions & 0 deletions packages/document-api/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ import type { CommentInfo, CommentsListQuery, CommentsListResult } from './comme
import type {
CommentsAdapter,
CommentsApi,
CommentsChangeListener,
CommentsCreateInput,
CommentsPatchInput,
CommentsDeleteInput,
Expand Down Expand Up @@ -1982,6 +1983,9 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi {
list(query?: CommentsListQuery): CommentsListResult {
return executeListComments(adapters.comments, query);
},
onChange(listener: CommentsChangeListener): () => void {
return adapters.comments.onChange(listener);
},
},
insert(input: InsertInput, options?: MutationOptions): SDMutationReceipt {
return executeInsert(adapters.selectionMutation, adapters.write, input, options);
Expand Down
1 change: 1 addition & 0 deletions packages/document-api/src/invoke/invoke.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ function makeAdapters() {
status: 'open' as const,
})),
list: mock(() => ({ evaluatedRevision: '', total: 0, items: [], page: { limit: 50, offset: 0, returned: 0 } })),
onChange: mock(() => () => {}),
};
const writeAdapter: WriteAdapter = {
write: mock(() => ({
Expand Down
1 change: 1 addition & 0 deletions packages/document-api/src/overview-examples.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ function makeCommentsAdapter() {
],
page: { limit: 1, offset: 0, returned: 1 },
})),
onChange: mock(() => () => {}),
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
AddCommentInput,
CommentInfo,
CommentsAdapter,
CommentsChangeListener,

Check failure on line 15 in packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/comments-wrappers.ts

View workflow job for this annotation

GitHub Actions / test (chromium, 3)

Module '"@superdoc/document-api"' has no exported member 'CommentsChangeListener'.

Check failure on line 15 in packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/comments-wrappers.ts

View workflow job for this annotation

GitHub Actions / test (chromium, 1)

Module '"@superdoc/document-api"' has no exported member 'CommentsChangeListener'.

Check failure on line 15 in packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/comments-wrappers.ts

View workflow job for this annotation

GitHub Actions / test (chromium, 4)

Module '"@superdoc/document-api"' has no exported member 'CommentsChangeListener'.

Check failure on line 15 in packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/comments-wrappers.ts

View workflow job for this annotation

GitHub Actions / test (chromium, 2)

Module '"@superdoc/document-api"' has no exported member 'CommentsChangeListener'.

Check failure on line 15 in packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/comments-wrappers.ts

View workflow job for this annotation

GitHub Actions / build

Module '"@superdoc/document-api"' has no exported member 'CommentsChangeListener'.
CommentsListQuery,
CommentsListResult,
EditCommentInput,
Expand Down Expand Up @@ -977,6 +978,42 @@
// Adapter factory
// ---------------------------------------------------------------------------

/**
* Subscribe to comments-collection changes. Forwards the editor's
* `commentsUpdate` event to the listener with no payload — consumers
* call `comments.list()` to read fresh state.
*
* Coalesces bursts to one notification per microtask so multi-step
* transactions and DOCX reload don't storm consumers. Returns an
* unsubscribe function.
*/
function subscribeToCommentsChanges(editor: Editor, listener: CommentsChangeListener): () => void {
let scheduled = false;
let active = true;
const handler = () => {
if (scheduled || !active) return;
scheduled = true;
queueMicrotask(() => {
scheduled = false;
if (!active) return;
try {
listener();
} catch {
// Listener errors are not propagated. Subscribers manage their
// own error handling; we don't want one buggy listener to wedge
// the editor's event loop or block other listeners.
}
});
};

editor.on?.('commentsUpdate', handler);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Listen for commentsLoaded in comments.onChange

subscribeToCommentsChanges only subscribes to commentsUpdate, but document open/reload initializes comments via the editor’s commentsLoaded event (Editor.#initComments) without guaranteeing a commentsUpdate afterwards. That means editor.doc.comments.onChange() can miss the initial/reloaded comment collection entirely, leaving consumers (e.g. comment sidebars that call comments.list() in the callback) stale until a later mutation happens.

Useful? React with 👍 / 👎.


return () => {
active = false;
editor.off?.('commentsUpdate', handler);
};
}

export function createCommentsWrapper(editor: Editor): CommentsAdapter {
return {
add: (input: AddCommentInput, options?: RevisionGuardOptions) => addCommentHandler(editor, input, options),
Expand All @@ -994,5 +1031,6 @@
goTo: (input: GoToCommentInput) => goToCommentHandler(editor, input),
get: (input: GetCommentInput) => getCommentHandler(editor, input),
list: (query?: CommentsListQuery) => listCommentsHandler(editor, query),
onChange: (listener: CommentsChangeListener) => subscribeToCommentsChanges(editor, listener),
};
}
Loading