Skip to content
Merged
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
17 changes: 6 additions & 11 deletions packages/super-editor/src/extensions/comment/comments-plugin.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
import { Plugin, PluginKey, TextSelection } from 'prosemirror-state';
import { Extension } from '@core/Extension.js';
import { Plugin, PluginKey, TextSelection } from 'prosemirror-state';
import { Decoration, DecorationSet } from 'prosemirror-view';
import { CommentMarkName } from './comments-constants.js';
import {
getHighlightColor,
removeCommentsById,
resolveCommentById,
getHighlightColor,
translateFormatChangesToEnglish,
} from './comments-helpers.js';
import { CommentMarkName } from './comments-constants.js';

// Example tracked-change keys, if needed
import { TrackInsertMarkName, TrackDeleteMarkName, TrackFormatMarkName } from '../track-changes/constants.js';
import { comments_module_events } from '@superdoc/common';
import { v4 as uuidv4 } from 'uuid';
import { TrackDeleteMarkName, TrackFormatMarkName, TrackInsertMarkName } from '../track-changes/constants.js';
import { TrackChangesBasePluginKey } from '../track-changes/plugins/index.js';
import { getTrackChanges } from '../track-changes/trackChangesHelpers/getTrackChanges.js';
import { comments_module_events } from '@superdoc/common';
import { normalizeCommentEventPayload, updatePosition } from './helpers/index.js';
import { v4 as uuidv4 } from 'uuid';

const TRACK_CHANGE_MARKS = [TrackInsertMarkName, TrackDeleteMarkName, TrackFormatMarkName];

Expand Down Expand Up @@ -510,11 +510,6 @@ export const CommentsPlugin = Extension.create({

shouldUpdate = true;
editor.emit('commentsUpdate', update);

const { tr: newTr } = editor.view.state;
const { dispatch } = editor.view;
newTr.setMeta(CommentsPluginKey, { type: 'force' });
dispatch(newTr);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ const createEditorEnvironment = (schema, doc) => {
extension.addPmPlugins = CommentsPlugin.config.addPmPlugins.bind(extension);
extension.editor = editor;

return { editor, commands: extension.addCommands(), view };
return { editor, commands: extension.addCommands(), view, extension };
};

describe('CommentsPlugin commands', () => {
Expand Down Expand Up @@ -1229,3 +1229,86 @@ describe('getActiveCommentId - nested comments and TC precedence', () => {
expect(getActiveCommentId(doc, TextSelection.create(doc, 8))).toBe('b');
});
});

describe('SD-1940: no recursive dispatch from apply() on selection change', () => {
it('does not dispatch from apply() when selection moves onto a comment', () => {
const schema = createCommentSchema();
const commentMark = schema.marks[CommentMarkName].create({ commentId: 'c-1' });
const paragraph = schema.node('paragraph', null, [
schema.text('Plain text. '),
schema.text('Commented text.', [commentMark]),
]);
const doc = schema.node('doc', null, [paragraph]);

const { editor, view, extension } = createEditorEnvironment(schema, doc);
const plugins = extension.addPmPlugins();

// Create state WITH the comments plugin so apply() runs
const initialState = EditorState.create({
schema,
doc,
selection: TextSelection.create(doc, 3),
plugins,
});
view.state = initialState;

// Track dispatch calls
const dispatchSpy = vi.fn((tr) => {
view.state = view.state.apply(tr);
});
view.dispatch = dispatchSpy;

// Move selection onto commented text (pos 14) — triggers active thread change in apply()
const tr = initialState.tr.setSelection(TextSelection.create(doc, 14));
const newState = initialState.apply(tr);
view.state = newState;

// apply() should NOT have called view.dispatch() (the old bug dispatched a 'force' transaction)
expect(dispatchSpy).not.toHaveBeenCalled();

// But the commentsUpdate event should still have been emitted
expect(editor.emit).toHaveBeenCalledWith(
'commentsUpdate',
expect.objectContaining({
type: comments_module_events.SELECTED,
activeCommentId: 'c-1',
}),
);
});

it('handles programmatic selection + addComment without recursive dispatch', () => {
const schema = createCommentSchema();
const paragraph = schema.node('paragraph', null, [schema.text('Hello world')]);
const doc = schema.node('doc', null, [paragraph]);

const { editor, commands, view, extension } = createEditorEnvironment(schema, doc);
const plugins = extension.addPmPlugins();

const initialState = EditorState.create({
schema,
doc,
selection: TextSelection.create(doc, 1, 1),
plugins,
});
view.state = initialState;

let dispatchCount = 0;
view.dispatch = vi.fn((tr) => {
dispatchCount++;
if (dispatchCount > 10) throw new Error('Dispatch loop detected — exceeded 10 dispatches');
view.state = view.state.apply(tr);
});

// Step 1: Programmatically select text (like the customer's code)
const selTr = view.state.tr.setSelection(TextSelection.create(view.state.doc, 1, 6));
view.dispatch(selTr);

// Step 2: Immediately add a comment (like the customer's code)
const addCommentCmd = commands.addComment({ content: 'Test comment' });
const addTr = view.state.tr;
addCommentCmd({ tr: addTr, state: view.state, dispatch: view.dispatch, editor });

// Should complete without loop — max 2-3 dispatches (selection + addComment + maybe decoration)
expect(dispatchCount).toBeLessThanOrEqual(3);
});
});
22 changes: 22 additions & 0 deletions packages/superdoc/src/stores/comments-store.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,28 @@ describe('comments-store', () => {
expect(store.activeComment).toBeNull();
});

it('still syncs editor active comment when store was pre-updated by caller', () => {
const setActiveCommentSpy = vi.fn();
const superdoc = {
activeEditor: {
commands: {
setActiveComment: setActiveCommentSpy,
},
},
};

store.commentsList = [{ commentId: 'comment-3' }];

// Simulate UI flow that pre-updates store state before syncing editor/plugin state.
store.setActiveComment(undefined, 'comment-3');
expect(store.activeComment).toBe('comment-3');

store.setActiveComment(superdoc, 'comment-3');

expect(setActiveCommentSpy).toHaveBeenCalledTimes(1);
expect(setActiveCommentSpy).toHaveBeenCalledWith({ commentId: 'comment-3' });
});

it('updates tracked change comments and emits events', () => {
const superdoc = {
emit: vi.fn(),
Expand Down
91 changes: 91 additions & 0 deletions tests/behavior/tests/comments/sd-1940-comment-focus-loop.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import { test, expect } from '../../fixtures/superdoc.js';
import { assertDocumentApiReady, listComments } from '../../helpers/document-api.js';

test.use({ config: { toolbar: 'full', comments: 'on' } });

test('SD-1940: programmatic selection + addComment does not enter a dispatch loop', async ({ superdoc }) => {
await assertDocumentApiReady(superdoc.page);

await superdoc.type('Hello world');
await superdoc.waitForStable();

const initialComments = await listComments(superdoc.page, { includeResolved: true });

const result = await superdoc.page.evaluate(() => {
const editor = (window as any).editor;
if (!editor?.view?.dispatch) {
throw new Error('Expected editor.view.dispatch to be available.');
}
if (!editor?.commands?.addComment) {
throw new Error('Expected editor.commands.addComment to be available.');
}

const originalDispatch = editor.view.dispatch.bind(editor.view);
let dispatchCount = 0;

editor.view.dispatch = (tr: unknown) => {
dispatchCount += 1;
if (dispatchCount > 25) {
throw new Error('Dispatch loop detected while adding a programmatic comment.');
}
return originalDispatch(tr);
};

try {
const state = editor.view.state;
const SelectionCtor = state.selection?.constructor;
if (!SelectionCtor || typeof SelectionCtor.create !== 'function') {
throw new Error('Unable to construct a text selection from editor state.');
}

const selectionTr = state.tr.setSelection(SelectionCtor.create(state.doc, 1, 6));
editor.view.focus();
editor.view.dispatch(selectionTr);

const addCommentResult = editor.commands.addComment({
content: 'Programmatic comment from SD-1940 behavior test',
});

return {
addCommentResult: addCommentResult === true,
dispatchCount,
};
} finally {
editor.view.dispatch = originalDispatch;
}
});

await superdoc.waitForStable();

expect(result.addCommentResult).toBe(true);
expect(result.dispatchCount).toBeLessThanOrEqual(25);

await expect
.poll(async () => (await listComments(superdoc.page, { includeResolved: true })).total)
.toBe(initialComments.total + 1);

const secondAddCommentResult = await superdoc.page.evaluate(() => {
const editor = (window as any).editor;
const state = editor.view.state;
const SelectionCtor = state.selection?.constructor;
if (!SelectionCtor || typeof SelectionCtor.create !== 'function') {
throw new Error('Unable to construct a follow-up text selection from editor state.');
}

// Select "world" and add a second comment to verify the editor remains usable.
const secondSelectionTr = state.tr.setSelection(SelectionCtor.create(state.doc, 7, 12));
editor.view.dispatch(secondSelectionTr);

return (
editor.commands.addComment({
content: 'Second programmatic comment after SD-1940 sequence',
}) === true
);
});
await superdoc.waitForStable();

expect(secondAddCommentResult).toBe(true);
await expect
.poll(async () => (await listComments(superdoc.page, { includeResolved: true })).total)
.toBe(initialComments.total + 2);
});
Loading