CoreImageAttributes.width is type String, but should be a Float

[justlevine]

Noticing that the width field on CoreImageAttributes is type String when it should be of type Float:

I'm not 💯sure how this plugin is automating the {BlockType}Attributes field registration, otherwise I'd open a PR myself.

[theodesp]

Actually this causes an issue if you turn this into a Float when you combine this with CoreColumns width attribute which is of type =String.

Take a look at this testcase:

https://github.com/wpengine/wp-graphql-content-blocks/blob/main/tests/unit/blocks/CoreImageTest.php#L38

I think my fix was premature. Any ideas or thoughts on how to support this better is welcome.

[justlevine]

I think my fix was premature. Any ideas or thoughts on how to support this better is welcome.

From a search of WordPress's codebase, it seems, the Column and Spacer blocks are the only one that use a string width, while the rest use a number. As such it should be treated as outlier behavior.

As mentioned, I'm unclear where this is being handled in the code and I don't see a particular PR that made this fix, but my suggestion would be to parse the width into a width: Float, widthUnit: CSSUnitEnum. This is the pattern used by the Search block, and will likely be adopted by the Column and Spacer blocks in a future release, so not just would this be a semantically ideal pattern, it's forward compatible.

[theodesp]

Hey @justlevine. After a conversation with @jasonbahl, we found that this is indeed inconsistent. Could you open a PR for this? There is a test case that needs to addressed here when you revert this back to Float type. So in that case the test case is expected to fail with the error mentioned above.

If you haven't got availability, I will create a ticket and follow up. Thanks.

[justlevine]

I have some availability over the weekend but I'm not entirely sure I understand this plugin's lifecycle. If you link me to the parts that are currently responsible for fetching/parsing the block attributes, I'd be happy to take a stab at it 😁

[theodesp]

Sure. It's this line need to be removed:

https://github.com/wpengine/wp-graphql-content-blocks/blob/main/includes/Blocks/CoreImage.php#L32

It will default back to Float

[justlevine]

It's the attribute fetching/parsing_ that I'm looking for. I need that in order to take the original string $attributes['width'] and split it into (float) width and (enum) cssUnit. It's either in the abstract block class or the BlocksResolver, I think, but that's where I start to get a bit lost.

[theodesp]

You can still specify both explicitly:

'width'        => array( 'type' => 'float' ),
'widthUnit' =>  array( 'type' => 'string' ),

The type parsing is located here:
https://github.com/wpengine/wp-graphql-content-blocks/blob/main/includes/Blocks/Block.php#L152-L168

However we don't handle enum types yet.

https://developer.wordpress.org/block-editor/reference-guides/block-api/block-attributes/#enum-validation
https://www.apollographql.com/tutorials/side-quest-intermediate-schema-design/02-the-enum-type

Implementing this would be outside the scope of this ticket.

[LarsEjaas]

This is a major pain when working with something like GraphQL Codegen for TypeScript types.

I see this problem in several places throughout the attributes for the Core blocks:

CoreAudioAttributes on CoreAudio:

• id

CoreColumnAttributes on CoreColumn

• width

CoreCoverAttributes on CoreCover

• id

CoreFileAttributes on CoreFile

• id

CoreImageAttributes on CoreImage

• id
• width

CoreNavigationLinkAttributes on CoreNavigationLink

• id

CoreNavigationSubmenuAttributes on CoreNavigationSubmenu

• id

CorePageListItemAttributes on CorePageListItem

• id

CorePostFeaturedImageAttributes on CorePostFeaturedImage

• width
• height

CoreSiteLogoAttributes on CoreSiteLogo

• width

CoreSocialLinksAttributes onCoreSocialLinks

• size

CoreSpacerAttributes on CoreSpacer

• height
• width

CoreTextColumnsAttributes on CoreTextColumns

• width
• content

CoreVideoAttributes on CoreVideo

• id

[justlevine]

I see this problem in several places throughout the attributes for the Core blocks:

@LarsEjaas the root cause /solution for all these is likely similar. Apart from witdh, what are the types currently being returned and what should they be?

[LarsEjaas]

I see this problem in several places throughout the attributes for the Core blocks:

@LarsEjaas the root cause /solution for all these is likely similar. Apart from witdh, what are the types currently being returned and what should they be?

I am not sure what they should be actually. But for me, the main issue is that the types are different depending on where I make a query for the blocks.
I can make a query for the EditorBlock on Post or Page. So my issue here is not so much WHAT the types should be - I just want them to be the same so I can reuse GraphQL fragments for the different Core blocks everywhere.

Example:
Right now for CoreSpacer I would need to have a different CoreSpacerAttributes fragment for pages and posts - that is a pain - I just want the types to be identical. In the headless frontend I would also have to account for the difference in types in the components.

[theodesp]

I noticed that this would happen on each fragment that contains the same attribute name but different type. For example:

fragment CorePostFeaturedImageFragment on CorePostFeaturedImage {
  attributes {
    width
  }
}
fragment CoreSiteLogoFragment on CoreSiteLogo {
  attributes {
    width
  }
}
{
  posts {
    nodes {
      editorBlocks {
        name
      	__typename
        ...CorePostFeaturedImageFragment
        ...CoreSiteLogoFragment
      }
    }
  }
}

 "message": "Fields \"attributes\" conflict because subfields \"width\" conflict because they return conflicting types String and Float. Use different aliases on the fields to fetch both if this was intentional.", "message": "Fields \"attributes\" conflict because subfields \"width\" conflict because they return conflicting types String and Float. Use different aliases on the fields to fetch both if this was intentional.",

One potential solution is to use unique aliases for each attributes field. This would avoid the conflicts.

fragment CorePostFeaturedImageFragment on CorePostFeaturedImage {
  corePostAttributes: attributes {
    width
  }
}
fragment CoreSiteLogoFragment on CoreSiteLogo {
  coreSiteLogoAttributes: attributes {
    width
  }
}
{
  posts {
    nodes {
      editorBlocks {
        name
      	__typename
        ...CorePostFeaturedImageFragment
        ...CoreSiteLogoFragment
      }
    }
  }
}