Make WordPress Core

Opened 5 weeks ago

Closed 3 weeks ago

Last modified 9 days ago

#61185 closed enhancement (fixed)

Interactivity API: Move directive processing to `WP_Block` class

Reported by: gziolo's profile gziolo Owned by: gziolo's profile gziolo
Milestone: 6.6 Priority: normal
Severity: normal Version: 6.5
Component: Interactivity API Keywords: has-patch
Focuses: Cc:

Description

The way directive processing got integrated into WordPress Core was largely influenced by the implementation present in the Gutenberg plugin. We don't need to use WP hooks, but instead it's possible to directly integrate the processing into the WP_Block class. It removes the overhead of running additional hooks on parsed blocks and simplifies the way we detect whether the directive processing should run on an interactive region of the produced final HTML for the blocks.

Change History (14)

This ticket was mentioned in PR #6331 on WordPress/wordpress-develop by @gziolo.


5 weeks ago
#1

Trac ticket: https://core.trac.wordpress.org/ticket/61185#ticket

Testing performance with TT4 and homepage:

https://github.com/WordPress/wordpress-develop/assets/699132/f06a74d7-5e79-4957-9604-17ca5fa5eb85

Before (trunk): wp-total (median) 55.59 ms
After (branch): wp-total (median) 55.18 ms

#2 follow-up: @gziolo
5 weeks ago

I now see that it might have some implications on e2e tests in the Gutenberg plugin where wp_interactivity_process_directives_of_interactive_blocks gets removed:

https://github.com/WordPress/gutenberg/blob/f69bd7af630194b459c7de1cba7627a39715e5fa/packages/e2e-tests/plugins/interactive-blocks.php#L32-L36

We would need to figure out an alternative way to disable directive processing. It would be valuable to have a formal way to do it anyway as the need clearly exists. Any ideas how that could be done?

#3 in reply to: ↑ 2 @cbravobernal
5 weeks ago

Replying to gziolo:

I now see that it might have some implications on e2e tests in the Gutenberg plugin where wp_interactivity_process_directives_of_interactive_blocks gets removed:

https://github.com/WordPress/gutenberg/blob/f69bd7af630194b459c7de1cba7627a39715e5fa/packages/e2e-tests/plugins/interactive-blocks.php#L32-L36

We would need to figure out an alternative way to disable directive processing. It would be valuable to have a formal way to do it anyway as the need clearly exists. Any ideas how that could be done?

Those tests are testing that the runtime is still working, while the SSR is not. I am not sure if it is a real need to be able to disable SSR, but it would be a great addition.

We could add a filter (seems pretty straightforward) or use wp_interactivity_config.
https://github.com/gziolo/wordpress-develop/blob/ff14587d0885caaaf620c4bc5c7951b1a7f388c9/src/wp-includes/interactivity-api/interactivity-api.php#L77

Last edited 5 weeks ago by cbravobernal (previous) (diff)

#4 @oglekler
3 weeks ago

@gziolo and @cbravobernal, we have 1 week before Beta 1. Please look realistic on ability to finish this task in time. From first glance is looks like you don't have final decision and this ticket should be moved to 6.7, but if there are some dependencies that will be broken without change, then you should hurry up with the change. Thank you!

@cbravobernal commented on PR #6331:


3 weeks ago
#5

Added a filter to be able to enable/disable server directives processing and an unit test for it.

@gziolo commented on PR #6331:


3 weeks ago
#6

I documented the new filter. I renamed it to interactivity_process_directives as wp_ prefix doesn't add too much value here. It's used sometimes with functions to ensure more general names don't conflict.

I also opened https://github.com/WordPress/gutenberg/pull/62095 to have the fix in place to Gutenberg at the time of the commit so E2E tests continue to pass.

#7 @gziolo
3 weeks ago

  • Owner set to gziolo
  • Resolution set to fixed
  • Status changed from new to closed

In 58234:

Interactivity API: Move directive processing to WP_Block class

Integrates the directives processing into the WP_Block class. It removes the overhead of running additional hooks when rendering blocks and simplifies the way we detect whether the directive processing should run on an interactive region of the produced final HTML for the blocks.

Introduces interactivity_process_directives filter to offer a way to opt out from directives processing. It's needed in Gutenberg: https://github.com/WordPress/gutenberg/pull/62095.

Props gziolo, cbravobernal.
Fixes #61185.

@luisherranz commented on PR #6331:


10 days ago
#9

I documented the new filter. I renamed it to interactivity_process_directives as wp_ prefix doesn't add too much value here. It's used sometimes with functions to ensure more general names don't conflict.

Since this filter is used to control whether the blocks are processed or not, wouldn't it be better to include the word "blocks" in the name of the filter? Otherwise, people can think it is related to the wp_interactivity_process_directives( $html ) function, that works for arbitrary HTML.

Like interactivity_process_directives_in_blocks or something similar...

@gziolo commented on PR #6331:


10 days ago
#10

It also could be moved to the body of wp_interactivity_process_directives to cover all use cases with a single filter. This way we also won't need to change anything in the Gutenberg plugin.

@luisherranz commented on PR #6331:


10 days ago
#11

What is the main use of this filter? Is it so that plugins can disable the Server Directive Processing for some reason, or is it so that Gutenberg can replace it with a more updated version?

@cbravobernal commented on PR #6331:


10 days ago
#12

We have e2e tests that disable the Server Directive Processing. That's the only one that come to my mind right now. Gutenberg crashed and we had to choose between 2 options:

  • Remove those tests, or update them to take into account SSR.
  • Add a filter to allow developers to choose.

@luisherranz commented on PR #6331:


10 days ago
#13

Ok, thanks Carlos 🙂

I don't have a strong opinion on whether we should rename the filter or also include the processing of the wp_interactivity_process_directives function, but I would do one of those.

@cbravobernal commented on PR #6331:


9 days ago
#14

Ok, thanks Carlos 🙂

I don't have a strong opinion on whether we should rename the filter or also include the processing of the wp_interactivity_process_directives function, but I would do one of those.

I think disabling all SSR would make more sense than only in blocks, right?

Note: See TracTickets for help on using tickets.