Limit the hydration to block when client-side navigations are deactivated

[luisherranz]

What

This PR will limit the hydration so it only happens in the interactive blocks.

Fixes #93.

Why

To avoid the cost of the Directives Hydration when it's strictly not necessary. Full explanation in #93.

How

It keeps using the same meta tag mechanism to know if client-side navigations are enabled or not. If they are not, instead of doing the Directives Hydration, it looks for the interactive blocks and hydrates only those. Interactive blocks opt-in using a supports.interactivity property in their block.json and they are marked with a wp-island attribute in the HTML.

Tasks

• Add a block example
• Add wp-island attribute to blocks with supports.interactivity
• Limit the hydration to the blocks with the wp-island attribute
• Add wp-ignore attribute
• Add inner blocks to the blocks
• Stop the hydration in the wp-ignore nodes
• Add support for interactive block parents (as explained in Limit the hydration of the Directives Hydration when the site doesn't support client-side navigations #93)
• Don't stop the hydration for interactive block parents
• Fix syntax (switch from prettier-php to phpcs)
• Don't hydrate interactive blocks that are inside interactive block parents twice
• Support hydration of interactive blocks that are also inner blocks
• Add e2e tests
• Don't add wp-island and wp-ignore when client-side navigations are enabled
• Don't stop in wp-ignore when client-side navigations are enabled

Additional comments

• I've refactored the admin page because it was not saving the setting, I don't know why.
• I've fixed a problem in the deepsignal code, which was broken in the main branch.

[SantosGuillamot]

Thanks for opening the Pull Request!

Something I was wondering about: Would it make sense, and would it be possible, to automatically detect if a block is using directives and add the wp-interactive-block attribute even if the block developer didn't add the supports.interactivity property in the block.json? Now that we are planning to use the WP_HTML_Tag_Processor, it seems feasible to me, but I am not sure if it would be safe or if it even makes sense. Any thoughts?

Btw, it could be something to do in the future and it may be out of the scope of this PR.

[luisherranz]

automatically detect if a block is using directives and add the wp-interactive-block attribute

Yes, it would be possible 🙂

What is not possible is to differentiate between an "isolated" and a "parent" block. But maybe we could do the opposite and treat every interactive block as a parent block unless it specifies that it is isolated? That way it'd be safer to automatically detect the directives.

[SantosGuillamot]

I've just watched again the video you shared in the issue, and you mentioned it. I knew I had heard the idea somewhere 😄

But maybe we could do the opposite and treat every interactive block as a parent block unless it specifies that it is isolated?

Maybe not every interactive block but the ones including directives that could imply it needs inter-block communication like wp-context, error-boundaries, etc.? As a block developer, I believe I would prefer to use the block.json to opt-out and not having to include a property for each interactive block I create. It could also help for plugins modifying the markup of a specific block to add some directives and not have to modify the block.json as well.

[luisherranz]

the ones including directives that could imply it needs inter-block communication like wp-context, error-boundaries, etc.?

Yeah, good point. That would require declaring it somehow on the directive declaration, but directive declarations will be used way less often so we can push that complexity there 🙂

It could also help for plugins modifying the markup of a specific block to add some directives and not have to modify the block.json as well.

True!

---

Let's do that then, but in a subsequent PR.

[luisherranz]

This should be ready for review.

https://www.loom.com/share/78003554808d492b819794170e95f38a
https://excalidraw.com/#json=yPz4d84xWWLZR-KqTQ0_K,-FjTu31WIkjU4HSTDfh5jQ

Some notes:

• I didn't add tests for the PHP code yet.
• We need to confirm that querySelectorAll returns parents first. Confirmed by Mario here: Limit the hydration to block when client-side navigations are deactivated #124 (comment)
• We need to confirm that doing dangerouslySetInnerHtml doesn't recreate the HTML nodes.
• We should prevent wpx from being bundled inside each view.js file.
• I forgot to mention in the video that the Playwright VS Code extension is awesome.
• Thanks to @c4rl0sbr4v0 for helping me find the bug in the admin page.
• Thanks to @DAreRodz for his code to add wp-ignore to the inner blocks.

[luisherranz]

By the way, once this is merged, I want to add it to #103 to compare the querySelectorAll with createTreeWalker to see if what Manu from Qwik said in this tweet is true in our use case as well.

This is already done with createTreeWalker() which we found even faster than doing a querySelectAll()

[SantosGuillamot]

I started taking a look at the PR. It looks great! I have a couple of comments so far:

• I was testing the blocks, as you do in the video, and it seems that when it is isolated, it only adds the wp-ignore attribute to the first children of the same block. Maybe we have to iterate somehow here? For example, adding the following custom HTML in an isolated block doesn't seem to work as expected because only the paragraph gets the attribute:

<p>Isolated block</p>
<div><wp-show when="state.dontShow">hide me</wp-show></div>

• Regarding the potential issue with querySelectorAll returning the children before the parent, it seems the method returns the NodeList in document order: "The querySelectorAll() methods on the Document, DocumentFragment, and Element interfaces must return a NodeList containing all of the matching Element nodes within the subtrees of the context node, in document order."

[luisherranz]

it only adds the wp-ignore attribute to the first children of the same block

Yes, the requirement for blocks to have a single wrapper node extends to inner blocks as well. Maybe the Custom HTML block should have a default wrapper: <div class="wp-block-custom-html">...</div>.

But maybe something can be done with the new capabilities of the WP_HTML_Tag_Processor to overcome this limitation.

Regarding the potential issue with querySelectorAll returning the children before the parent, it seems the method returns the NodeList in document order

Wonderful. Thanks for checking it out, Mario!

[SantosGuillamot]

Yes, the requirement for blocks to have a single wrapper node extends to inner blocks as well. Maybe the Custom HTML block should have a default wrapper.

Okay, I didn't remember that part. Then everything looks great to me 🙂 Although it would be great if someone else can double check the code.

[cbravobernal]

Can we here just call the function that gets the options instead of creating a new filter? get_option() by default uses filter and return false in case it does not exist.

Something like

$directive_plugins_settings = get_option('wp_directives_plugin_settings');
is_array($directive_plugins_settings) return $directive_plugins_settings['client_side_transitions'];
return $false;

[luisherranz]

It's very unlikely that we will start storing that option in a plugin's setting. It will likely be an experimental feature that will need to be turned on using the filter, so I'd rather keep it as it is 🙂

[luisherranz]

Merging now so we can continue. If there's something broken, we'll fix it later 🙂

[SantosGuillamot]

Btw, should we open a new issue to automate this with the HTML_Tag_Processor? Or is there a better place to keep track of that?

[luisherranz]

Done!

• Automatically detect if a block is using directives #126