Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds experimental blocks flag #64121

Merged
merged 1 commit into from
Aug 1, 2024
Merged

Conversation

creativecoder
Copy link
Contributor

@creativecoder creativecoder commented Jul 30, 2024

What?

Adds a new flag on the Gutenberg > Experiments wp-admin page for experimental blocks.

I plan to use this for #63689, but created a separate PR here so it can be used by others, as well, before that PR is merged.

Why?

Blocks in development may not be ready for general use. By having users opt in to an experiment to test these blocks, we can iterate without worrying about writing multiple block deprecations.

Once a block is stable, it can be promoted from the experiment to being included by default in block-library.

How?

Adds gutenberg-block-experiments setting included on the gutenberg-experiments wp-admin page.

To add a new block to the experiment, conditionally add it in https://github.com/WordPress/gutenberg/blob/trunk/packages/block-library/src/index.js using window?.__experimentalEnableBlockExperiments (see the existing form/input blocks in that file as an example).

Testing Instructions

  • Open /wp-admin/admin.php?page=gutenberg-experiments and see an option for "Experimental blocks"
  • Check the box and reload the page to turn it on (no additional blocks are included in this PR)
  • Verify that the existing "Form and input blocks" experiment still works, that the form block is available in the editor when the experiment is on

Testing Instructions for Keyboard

n/a

Screenshots or screencast

image

Copy link

github-actions bot commented Jul 30, 2024

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: creativecoder <grantmkin@git.wordpress.org>
Co-authored-by: mikachan <mikachan@git.wordpress.org>
Co-authored-by: vcanales <vcanales@git.wordpress.org>
Co-authored-by: jffng <jffng@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>

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

@creativecoder creativecoder added the [Type] Experimental Experimental feature or API. label Jul 30, 2024
@creativecoder creativecoder added the No Core Sync Required Indicates that any changes do not need to be synced to WordPress Core label Jul 30, 2024
Copy link
Member

@mikachan mikachan left a comment

Choose a reason for hiding this comment

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

Thanks @creativecoder, all the code changes look good to me.

  • I opened /wp-admin/admin.php?page=gutenberg-experiments and I see an option for "Experimental blocks" ✅
  • Checked the box and reloaded the page to turn it on ✅
  • Checked that the existing "Form and input blocks" experiment still works when that experiment is turned on ✅

*/
function gutenberg_enable_form_input_blocks() {
_deprecated_function( __FUNCTION__, 'Gutenberg 19.0.0', 'gutenberg_enable_block_experiments' );
Copy link
Member

Choose a reason for hiding this comment

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

Given that we're folding the form input blocks into the block experiments, is the related settings field still necessary?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this function itself would be called by anything other than the Gutenberg plugin. How about removing this function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Given that we're folding the form input blocks into the block experiments, is the related settings field still necessary?

I don't know the history or status of the form blocks experiment, so I want keep it separate for now and avoid needing to migrate the settings.

I don't think this function itself would be called by anything other than the Gutenberg plugin. How about removing this function?

Good point, though I prefer to be cautious for now. I'll set a reminder to come back and remove it in a couple of releases.

Copy link
Contributor

@jffng jffng left a comment

Choose a reason for hiding this comment

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

Works as described, I think it's okay to keep a separate option for the form blocks vs general block experiments.

@creativecoder creativecoder merged commit ff59ffa into trunk Aug 1, 2024
76 of 79 checks passed
@creativecoder creativecoder deleted the add/block-experiments-flag branch August 1, 2024 01:30
@github-actions github-actions bot added this to the Gutenberg 19.0 milestone Aug 1, 2024
@creativecoder creativecoder mentioned this pull request Aug 5, 2024
@ntsekouras
Copy link
Contributor

I'm not sure why would we need an experimental flag for blocks.. I don't know the full story about the form input block, but in general experiments are for way bigger features that need many iterations with lots of uncertainty (see real time collaboration, etc..).

A single block is usually well scoped and when it's quite close to land, but we're not sure if it's going to be stable for a WP release, we have the __experimental flag in block's metadata (block.json). In all other cases, the block should be tested in the respective PR and not add extra unfinished code in the repo.

@t-hamano
Copy link
Contributor

t-hamano commented Aug 7, 2024

Maybe we need to clarify the rules and definitions of blocks marked with "__experimental": true. For example, whether destructive changes should be allowed or whether a block deprecation should be added.

Until then, should we revert this PR and prevent it from being shipped in Gutenberg 19.0?

By the way, I feel that Form blocks are still unstable even on the Gutenberg plugin and need more iterations. That may be one of the reasons why there is an explicit option to enable these blocks.

@ntsekouras
Copy link
Contributor

ntsekouras commented Aug 7, 2024

Maybe we need to clarify the rules and definitions of blocks marked with "__experimental": true. For example, whether destructive changes should be allowed or whether a block deprecation should be added.

That's a good point! Regarding an experimental block's iterations, I think they should be mostly about getting it out of that experimental state.

For now, I lean towards reverting this and we should also evaluate the form input blocks.. I have no experience about that feature, but is it being iterated or used?

@t-hamano
Copy link
Contributor

t-hamano commented Aug 7, 2024

Until the positioning of the experimental block is clarified, I also think it's ok to revert this PR. @creativecoder What do you think?

As for Form blocks, I don't think they are being actively developed or iterated on at this time. However, it seems that the new TT5 proposes to include Form blocks. Therefore, I agree that the Form blocks need to be evaluated in the near term.

@t-hamano
Copy link
Contributor

t-hamano commented Aug 7, 2024

Just to be safe, I have prepared PR #64336 to revert this.

@creativecoder
Copy link
Contributor Author

I think the reason for having a block experiments flag boils down to

  • More quickly and confidently iterating on new blocks with smaller and easier to review PRs.
  • Making it clear to Gutenberg plugin users when a block is still in early development and may have breaking changes, so they can decide if they want to use it or not.

With the tabs and accordion blocks in progress, there are subtle but important interactions related to inner blocks, and we may need to iterate on the underlying block APIs and/or try different configurations of inner blocks to get them right.

For more complex blocks like these that use multiple inner blocks to construct their content, getting the structure stable/near final in one PR is a large burden, both to develop and even more so to review the PR. I think having another step in the block development process that allows marking blocks as in an early, less stable stage will let us iterate more quickly and effectively.

In practice, the __experimental property in block.json seems to be an indicator if a block is ready for core, or not, and there's no visible indication to users that those blocks are experimental. While silent block validation errors that recover automatically are not really a problem for blocks in development, changing these blocks in a way that triggers the block recovery overlay and potentially loses content is a poor experience for users of the Gutenberg plugin. Having an opt-in to use them gives a very clear warning and allows us more flexibility in the early iterations of new blocks, when needed.

If we're going to revert this PR and rely on the __experimental block.json property for these purposes, I would like there to be a very clear visual indicator that these blocks are potentially not stable, to deter users who aren't ready for the potential breakage. And perhaps we should uncheck these in the editor preferences for blocks, by default, to hide them from the inserter, so there's still an opt-in step for users.

@t-hamano
Copy link
Contributor

t-hamano commented Aug 8, 2024

cc @WordPress/gutenberg-core

We are discussing whether to revert the experimental block flag added in this PR.

@talldan
Copy link
Contributor

talldan commented Aug 8, 2024

Without the experiment flag, would the blocks would be available to anyone running the Gutenberg plugin in production?

If so, it sounds risky if the blocks are only in an early iteration.

@andrewserong
Copy link
Contributor

If so, it sounds risky if the blocks are only in an early iteration.

I agree, given that there are lots of sites that run Gutenberg in production. I also quite like that experimental flags allow features to be developed more iteratively and place less pressure on larger PRs that can be harder to review and keep up-to-date. So, in principle, having an experimental flag for blocks currently in development (but not ready for production) sounds pretty good to me, but that's just my 2 cents!

@aaronrobertshaw
Copy link
Contributor

I can see benefits in both approaches.

Mandating that only polished blocks ready for release are made available avoids a lot of backward compatibility issues and excessive deprecations that need to hang around forever etc. On the flip side, I'm sympathetic to being able to iterate quickly and get broader testing, feedback etc.

As we've seen with other experimental APIs, once it's available, we've been obligated to maintain BC, despite how well that experimental nature may have been communicated.

Even with an experimental flag, I'm not sure that means we can ignore the need to write deprecations. If a block is changing that much, that quickly, that deprecations are a problem, that's a sign it wasn't ready for public exposure in Gutenberg, behind an experiment flag or not.

If pushed to add a vote here, I'd lean towards keeping the experimental flag, with the expectation that we are judicious about what new blocks qualify to be exposed as experimental, and commit to some level of backward compatibility for them.

@mikachan
Copy link
Member

mikachan commented Aug 8, 2024

I'd also like to add a vote for keeping the experimental blocks flag:

  • To allow for quicker iterations of these blocks
  • To avoid large PRs as much as possible
  • To make it more clear to Gutenberg users that they are opting into experimental blocks

However, I understand the desire to commit to an agreed level of backward compatibility and assess which new blocks qualify as experimental as they are introduced.

@t-hamano
Copy link
Contributor

Thanks for all the feedback. It seems like it would be better to keep the experimental flag added in this PR.

An important thing to consider is whether to add a deprecation if the HTML of the experimentally flagged block changes after it ships. Although the flag description says that "validation errors may occur," I think it's a good idea to add the deprecation if the work of adding it is minor.

However, it is important to note that experimental blocks may not generate fixtures correctly. We may need to update your fixture test logic as done in #56719.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No Core Sync Required Indicates that any changes do not need to be synced to WordPress Core [Type] Experimental Experimental feature or API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants