wp-graphql-content-blocks icon indicating copy to clipboard operation
wp-graphql-content-blocks copied to clipboard

Published 20 hours ago • verified publisher icon wpengine

Fix revert coreimage block width type

Open theodesp opened this issue 9 months ago • 8 comments
trafficstars

Reverts https://github.com/wpengine/wp-graphql-content-blocks/pull/130

theodesp avatar Feb 06 '25 10:02 theodesp

🦋 Changeset detected

Latest commit: 18d9e8b3a81b6a2af0a1fe70c12fd0771946ac04

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@wpengine/wp-graphql-content-blocks Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar Feb 06 '25 10:02 changeset-bot[bot]

Latest Image block.json width is of type "string" so I don' think this is a breaking change

https://github.com/WordPress/WordPress/blob/master/wp-includes/blocks/image/block.json#L74 https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/blocks/image/block.json#L74

theodesp avatar Feb 06 '25 11:02 theodesp

@theodesp LTGM

colinmurphy avatar Feb 07 '25 15:02 colinmurphy

Latest Image block.json width is of type "string" so I don' think this is a breaking change

https://github.com/WordPress/WordPress/blob/master/wp-includes/blocks/image/block.json#L74 https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/blocks/image/block.json#L74

Yes it is...

image

justlevine avatar Feb 09 '25 21:02 justlevine

Latest Image block.json width is of type "string" so I don' think this is a breaking change https://github.com/WordPress/WordPress/blob/master/wp-includes/blocks/image/block.json#L74 https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/blocks/image/block.json#L74

Yes it is...

image

Yes. I've seen this. The thing is, I don't see where this width: "number" type is coming from? All the references of the image width in wordpress latest or develop are "width: "string"

Also after trying to query the Schema with this PR with WP v6.7.1, I can see the type is string:

Screenshot 2025-02-10 at 12 00 04

Is it the schema.graphql out of date maybe?

theodesp avatar Feb 10 '25 12:02 theodesp

Latest Image block.json width is of type "string" so I don' think this is a breaking change https://github.com/WordPress/WordPress/blob/master/wp-includes/blocks/image/block.json#L74 https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/blocks/image/block.json#L74

Yes it is... image

Yes. I've seen this. The thing is, I don't see where this width: "number" type is coming from? All the references of the image width in wordpress latest or develop are "width: "string"

Also after trying to query the Schema with this PR with WP v6.7.1, I can see the type is string:

Screenshot 2025-02-10 at 12 00 04 Is it the `schema.graphql` out of date maybe?

My understanding of your CI, is that you're defining the WordPress version in the .env.dist instead of in the CI itself, so makes sense it would get out dated, but that's not the issue here.

The issue is that the minimum WP requirement isn't 6.7, and as such removing this is a breaking change for users on earlier versions of WordPress.

(Personally, I'd much rather break this just once as part of #136 / interfaces that stabilize the schema independently underlying changes to WordPress )

justlevine avatar Feb 10 '25 12:02 justlevine

@justlevine Thanks I will just increase the major version with a note.

theodesp avatar Feb 10 '25 15:02 theodesp

@justlevine @theodesp

I was just checking what is the status of this PR? Should this be addressed in #136 and closed?

colinmurphy avatar Oct 10 '25 10:10 colinmurphy