fix(numbering): prevent infinite appendTransaction loop in collaboration mode (SD-2060)#2224
Conversation
…ion mode SD-2060: When numberingPlugin sets listRendering via setNodeAttribute, y-prosemirror syncs these changes to the Y.js XmlFragment, which then echoes them back as a new transaction — triggering numberingPlugin again in an infinite loop. This adds a guard using ySyncPluginKey metadata to skip appendTransaction when the transaction originates from Y.js sync.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12daeb162a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (isFromPlugin || isFromYjsSync || (!forcePluginPass && !forceFullRecompute && !hasDocChanges)) { | ||
| return null; |
There was a problem hiding this comment.
Handle remote Yjs updates without skipping recomputation
The new early return on isFromYjsSync skips appendTransaction for every transaction tagged with ySyncPluginKey.isChangeOrigin, but that flag is used for all Yjs-origin updates (not only local echo-back). As a result, remote collaborator edits that change list structure never trigger a local numbering recompute, so merged concurrent edits can leave stale listRendering values until someone makes another non-Yjs edit. Please narrow this guard to skip only self-echo attribute transactions rather than all Yjs-origin document changes.
Useful? React with 👍 / 👎.
| // sdBlockRev). Without this guard, y-prosemirror syncs our setNodeAttribute | ||
| // calls to the Y.XmlFragment, then fires a change event that dispatches a | ||
| // new transaction back to PM — creating an infinite appendTransaction loop. | ||
| const isFromYjsSync = transactions.some((tr) => { |
No description provided.