Limit the hydration to block when client-side navigations are deactivated by luisherranz · Pull Request #124 · WordPress/block-interactivity-experiments

luisherranz · 2022-12-20T14:36:55Z

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

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.

It's still not working, at least in my local environment, but at least I didn't change it.

SantosGuillamot · 2022-12-20T15:25:18Z

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 · 2022-12-20T15:58:48Z

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 · 2022-12-21T07:59:13Z

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 · 2022-12-21T08:11:12Z

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.

…abled

luisherranz · 2022-12-22T17:42:09Z

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 · 2022-12-23T07:46:08Z

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 · 2022-12-23T08:47:24Z

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 · 2022-12-23T10:01:14Z

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!

wp-directives.php

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 · 2022-12-23T10:35:33Z

wp-directives.php

+function wp_directives_get_client_side_transitions() {
+	static $client_side_transitions = null;
+	if ( is_null( $client_side_transitions ) ) {
+		$client_side_transitions = apply_filters( 'client_side_transitions', false );
+	}
+	return $client_side_transitions;
+}


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;

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 · 2022-12-23T11:03:41Z

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

SantosGuillamot · 2022-12-23T13:01:01Z

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 · 2022-12-23T15:44:53Z

Done!

Automatically detect if a block is using directives #126

luisherranz added 2 commits December 20, 2022 12:20

Add tabs block

75c7758

Add interactivity support

08d3a01

luisherranz added the 🎨 WordPress Directives Plugin label Dec 20, 2022

luisherranz self-assigned this Dec 20, 2022

This was referenced Dec 20, 2022

Limit the hydration of the Directives Hydration when the site doesn't support client-side navigations #93

Closed

Tracking issue: WordPress Directives Plugin 🎨 #80

Closed

luisherranz added 3 commits December 20, 2022 16:08

Revert changes to admin page

a40cbcc

It's still not working, at least in my local environment, but at least I didn't change it.

Disable full vDOM hydration

69e2006

Hydrate interactive blocks (don't stop)

54dcc51

Add wp-inner-block attribute

c656209

luisherranz added 4 commits December 20, 2022 17:27

Do either directive attributes or tag

2d4df41

Stop hydration when it finds wp-inner-block

77bbf08

Rename to wpx to avoid conflicts with wp-

691da6d

Switch to WP format

6a9dd4a

luisherranz added 12 commits December 21, 2022 16:58

Rename blocks

a3dc09b

Only add wpx-non-interactive to inner blocks of isolated blocks

e55b61f

Rename wpx to ignore and island

4e415a5

Rename back wpx to wp

d0f29a7

Init playwright

ab16c13

Add first e2e tests

cdb77f2

Start GH action

3860148

Add build step

5626c59

Move wp-ignore inside the wp- logic

31a22ea

Don't hydrate interactive blocks twice

b851f5b

Test islands inside inner block of isolated islands

1cba1bb

Small text changes

dcb811d

luisherranz added 7 commits December 22, 2022 11:52

Fix syntax

54cc766

Fix admin (back to original format)

8c06850

Fix syntax and correct broken code

2c70c56

Don't add wp-island and wp-ignore when client-side transitions are en…

6e4fb91

…abled

Rename tests

0381d82

Fix full tests

fccda53

Switch to wpx specific for the e2e tests

9baabb7

luisherranz changed the title ~~WIP: Limit the hydration to block when client-side transitions are deactivated~~ Limit the hydration to block when client-side transitions are deactivated Dec 22, 2022

Remove blocks

7656707

luisherranz marked this pull request as ready for review December 22, 2022 17:42

luisherranz requested a review from SantosGuillamot December 22, 2022 17:42

luisherranz requested a review from DAreRodz December 23, 2022 07:50

cbravobernal reviewed Dec 23, 2022

View reviewed changes

wp-directives.php Outdated Show resolved Hide resolved

SantosGuillamot approved these changes Dec 23, 2022

View reviewed changes

Avoid unnamed functions

94f594f

cbravobernal reviewed Dec 23, 2022

View reviewed changes

luisherranz merged commit 168d09a into main-wp-directives-plugin Dec 23, 2022

luisherranz deleted the limit-hydration branch December 23, 2022 11:03

luisherranz mentioned this pull request Dec 23, 2022

Automatically detect if a block is using directives #126

Closed

luisherranz mentioned this pull request Dec 28, 2022

Fix wp-class and introduce the evaluate function #127

Merged

SantosGuillamot mentioned this pull request Jan 11, 2023

Tracking Issue: Stress-testing Directives Hydration 🧪 #92

Closed

luisherranz mentioned this pull request Jan 16, 2023

Add lazy hydration techniques: idle and view #75

Closed

luisherranz changed the title ~~Limit the hydration to block when client-side transitions are deactivated~~ Limit the hydration to block when client-side navigations are deactivated Jan 30, 2023

Conversation

luisherranz commented Dec 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

How

Tasks

Additional comments

Uh oh!

SantosGuillamot commented Dec 20, 2022

Uh oh!

luisherranz commented Dec 20, 2022

Uh oh!

SantosGuillamot commented Dec 21, 2022

Uh oh!

luisherranz commented Dec 21, 2022

Uh oh!

luisherranz commented Dec 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

luisherranz commented Dec 23, 2022

Uh oh!

SantosGuillamot commented Dec 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

luisherranz commented Dec 23, 2022

Uh oh!

Uh oh!

SantosGuillamot left a comment

Choose a reason for hiding this comment

Uh oh!

cbravobernal Dec 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luisherranz Dec 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luisherranz commented Dec 23, 2022

Uh oh!

SantosGuillamot commented Dec 23, 2022

Uh oh!

luisherranz commented Dec 23, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

luisherranz commented Dec 20, 2022 •

edited

Loading

luisherranz commented Dec 22, 2022 •

edited

Loading

SantosGuillamot commented Dec 23, 2022 •

edited

Loading

cbravobernal Dec 23, 2022 •

edited

Loading

luisherranz Dec 23, 2022 •

edited

Loading