Repository navigation
Conversation
A block's nested children are no longer a reason to replace the block. A binding transformer shows all of a block's child groups as one group and keeps emptied groups (hidden) instead of deleting them, so indents, unindents and concurrent first children diff as moved/added blocks only.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
YousefED
added this pull request to stack #3169
October 8, 2026 04:22
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
yDocToBlocks converted Y without the binding's transformers, so a block with two child groups (concurrent first children) made the whole document read as empty.
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/diagram-block
@blocknote/mantine
@blocknote/math-block
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-ai
@blocknote/xl-docx-exporter
@blocknote/xl-email-exporter
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
@blocknote/xl-typst-exporter
commit: |
|
…ffs-in-place # Conflicts: # packages/core/src/y/extensions/versionDiffAttribution.test.ts
…ffs-in-place # Conflicts: # examples/07-collaboration/14-suggestion-gallery/src/scenarios.ts # packages/core/src/y/extensions/versionDiffAttribution.test.ts
…ffs-in-place # Conflicts: # examples/07-collaboration/14-suggestion-gallery/src/scenarios.ts # packages/core/src/y/extensions/versionDiffAttribution.test.ts
3 of 4 tasks
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #3167. Draft: the transformer is new. It changes how concurrent moves resolve (see Trade-off).
Problem
blockMatchNodesreplaces the full container of a block when the block gets or loses its child group. Thus, the diff of an indent shows three changes:Unindent, a new first child and the deletion of a last child also do this. This causes most of the "noisy diff" in version history and suggestions.
The rule has a reason. Two users can give a block its first child at the same time. Without the rule, this puts two
blockGroups in one container. The schema (blockContent blockGroup?) cannot show this. Thus, @y/prosemirror drops a group.Change
blockMatchNodes.ts: remove the nesting rule. If only the children of a block change, the container of the block stays.mergeBlockGroups.ts(new): a binding transformer on eachblockContainer, as Kevin suggested:YSync.ts: registers the transformer.utils.ts:yDocToBlocks/yfragmentToBlocks(for example,ServerBlockNoteEditor) andyNodeToTransactionnow use the same render pipeline as the binding. Before this change, a block with two child groups causedyDocToBlocksto return an empty document. TheyNodeToTransactionchange is only for consistency. Its PM-to-PM diffs cannot make two groups.Covered
Baselines updated (reviewed, Chromium/Firefox/WebKit):
addRemoveBlocks(delete nested, nest bullet);nesting(indent, unindent);nesting.concurrent(both cases);moveBlocksHTML snapshot. The group itself no longer has an attribution mark. Its children have the mark. Thus, it renders the same.Versioning snapshot (
tests/src/end-to-end/y-prosemirror/__snapshots__/versioning.test.tsx.snap):blockGroupnodes no longer have their own attribution mark.Not covered / trade-off
A move is still a delete and a copy. Yjs has no move. This limits the cases that follow:
A user writes content into a block that a different user moves at the same time. This content is lost. The gallery shows this:
Thus, the old rule changed these conflicts into duplicates. This PR changes them into loss. In the two cases, no deletion is attributed to a user who did not make it. A correct fix needs move support, or a pair of delete and insert by block id.
Known issue: a version deletes the only top-level block of the document (for example, "Delete a parent block" and "Delete parent with mixed children"). The diff pairs the deleted parent with the empty placeholder block of the editor. Thus, the deleted parent loses its block-level delete mark. Its text and its children still show as deleted by A. The versioning snapshot records this.
A user changes the type of a parent while a different user edits its child (gallery case "Change a parent's type vs edit its child"): the edit of B is lost. This occurs with and without this PR, because a type change still replaces the block.
Enter at the start of a heading (single-user gallery case): the diff shows the heading text as deleted and inserted again in a new block. The split keeps the block id on the empty first half. Thus, this is not a nesting problem. An id check in
blockMatchNodesdoes not fix it.Mixed versions: a client without the transformer can edit the same document. That client sees the extra groups as content that the schema does not allow. It drops them, and it deletes empty groups. feat!: rebuild version history and customize snapshot actions #3090 is not released. Thus, this is important only if the two are released separately.
Interaction with fix(versioning): diff structural changes made by the old Yjs binding #3167:
splitChangedBlocksno longer splits a nesting change from the old binding, because the nesting rule is removed. Instead, the transformer shows the change in place. The legacy tests of fix(versioning): diff structural changes made by the old Yjs binding #3167 check this. They pass without changes.This PR does not change columns and tables. The transformer merges only
blockGroups insideblockContainer.Tests
The tests of this PR are now in #3090, in
nestingChanges.test.ts. The old name of this file wasmergeBlockGroups.test.ts. This PR no longer adds a test file.On #3090, 9 tests have the comment "To be fixed by #3168" and are marked
it.fails. This PR changes them to normal tests:nestingChanges.test.ts(7 tests):yDocToBlocksreads both children of that state.versionDiffAttribution.test.ts(2 cases): a block that is lost to cascading indents shows no author, for the two save orders.