Skip to content

Fix: properly merged schema during mergeSchemas part in getBlockContentSchemaFromTransforms - #70615

Merged
sirreal merged 17 commits into
WordPress:trunkfrom
USERSATOSHI:fix/mergeschema-array-to-object
May 14, 2026
Merged

Fix: properly merged schema during mergeSchemas part in getBlockContentSchemaFromTransforms #70615
sirreal merged 17 commits into
WordPress:trunkfrom
USERSATOSHI:fix/mergeschema-array-to-object

Conversation

@USERSATOSHI

@USERSATOSHI USERSATOSHI commented Jul 4, 2025

Copy link
Copy Markdown
Contributor

What?

Closes #70612

This PR fixes the bug where merging schemas causes top level classes property to convert from array to object with indices as keys.

Why?

As the classes gets converted to Object the cleanNodeList function fails when it tries to map the classes property.
Error at this line

How?

The Solution checks whether b[key] is an array or not, if it is an array , set a[key] to be copy of b[key] else the original object shallow clone.

Testing Instructions

Follow the instructions of #70612

Screenshots or screencast

Before

Screen.Recording.2025-07-04.at.2.08.59.PM.mov

Results

Screen.Recording.2025-07-04.at.2.35.36.PM.mov

Block Transform Code

Screen.Recording.2025-07-04.at.2.37.14.PM.mov

@USERSATOSHI
USERSATOSHI marked this pull request as draft July 4, 2025 09:10
@github-actions

github-actions Bot commented Jul 4, 2025

Copy link
Copy Markdown

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: USERSATOSHI <tusharbharti@git.wordpress.org>
Co-authored-by: sirreal <jonsurrell@git.wordpress.org>
Co-authored-by: Adi-ty <iamadisingh@git.wordpress.org>
Co-authored-by: aaronjorbin <jorbin@git.wordpress.org>
Co-authored-by: desrosj <desrosj@git.wordpress.org>
Co-authored-by: getdave <get_dave@git.wordpress.org>
Co-authored-by: SirLouen <sirlouen@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@USERSATOSHI
USERSATOSHI marked this pull request as ready for review July 4, 2025 09:45
@t-hamano t-hamano added [Type] Bug An existing feature does not function as intended [Package] Blocks /packages/blocks labels Jul 4, 2025
@SirLouen

Copy link
Copy Markdown
Member

@getdave do you think this is ready to be shipped?

@sirreal

sirreal commented Nov 18, 2025

Copy link
Copy Markdown
Member

Thanks for working on this, I'd love to see this fix land.

I believe there's a remaining issue here that's unaddressed. If classes appears under children it will not merge. Here's a change to tests that captures it:

diff --git c/packages/blocks/src/api/raw-handling/test/utils.js i/packages/blocks/src/api/raw-handling/test/utils.js
index fc532c7400..ae1384dad0 100644
--- c/packages/blocks/src/api/raw-handling/test/utils.js
+++ i/packages/blocks/src/api/raw-handling/test/utils.js
@@ -128,7 +128,9 @@ describe( 'getBlockContentSchema', () => {
 						children: {
 							sub: {},
 							sup: {},
-							strong: {},
+							strong: {
+								classes: [ 'test-class' ],
+							},
 						},
 					},
 				},
@@ -146,7 +148,9 @@ describe( 'getBlockContentSchema', () => {
 		const output = {
 			pre: {
 				children: {
-					strong: {},
+					strong: {
+						classes: [ 'test-class' ],
+					},
 					em: {},
 					sub: {},
 					sup: {},

I discovered this when I tried to capture a target class below the root, but it was always stripped in my transform:

-<pre><code class="TARGET_CLASS"></code></pre>
+<pre><code></code></pre>

I've confirmed that issue remains on this branch.

@sirreal sirreal left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A side note: I really miss types when trying to understand all of this logic. We could consider migrating this file to TypeScript as a first step. It could be very beneficial when understanding the recursive data structure.

Comment thread packages/blocks/src/api/raw-handling/utils.js Outdated
@SirLouen
SirLouen self-requested a review December 29, 2025 21:59
@USERSATOSHI

Copy link
Copy Markdown
Contributor Author

@SirLouen @sirreal Hi, I’ve made the updates based on your suggestions.
Apologies for the delay. I’ve had limited bandwidth over the past few months.

@USERSATOSHI
USERSATOSHI requested a review from sirreal April 9, 2026 07:39
@USERSATOSHI

Copy link
Copy Markdown
Contributor Author

I think this PR is ready for merge now, please let me know if anything else is needed.

@sirreal

sirreal commented May 14, 2026

Copy link
Copy Markdown
Member

This seems ready. @USERSATOSHI will you add a bug fix changelog entry to the package?

This comment was marked as resolved.

@sirreal sirreal left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pre-approving. I want to wait for the changelog.

Thanks for this fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Good First Review A PR that's suitable for someone looking to contribute for the first time by reviewing code [Package] Block editor /packages/block-editor [Package] Blocks /packages/blocks [Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] classes property in transforms breaks if it is not defined in the first block schema

10 participants

Sponsor
SponsoredKunjungi sekarang
Promo