Merged
Conversation
|
Size Change: +376 B (0%) Total Size: 7.73 MB
ℹ️ View Unchanged
|
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
alecgeatches
added
[Type] Task
chriszarate
added
the
[Feature] Real-time Collaboration
|
Flaky tests detected in cf895af. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/23817668983
|
alecgeatches
removed
the
[Type] Bug
ingeniumed
reviewed
Mar 31, 2026
Contributor
ingeniumed
left a comment
ingeniumed
left a comment
There was a problem hiding this comment.
It worked well in my testing. Considering how targeted each PR is getting, I noted down gaps in the tests that'd be useful to cover. That way the unit tests cover any changes that are accidentally made. Don't mind whether the coverage happens via unit or e2e tests - whatever's easier
| } | ||
|
|
||
| // Delete properties absent from the incoming object. | ||
| for ( const key of yMap.keys() ) { |
Contributor
There was a problem hiding this comment.
Tests: Don't think this is covered atm.
| query, | ||
| cursorPosition | ||
| ); | ||
| } else { |
Contributor
There was a problem hiding this comment.
Tests: Don't believe this is covered rn
| query: Record< string, BlockAttributeSchema >, | ||
| obj: unknown | ||
| ): Y.Map< unknown > { | ||
| if ( ! isRecord( obj ) ) { |
Contributor
There was a problem hiding this comment.
Minor: Is it valuable to cover this path in the tests?
| currentVal instanceof Y.Array | ||
| ) { | ||
| mergeYArray( currentVal, newVal, schema, cursorPosition ); | ||
| } else if ( |
Contributor
There was a problem hiding this comment.
Tests: Would cover this path, as I don't think its covered rn
3 tasks
alecgeatches
changed the title
Contributor
Author
|
@ingeniumed Added a few more test cases in cf895af for deleted properties, type differences/migration, and cases when a top-level query represents an object. |
ingeniumed
approved these changes
Mar 31, 2026
Contributor
Author
|
I'm merging this now. As mentioned above, this is not intended for 7.0 backport at this time. |
ciampo
pushed a commit
that referenced
this pull request
Apr 3, 2026
* Add tests for complex schema merging * Add schema-aware merging and creation functions for Y data * Add extra guard if there's an unexpected type while merging * Use Object.hasOwn instead of .hasOwnProperty * Add nested plain-text attribute tests * Skipp fastDeepEqual when comparing raw objects to Y types * Add more unit tests around map building edge-cases Co-authored-by: alecgeatches <alecgeatches@git.wordpress.org> Co-authored-by: ingeniumed <ingeniumed@git.wordpress.org>
alecgeatches
added a commit
that referenced
this pull request
Apr 6, 2026
adamsilverstein
pushed a commit
that referenced
this pull request
Apr 7, 2026
* Add tests for complex schema merging * Add schema-aware merging and creation functions for Y data * Add extra guard if there's an unexpected type while merging * Use Object.hasOwn instead of .hasOwnProperty * Add nested plain-text attribute tests * Skipp fastDeepEqual when comparing raw objects to Y types * Add more unit tests around map building edge-cases Co-authored-by: alecgeatches <alecgeatches@git.wordpress.org> Co-authored-by: ingeniumed <ingeniumed@git.wordpress.org>
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
3 participants
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.
What?
This is a bit risky of a change for late-stage 7.0, so it's intended for post-7.0 merging into Gutenberg. Part of the overall work in #76915.
Closes #76908:
typing-in-the-same-table.mov
Users can not add simultaneous updates to separate cells of the same table
This PR changes how query block attributes like a table
bodyare stored in the CRDT document. This gives each table cell its own independentY.Text, so two users editing different cells no longer overwrite each other:simultaneous-typing.mov
Demonstration of correct cell data merging in
core/tableWhy?
Previously, the entire table
bodywas stored as a single opaque value in the block'sY.Map:Any cell edit (e.g. modifying "A1") replaced the whole
bodyarray. With slower HTTP polling, two users typing in different cells of the same table can easily overlap on updates, and Yjs would silently discard one user's edit asbodyis a big non-Y-type blob.Note that this is related but different from #76597. That PR fixed nested
RichTextDatainstances inside table cells that were accidentally being serialized to empty data. This PR improves in a different direction by restructuring the whole table layout in the CRDT to nested Yjs types.How?
Previously, we only cared about top-level block
RichTextattributes intoY.Text, but other attribute types would get serialized into the CRDT as-is. The change uses the existingqueryproperty in block attribute schemas to recursively create nested Yjs types. For a tablebodyattribute, the new structure looks like this:After (deep Yjs types):
With this structure, User A editing cell A1 and User B editing cell B2 write to different Y.Text instances, and Yjs can merge them correctly.
To get started reading the code, the best entry point is
updateYBlockAttribute(), which calls intomergeYValue(). This function handles merging or creating new Y values. PreviouslyupdateYBlockAttribute()only reacted to top-levelY.Textvalues, but nowmergeYValue()recurses into schema definitions to create the right Y types for internal arrays and objects.Any block attribute with
type: 'array'ortype: 'object'and aqueryschema definition gets nested Y types automatically. This change also affectscore/galleryimages, though table is the much more important case.For structural changes (adding/removing rows), the Y.Array is rebuilt from scratch as a fallback. This means a concurrent cell edit during a structural change could still conflict, but these are infrequent and should be a reasonable fallback.
This is also related to #76812, because we can't render cursors without
Y.Textinstances for table cells. The changes in here should allow for fixing #76812.Testing Instructions
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Used for: Implementation of nested Y.js types for array/object block attributes