Automattic / vip-block-data-api

WordPress plugin that provides an API to retrieve Gutenberg content as structured JSON.
http://wpvip.com
GNU General Public License v3.0
97 stars 7 forks source link

Add block binding support #74

Closed chriszarate closed 2 weeks ago

chriszarate commented 3 weeks ago

Description

This PR adds support for block bindings, introduced in WordPress 6.5.

The changes here are relatively small, although they are accompanied by some test improvements and cleanup, so the diff is a bit larger than it otherwise would be. Breakdown of commits:

New functionality

Test improvements

No test data was changed in this PR (only ordering).

Additional test cases

alecgeatches commented 2 weeks ago

@chriszarate The discussion in https://github.com/Automattic/vip-block-data-api/pull/74#discussion_r1731992594 made me think about a potential version bump to v2, as in vip-block-data-api/v2/.... We recently did a similar thing for the GraphQL endpoint with blockDataV2: https://github.com/Automattic/vip-block-data-api/pull/72.

Are there any changes in this PR or #75 that would justify a version upgrade? It seems like we're mostly adding more data to the response with the original binding data + actual computed binding results, which isn't necessarily a breaking change but could be. What do you think? This would entail some more work to duplicate some tests and separate a second parser class, but it may be an easier path and allow some other minor upgrades like we discussed.

chriszarate commented 2 weeks ago

Are there any changes in this PR or https://github.com/Automattic/vip-block-data-api/pull/75 that would justify a version upgrade?

There aren't any breaking changes to the response shape—no new properties or changes to existing ones. The functional changes (supporting block bindings and synced patterns) reflect the current approach and behavior of the plugin, and just enhance the existing response.

Anything can be a surprise of course, but let's use some hypotheticals to guide us. All of these assume we release these two PRs and don't bump to v2:

Block bindings

The effect of block bindings in this plugin is potentially updated string attribute values for specific core blocks when block bindings are in use. There is currently no UI for block bindings in the block editor and very little adoption, so anyone using them must write and deploy code to provide them. If a Block Data API user also uses block bindings, they are certainly aware that they are not supported—and probably frustrated.

  1. I am a Block Data API consumer and I don't use block bindings. Upgrading has no effect.
  2. I am a Block Data API consumer and I use block bindings. I created my own implementation to resolve the bindings using the returned metadata attributes. Upgrading has no effect since my implementation governs and overrides. I can remove my implementation at any time of my choosing.
  3. I am a Block Data API consumer and I use block bindings. The returned data is incorrect, but I have decided to use it anyway. Upgrading has an effect by providing the correct data, and I consider this a breaking change.

Scenarios 1 and 2 are not breaking changes. A breaking change via scenario 3 is possible, but I struggle to imagine a realistic example.

Synced patterns

As shown in the PR description, the effect of synced pattern support is a populated innerBlocks property for core/block blocks.

Synced patterns do have UI in the block editor, but Block Data API users are almost certainly aware that they are not supported.

  1. I am a Block Data API consumer and I don't use synced patterns. Upgrading has no effect.
  2. I am a Block Data API consumer and I use synced patterns. I created my own implementation to resolve the synced patterns using the returned ref attribute. Upgrading has no effect since my implementation governs and overrides. I can remove my implementation at any time of my choosing.
  3. I am a Block Data API consumer and I use synced patterns. My consuming code explicitly ignores core/block blocks since they have no content. Upgrading has no effect.
  4. I am a Block Data API consumer and I use synced patterns. My consuming code effectively ignore core/block blocks because they currently have no inner blocks. But if they did have inner blocks, my code would render them. Upgrading has an effect, and I am potentially caught off guard by content that is now rendered.

Scenarios 1–3 are not breaking changes. A breaking change via scenario 4 is possible but unlikely IMO.

In summary, we have some theoretically possible breaking changes for you to consider. If we do add a v2 namespace, we require Block Data API users who want to gain support for these emerging WordPress core features to (1) notice that these features are available (2) update their code to point to the new endpoint.

In addition, we perhaps set a precedent. When this plugin adds support for new core features that alter the content—but not the shape—of the response, are we obligated to add a new version namespace? Perhaps. With enhancements to block bindings and patterns coming, this question will probably come up again.

alecgeatches commented 2 weeks ago

@chriszarate Good points. I think we should stick with v1. In short, given that block bindings are inert with the current Block Data API and unlikely to be relied upon for functionality, this is an improvement that opens new patterns up to future API consumers without disruption.