Interactivity API: initial support for SSR

[luisherranz]

What?

This PR adds server-side rendering for the following directives:

• wp-context
• wp-bind
• wp-class
• wp-style
• wp-text

Tracking issue: #51056

Why?

Server-side rendering of the directives ensures that the HTML that the server sends will match the first HTML generated after the initial hydration improving the UX and SEO.

How?

Most of the code was created originally by @ockham in the block-interactivity-experiments repo with the help of @dmsnell and others, and ported by me to Gutenberg.

The current implementation uses either the HTML Tag Processor or a new class built on top of it (WP Directive Processor) that adds things like set_inner_html which are necessary for wp-text.

IMPORTANT DISCLAIMER: This code of the WP_Directive_Processor class is highly experimental and its only purpose is to provide a way to test the server-side rendering of the Interactivity API. Most of this code will be discarded once the HTML Processor is available. Please restrain from investing unnecessary time and effort trying to improve this code.

The technique to process only the root nodes is new (it was not in the BIE repo) but it doesn't yet work as it should because it processes the root blocks of each post loaded (the template, the template parts, the post content, the reusable blocks…), when it should only process the root blocks of the template to ensure that directives are only processed once. Therefore, there are still multiple processing of the same directives happening. Still, I would prefer to merge this PR and try to fix it in a subsequent one.

Testing Instructions

• Open a post
• Add an HTML block
• Paste this code

  <div data-wp-context='{ "hide": false, "color": "red" }'>
    <div
      class="another"
      style="text-decoration: underline"
      data-wp-bind--hidden="context.hide"
      data-wp-class--show="!context.hide"
      data-wp-style--color="context.color"
      data-wp-text="context.color"
    >
      default value
    </div>
  </div>

• Take a look at the frontend, there should be a red underlined text saying "red" with the class "show"
• Change the "color" to "blue".
• Now there should be a blue underlined text saying "blue"
• Change "hide" to true.
• The text now should have a hidden attribute and the "show" class should not be there anymore

[luisherranz]

I've added support for functions during the SSR in c8d3c7f. It works pretty much like in JS:

<?php
$unique_id = substr( uniqid(), -5 );
$store = wp_store();
$previous_id = isset( $store['state']['selected'] ) && $store['state']['selected'];

wp_store( array(
  'state' => array(
    'selected' => $previous_id || $unique_id,
  ),
  'selectors' => array(
    'isOpen' => function( $store ) {
      return $store['state']['selected'] === $store['context']['id'];
    }
  )
) );
?>

<div
  <?php echo get_block_wrapper_attributes(); ?>
  data-wp-context='{ "id": "<?php echo $unique_id; ?>" }'
>
  ...
</div>

import { store } from "@wordpress/interactivity";

store({
  selectors: {
    isOpen: ({ state, context }) => state.selected === context.id,
  },
});

[luisherranz]

I think this should be ready for review.

Performance tests are still failing, but they don't seem related. I'll try merging trunk with the interactivity branch to see if that solves the issue.

[DAreRodz]

Code LGTM! Funny to see directives work without the runtime. 😄

[luisherranz]

Thanks, David! Merging now 🙂

@ockham we can fix the markTestSkipped thing in the parent PR.

[dmsnell]

it would be great if we added explanatory comments in places like this where we're performing arbitrary transforms on data. I don't think it's at all obvious to anyone who hasn't worked recently in this code what the purpose is here, but a single example in a comment could clarify that instantly.

also, here and elsewhere, WordPress coding style will require all multi-line comments use the /* .. */ format.

 * Example
 *
 *     'wp-context' === $get_directive_type( 'wp-context' );
 *     'wp-bind'    === $get_directive_type( 'wp-bind--src' );
 */

[dmsnell]

for cases like this in performance sensitive code (of which this very much is) it would be good to adopt a default practice of avoiding multiple iterations and array functions. we can avoid a function call and avoid the costly array iterations and intersection with a direct foreach loop

$attributes = array();
foreach ( $tags->get_attribute_names_with_prefix( $prefix ) as $name ) {
	// Get "wp-bind" from "wp-bind--src" and "wp-context" from "wp-context" etc...
	list( $type ) = explode( '--', $name );
	if ( array_key_exists( $type, $directives ) ) {
		$attributes[] = $name;
	}
}

[dmsnell]

similar note here on an explanatory comment - it's extremely confusing to try and figure out what's happening here and why. so few words can save so much hassle. also, we are welcome to use str_starts_with

/*
 * I actually have no idea what this is doing.
 * At first I assumed this negated the value of a context value,
 * but "resolve the reference using the store and the context
 * from the provided path" is a jumble of very generic words and
 * I am lost. If I understood what's happening though this
 * comment would be quite terse.
 */
if ( str_starts_with( $path, '!' ) ) {

or, given that all we need to do is check the first character we can avoid a function call and do the much faster character check

// Meaningful explanatory comment.
if ( '!' === $path[0] ) {

[dmsnell]

is there a better name than $array? how about $data_path or $data_path_segments or $value_path etc…?

[dmsnell]

note that the comment and code contradict each other here. almost anything can be a callable, including things that we don't want to treat as callable here. "file" is callable, because there's a PHP function named file. this means that it's not possible to use a context path like users.avatar.file because we will try and run file and this will likely crash in surprising and unexpected ways.

given that the default behavior here is the dangerous and unsurprising one, it would be valuable to restrict this either to actual function data types or require more strict means for indicating that something ought to be called.

[DAreRodz]

There are other callable types, like an array of two strings, e.g., array( 'MyClass', 'some_static_method' ). Those are harder to be evaluated as real callable, but it is another possibility. 🤔

We could restrict $current to be an instance of Closure, which is the class that anonymous and arrow functions use. I don't think developers should be allowed to use a different callable type inside the store.

[luisherranz]

Sounds good to me.

[DAreRodz]

And, in any case, if you want to use another callable, you can do so inside an anonymous function. 🤷

[dmsnell]

why are we lower-casing the tag name? I think someone explained this once, but is there any reason to do so?

[dmsnell]

personal styling note, but I find a boolean value here might be clearer than a true-or-absent value

$should_negate_context_value = '!' === $path[0];
$path = $should_negate_context_value ? substr( $path, 1 ) : $path;

…

return $should_negate_context_value ? ! $current : $current;

[dmsnell]

how about a single function somewhere, maybe in class-wp-directive-processor.php, that parses the directive name. it's not much code, but given that we're already doing it in different ways and in each of these files, seems like it could be clearer to move it inward

// class-wp-directive-processor {
	public static function parse_attribute_name( $name ) {
		/*
		 * Blah blah blargy bloop.
		 *
		 * Example
		 *     'wp-island'            => array( 'wp-island', null )
		 *     'wp-bind--src'         => array( 'wp-bind', 'src' )
		 *     'wp-thing--and--thang' => array( 'wp-thing', 'and--thang' )
		 */
		return explode( '--', $name, 2 );
	}
}

list( $type ) = WP_Directive_Processor::parse_attribute_name( $name );

list( $type, $class_name ) = WP_Directive_Processor::parse_attribute_name( $name );

[dmsnell]

it's not much work to do basic error-handling here. right now we're setting context to null on any JSON decode failures. maybe we should abort in this case and circle back swiftly on it

$new_context = json_decode( $value, true );
if ( null === $new_context ) {
	return;
}

$context->set_context( $new_context );

[dmsnell]

it's not much work to do basic error-handling here. right now we're setting context to null on any JSON decode failures. maybe we should abort in this case and circle back swiftly on it

$new_context = json_decode( $value, true );
if ( null === $new_context ) {
	return;
}

$context->set_context( $new_context );