RTC: Fix useInnerBlockTemplateSync race condition#78696
Closed
alecgeatches wants to merge 2 commits into
Closed
Conversation
|
Size Change: +650 B (+0.01%) Total Size: 8.18 MB 📦 View Changed
ℹ️ View Unchanged
|
Contributor
Author
|
Closing in favor of #79021. |
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.
Edit: I don't really like this approach, and ideally the sender using
queueMicrotaskcan be in charge of keeping the RTC sync updates together. I'll keep this draft open while I try something new.What?
Presently in real-time collaboration over WebSockets, inserting templated blocks (like
core/list) can result in multiple inner block insertions:list-item-doubling.mov
Notice that two inner
core/list-itemblocks were inserted. This scales with the number of users collaborating on a post, so 10 users could see 10 empty list items inserted with one outercore/listblock.This PR prevents remotely received blocks from running local inner-block template insertion when they first appear in the editor. In practice, this fixes the RTC case above and any other block that utilizes
useInnerBlocksProps()with a template:list-item-fixed.mov
Why?
core/listblock is added with an innerBlocks template.queueMicrotask(async delay), the block detects it has no template yet, and inserts the template block into the list block.On WebSockets in production, the first action syncs to other users pretty much immediately, and then all users have an empty
core/listblock. They then all run the same template insertion logic and add a fresh list item to the list. This is less common in HTTP polling because operations are sent in 500ms merged updates, which typically contain both the outer and inner list blocks.In general, when a user inserts a block with a default inner-block template, the parent block can sync to other peers before the queued template-fill microtask runs. Each peer then sees an empty templated block and fills it locally, so the template child is duplicated across collaborators. This sort of deferred behavior is very difficult for RTC, since operations aren't rolled into the same transaction and can be spread across an arbitrary amount of time.
In this PR, we want the originator to remain responsible for the template fill, while remote peers wait for that canonical update instead of broadcasting their own copy.
How?
In short, we tag remote CRDT updates (
__unstableIsRemoteSynced) on remotely-synced blocks. When we hit the inner template insertion logic, we check to see if the outer block has a remote origin. We then run a template insertion only when the local editor is the originator. This takes some plumbing.Inner Blocks Remote-Sync Order
There are a lot of changed files in this PR, but many of them are just plumbing for the new flag. Here's how it's piped down from initial remote sync into the inner
queueMicrotaskcall:Usage
useEntityBlockEditorremains the lower-level tuple hook for callers that only need part of the block editor binding, such as readingblocksfor a preview or usingonChangefor a one-off template selection. Its tuple now has a backward-compatible fourthoptionsitem.__unstableUseEntityBlockEditorPropsis a small wrapper around it for controlled block-editor surfaces: it returns{ blocks, value, onInput, onChange, __unstableIsRemoteSynced }, whereblocksis the readable block array andvalueis the same array under the prop name expected byBlockEditorProvider,InnerBlocks, anduseInnerBlocksProps.Both exist to keep the existing tuple API backwards-compatible while making the safer path obvious for entity-backed block editor bindings. In general, use the props helper when connecting entity blocks to an editor surface, and use
useEntityBlockEditordirectly when a component only needs to inspect blocks or call one of the tuple callbacks.Tradeoffs
Remote-sync block marks are intentionally single-use, because the distinction doesn't matter after template insertion. Once
useInnerBlockTemplateSyncsees the mark, it clears it.useBlockSynccurrently marks the whole incoming controlled subtree instead of diffing only newly added blocks; that keeps the implementation simple and matches the existing whole-tree reset path, but it does mean remote updates do a full clientId walk. Core-data also keeps remote block edit source metadata until a later local block edit or record receive; the runtime guard only treats it as active while the current blocks reference is the remote edit, so stale metadata should be inert.This intentionally does not try to synthesize a missing template child on remote peers if the originator disappears before its queued template-fill microtask runs. Without a larger coordination mechanism, peers cannot reliably distinguish "the originator's fill is still in flight" from "the originator will never send it"; this PR chooses to prevent duplicate children rather than adding a fallback that could reintroduce the fan-out race.
Testing Instructions
Enable RTC via Settings -> Writing -> "Enable real-time collaboration" checkbox.
Open the same post in two browser sessions as different users.
IMPORTANT: Use two separate browsers, not just two tabs, as
queueMicrotask()seems to avoid race conditions between two tabs in the same browser.In one session, insert a List block.
Confirm the List block contains a single List Item in both sessions.
Repeat with three or more connected sessions and confirm only one List Item is created.
In one session, insert another block that uses a default inner-block template, such as Columns, and confirm peers do not create duplicate template children.
Confirm normal local insertion still works when RTC is disabled or only one editor session is open.
Run
npm run test:unit -- packages/block-editor/src/components/inner-blocks/test/use-inner-block-template-sync.js.Use of AI Tools
AI assistance: Yes
Tool(s): Codex + Claude
Used for: Investigate the RTC race, implement the remote-synced guard, make PR description additions.