Add Deprecated Function for Existing Blocks

[gregrickaby]

After learning that once a block is saved, you can't go back and update the markup (without breaking the block)

Please convert all the save methods from JSX to a render.php file inside each respective block/component's directory.

• call-to-action
• default
• github-gist
• hero
• related-posts
• two-column-block

Setup a branch feature/#24-block-save-to-render-php for working.

Edit: After some more digging by @efuller, we realized that there is a not-so-well documented deprecated function for rendering in JSX which allows you to add new fields, adjust the output, etc. without breaking the block on the backend. So that's nice! Updated the task name and directive here to add that in where we're able (i.e., blocks rendering via JSX and not PHP).

[jomurgel]

Am starting on two-column-block and working my way up.

[efuller]

Starting on call-to-action. I'll work top to bottom.

[efuller]

@jomurgel and I worked at this for a good bit today.

Right off the start, we ran into an issue where the <RichText> component was not saving attributes once the save() method was nulled out and a PHP render function was put into place.

If we changed out the <RichText> with a <PlainText> component, then everything seemed to work as expected.

I can verify that the attributes are getting changed when setAttributes is being called, but I'm not 100% sure they are being saved.

This could very well be a bug in the <RichText> component, but will need to investigate further and search the Gutenberg repo issues for more information.

[jomurgel]

It would appear that we've run into an issue that doesn't appear to be documented anywhere. Perhaps something to open a ticket for on the Gutenberg repo.

When the RichText component is in use (all of our blocks, save the Gist block) it is required that we utilize the save function and output our markup there. If we set save: () => null and try to use the render_callback function we find that only attributes that are strings (and not arrays) are pushed over to $attributes on the php side..

setAttribute function fails. Data is available in the editor during and after updated, but is not sent to the server side or available on page refresh.

The MediaUpload component has a simpliar issue, though different. The data saves and is pushed to the front end (available inside the $attributes array), but is then broken on the editor side, even though the imgID attribute has been saved. In this case imgURL and ImgAlt no longer populate to actually render the image inside the editor on page refresh.

My best guess is that it has something to do with the fact that those elements are arrays instead of strings, but this is conjecture at best.

Further digging is requred, but at this moment I'm marking the tasks as blocked until we can figure out a work around, fix, or Gutenberg issues an update.

[jomurgel]

Spent several hours trying to figure out what was going on here, or find a solution. I did not.

However, I did find that any component that returns an array to attributes fails if we're using the render_callback function. I haven't found any documentation about the specifics around this, but humanmade's editable HTML component docs shed a little light on this. They have a workaround passing HTML to string instead of returning an array (of React components).

I don't think this is an ideal solution since we should be getting our $attributes to deal with the output in the PHP, but for now, this might be the best solution available at the moment.

https://github.com/humanmade/hm-gutenberg-tools/wiki/Editable-HTML

Will continue to debug.

[efuller]

@jomurgel and I did more digging on this issue yesterday.

As a result, there are a few items I can confirm:

1. Components on the backend that use <RichText> do rely on a save function being defined. If one isn't defined, no content will be displayed for it. I also learned that if the save function is set to be a new instance of a Component, that components render function will be ported over to be the save function.
2. Jo and I were working to try and pin-point the exact time the attributes JSON is being stripped out of the_content, but we were unsuccessful at this. We were able to pin down between what actions it occurs, but not able to identify.

I feel like there is at the least a workaround we can come up with, it's just going to take more testing and digging in.

[efuller]

Gutenberg Rendering Issue

The main issue here, or at least what has prompted this discussion, is that when a block is registered in Gutenberg a save function is implemented.

Now let's say you add this block to a page in the WP admin and save it. Later, you decide you want to change the markup of the block so you go and edit the save function. When you return to the editor, you will get presented with this error:

The Initial Fix

After discussion on our weekly call, it was decided that Jo and I would go through each block and setup a server-side render function using PHP and register_block_type;

Using this method would eliminate the need for a save function on the client side and if markup changed inside the PHP block, all the things wouldn't be broken.

Something like this:

/**
 * Render block: Related Posts.
 *
 * @param array $attributes The attributes passed in from the Related Post block settings.
 * @return string The block markup.
 *
 * @since NEXT
 */
function render_block( $attributes ) {

	ob_start(); ?>

	<!-- wp:wds/related-posts -->
	<?php \WDS_Gutenberg\Src\Block_Options\display_block_options( $attributes ); ?>

		<?php \WDS_Gutenberg\Src\Component\display_block_title( $attributes ); ?>

	</section>
	<!-- /wp:wds/related-posts -->
	<?php

	return ob_get_clean();
}
register_block_type( 'wds/related-posts', [ 'render_callback' => __NAMESPACE__ . '\\render_block' ] );

Fix One Bug, Find Another

When using PHP to register and render a block, that block is passed an $attributes parameter that includes the client side attributes that were registered. This works great in most cases.

However, in the case of using a block like <RichText>, the client-side attributes are not validated, parsed and sent to the server side because it renders attributes inside of HTML. This also explains why the PlainText block works fine because it only deals with strings.

Better Explanation

Here is a better explanation of the issue by Andrew Duthie:

Rendering Issue

"So the issue here is that attribute “source” is not retrievable by the server, because it requires parsing the HTML. The other attributes which don’t have a “source” work fine because those are encoded in the comment delimiters, and the server’s post grammar is able to identify those. Further, the shape of the value of children (Editable’s value type) are currently React elements (pending change), and therefore don’t encode well even if you were to drop “source”.

Main GH Issue

The main Github issue dealing with this can be found here:

• Block API: Server-side awareness of block types · Issue #2751 · WordPress/gutenberg · GitHub

Community

I did some research on GH and Slack and did find this exact issue being discussed in a few different places:

• Can't retrive my block's attribute when using server-side rendering · Issue #4117 · WordPress/gutenberg · GitHub
• Rendering Issue

Other Noteable links

• Editable: Treat value as HTML string, eliminate children source · Issue #2750 · WordPress/gutenberg · GitHub
• Creating a Custom Gutenberg Block in WordPress
• Try: Block API: Remove children source in favor of HTML string by aduth · Pull Request #5380 · WordPress/gutenberg · GitHub

Thoughts / Moving Forward

It seems that either choice, client-side or server-side have their drawbacks at this point in time. Client-side seems to be a bit more reliable, but has the ability to break a lot while developing... let alone if a client comes back months later and wants a block updated. With server-side we lose the ability to have access to some client-side data.

Is this as big of an issue as we think? Others like Delicious Brains think it is, so we are not alone.

One idea that is worth exploring would be creating a Higher Order Component that takes the 'value/html' and converts it into string form so it can be passed to the server-side attributes. Human Made is doing something similar here: hm-gutenberg-tools/editable-html.js at master · humanmade/hm-gutenberg-tools · GitHub

/cc @gregrickaby @coreymcollins @kellenmace @jomurgel

[efuller]

Rendering Fix - Block Versioning

It does not seem that this topic is documented or even talked about too much. But it does look like there is a way to version blocks. This let's the validator inside Gutenberg know that the markup or attibutes for the block need to change.

This is being implemented on core blocks, so I guess that makes it OK for us to do?
gutenberg/index.js at master · WordPress/gutenberg · GitHub

Relevant Issues and PR's

• Blocks: Versioning and migrations · Issue #2541 · WordPress/gutenberg · GitHub
• Blocks: Adding versioning and migrations to the Block API by youknowriad · Pull Request #3327 · WordPress/gutenberg · GitHub
• Framework: Handle and upgrade deprecated blocks by youknowriad · Pull Request #3665 · WordPress/gutenberg · GitHub

The Gist

Although more discovery and testing needs to take place, here is how versioning works.

Copy any existing attribute declarations along with the contents of the save function to a new deprecated property (which takes an array) and gets added directly after the save function.

Since the original has been deprecated, you can feel free to make changes to the original save function.

Here is a link to another example:

Versioned Gutenberg Block · GitHub

Questions / Thoughts

While looking into this issue, I got to thinking in regards to the call-to-action block as to why we weren't also using the RichText component inside the save function. Instead we are using this:

<div className="content-block-content content-block">
  { props.attributes.message }
</div>

My first impression is that we should think about things like this. I personally think have no raw HTML in the edit or save functions could be a good pattern to follow?

/cc @gregrickaby @kellenmace @coreymcollins @jomurgel

[coreymcollins]

I was testing the deprecated functionality and ran into some weirdness that isn't necessarily a blocker, but I'm sure there's some way around it.

First, start with your block just the way it is - beautiful and perfect.

Add the deprecated function like so (using the WDS Default Block as an example):

deprecated: [
	{
		attributes: {
			message: {
				type: 'array',
				source: 'children',
				selector: '.content-block',
			},
			...BackgroundOptionsAttributes,
			...TextOptionsAttributes,
			...OtherOptionsAttributes,
		},
		save( props ) {
			return (
				<section
					className={ classnames(
						props.className,
						...BackgroundOptionsClasses( props ),
						...OtherOptionsClasses( props ),
					) }
					style={ {
						...BackgroundOptionsInlineStyles( props ),
						...TextOptionsInlineStyles( props ),
					} }
				>

					{ BackgroundOptionsVideoOutput( props ) }

					<header className="content-block-header">
						<h2>{ __( 'WDS Example Block with Options' ) }</h2>
					</header>

					<div className="content-block-content content-block">
						{ props.attributes.message }
					</div>
				</section>
			);
		},
	},
],

Now, you can add new attributes and edit/save blocks if you need a new field. When you refresh the page in the dashboard, you'll get your updated block with new fields - and it doesn't break!

However, you DO get this in the console:

index.js:23 Block validation: Block validation failed for `wds/default` (Object).

Expected:

<section class="wp-block-wds-default"><header class="content-block-header"><h2>WDS Example Block with Options</h2></header><div class="content-block-content content-block">
        <p>First block!</p>
    </div><div class="content-block-content content-block-new"></div></section>

Actual:

<section class="wp-block-wds-default">
    <header class="content-block-header">
        <h2>WDS Example Block with Options</h2>
    </header>
    <div class="content-block-content content-block">
        <p>First block!</p>
    </div>
</section>

The block actually still functions 100% and allows you to save values in your new field, and after you do so save & refresh the page, the console errors go away. So, the validation errors do not seem to affect the actual functionality and usage of the block, but I'm sure there's a way to get them to NOT show up.

[coreymcollins]

Committing to the same branch here rather than creating more confusion with more branches.

Added the deprecated function in for the Default block.

Added for the hero as well.

I don't believe we need to worry about this for the Github Gist block as it's a dynamic render. I was able to successfully add a new attribute and input to the block and refresh the page in the dashboard without it breaking, so it appears that dynamic render blocks may not need to worry about deprecated even when adding new attributes/inputs. Going to test this a little bit more to be sure.

Update: Confirmed in testing on the dummy Related Posts block I had in place that adding new inputs and attributes to a dynamic render block does not seem to require the use of deprecated. However, if we wind up changing the block wildly and need to switch from PHP rendering to JSX rendering, we'll want to be sure to start using deprecated on said block.

Going to leave CTA and Two-Column for @efuller and @jomurgel respectively so they can play around with deprecated as we wrap this one up.

Thanks for all of the great info above y'all!

[efuller]

@coreymcollins

Was the intention here just to add the deprecated property to all of our blocks (or the ones that break) whether they needed it or not? Please correct me if that was/is not the case.

What do you think about only adding the deprecated property only when needed? This would help cut down on bundle size as well as the code in the actual block.

In the meantime, I'm going to write up a bit of documentation on how to implement versioning.

[coreymcollins]

@efuller My thought was that it would be wise to add the deprecated function for our existing blocks so they're ready if/when we need make adjustments to them and to give devs a real-world example of how the function works. By working them into our blocks, we round out the code a bit more and allow for making those small adjustments (like adjusting classes or structure) that could otherwise break the block if we don't utilize deprecated.

[efuller]

I've added a page in the Wiki on block versioning! https://github.com/WebDevStudios/wds-gutenberg/wiki/Block-Versioning

I think we can wrap this issue tomorrow morning!