Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code improvements for the SSR part of the Interactivity API #51640

Merged
merged 18 commits into from
Jun 23, 2023
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -173,4 +173,21 @@ public static function is_html_void_element( $tag_name ) {
return false;
}
}

/**
* Extract and return the directive type and the the part after the double
* hyphen from an attribute name (if present), in an array format.
*
* Examples:
*
* 'wp-island' => array( 'wp-island', null )
* 'wp-bind--src' => array( 'wp-bind', 'src' )
* 'wp-thing--and--thang' => array( 'wp-thing', 'and--thang' )
*
* @param string $name The attribute name.
* @return array The resulting array
*/
public static function parse_attribute_name( $name ) {
return explode( '--', $name, 2 );
}
}
70 changes: 42 additions & 28 deletions lib/experimental/interactivity-api/directive-processing.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ function gutenberg_interactivity_process_directives( $tags, $prefix, $directives
$tag_stack = array();

while ( $tags->next_tag( array( 'tag_closers' => 'visit' ) ) ) {
$tag_name = strtolower( $tags->get_tag() );
$tag_name = $tags->get_tag();

// Is this a tag that closes the latest opening tag?
if ( $tags->is_tag_closer() ) {
Expand All @@ -81,27 +81,31 @@ function gutenberg_interactivity_process_directives( $tags, $prefix, $directives
if ( $latest_opening_tag_name === $tag_name ) {
array_pop( $tag_stack );

// If the matching opening tag didn't have any attribute directives,
// we move on.
// If the matching opening tag didn't have any directives, we move on.
if ( 0 === count( $attributes ) ) {
continue;
}
}
} else {
// Helper that removes the part after the double hyphen before looking for
// the directive processor inside `$attribute_directives`.
$get_directive_type = function ( $attr ) {
return explode( '--', $attr )[0];
};

$attributes = $tags->get_attribute_names_with_prefix( $prefix );
$attributes = array_map( $get_directive_type, $attributes );
$attributes = array_intersect( $attributes, array_keys( $directives ) );

// If this is an open tag, and if it either has attribute directives, or
// if we're inside a tag that does, take note of this tag and its
// attribute directives so we can call its directive processor once we
// encounter the matching closing tag.
$attributes = array();
foreach ( $tags->get_attribute_names_with_prefix( $prefix ) as $name ) {
/*
* Removes the part after the double hyphen before looking for
* the directive processor inside `$directives`, e.g., "wp-bind"
* from "wp-bind--src" and "wp-context" from "wp-context" etc...
*/
list( $type ) = WP_Directive_Processor::parse_attribute_name( $name );
if ( array_key_exists( $type, $directives ) ) {
$attributes[] = $type;
}
}

/*
* If this is an open tag, and if it either has directives, or if
* we're inside a tag that does, take note of this tag and its
* directives so we can call its directive processor once we
* encounter the matching closing tag.
*/
if (
! WP_Directive_Processor::is_html_void_element( $tags->get_tag() ) &&
( 0 !== count( $attributes ) || 0 !== count( $tag_stack ) )
Expand Down Expand Up @@ -131,26 +135,36 @@ function gutenberg_interactivity_evaluate_reference( $path, array $context = arr
array( 'context' => $context )
);

if ( strpos( $path, '!' ) === 0 ) {
$path = substr( $path, 1 );
$has_negation_operator = true;
}

$array = explode( '.', $path );
$current = $store;
foreach ( $array as $p ) {
/*
* Check first if the directive path is preceded by a negator operator (!),
* indicating that the value obtained from the Interactivity Store (or the
* passed context) using the subsequent path should be negated.
*/
$should_negate_value = '!' === $path[0];

$path = $should_negate_value ? substr( $path, 1 ) : $path;
$path_segments = explode( '.', $path );
$current = $store;
foreach ( $path_segments as $p ) {
if ( isset( $current[ $p ] ) ) {
$current = $current[ $p ];
} else {
return null;
}
}

// Check if $current is a function and if so, call it passing the store.
if ( is_callable( $current ) ) {
/*
* Check if $current is an anonymous function or an arrow function, and if
* so, call it passing the store. Other types of callables are ignored in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tiny nitpick: "ignored in on purpose"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. ✅ Thanks for the review, Dennis. 😄

* purpose, as arbitrary strings or arrays could be wrongly evaluated as
* "callables".
*
* E.g., "file" is an string and a "callable" (the "file" function exists).
*/
if ( $current instanceof Closure ) {
$current = call_user_func( $current, $store );
}

// Return the opposite if it has a negator operator (!).
return isset( $has_negation_operator ) ? ! $current : $current;
return $should_negate_value ? ! $current : $current;
}
2 changes: 1 addition & 1 deletion lib/experimental/interactivity-api/directives/wp-bind.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function gutenberg_interactivity_process_wp_bind( $tags, $context ) {
$prefixed_attributes = $tags->get_attribute_names_with_prefix( 'data-wp-bind--' );

foreach ( $prefixed_attributes as $attr ) {
list( , $bound_attr ) = explode( '--', $attr );
list( , $bound_attr ) = WP_Directive_Processor::parse_attribute_name( $attr );
if ( empty( $bound_attr ) ) {
continue;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/experimental/interactivity-api/directives/wp-class.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function gutenberg_interactivity_process_wp_class( $tags, $context ) {
$prefixed_attributes = $tags->get_attribute_names_with_prefix( 'data-wp-class--' );

foreach ( $prefixed_attributes as $attr ) {
list( , $class_name ) = explode( '--', $attr );
list( , $class_name ) = WP_Directive_Processor::parse_attribute_name( $attr );
if ( empty( $class_name ) ) {
continue;
}
Expand Down
5 changes: 4 additions & 1 deletion lib/experimental/interactivity-api/directives/wp-context.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ function gutenberg_interactivity_process_wp_context( $tags, $context ) {
}

$new_context = json_decode( $value, true );
// TODO: Error handling.
if ( null === $new_context ) {
// Invalid JSON defined in the directive.
return;
}
Comment on lines -27 to +30
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future, this type of error handling should be centralized in the WP_Directive_Context class so anyone using it would benefit.

Actually, it's pretty basic, but there is already protection against falsy values like null in the set_context method: link


$context->set_context( $new_context );
}
2 changes: 1 addition & 1 deletion lib/experimental/interactivity-api/directives/wp-style.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function gutenberg_interactivity_process_wp_style( $tags, $context ) {
$prefixed_attributes = $tags->get_attribute_names_with_prefix( 'data-wp-style--' );

foreach ( $prefixed_attributes as $attr ) {
list( , $style_name ) = explode( '--', $attr );
list( , $style_name ) = WP_Directive_Processor::parse_attribute_name( $attr );
if ( empty( $style_name ) ) {
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public function test_evaluate_function_should_return_null_for_unresolved_paths()
$this->assertNull( gutenberg_interactivity_evaluate_reference( 'this.property.doesnt.exist' ) );
}

public function test_evaluate_function_should_execute_functions() {
public function test_evaluate_function_should_execute_anonymous_functions() {
$context = new WP_Directive_Context( array( 'count' => 2 ) );
$helper = new Helper_Class;

Expand All @@ -140,6 +140,7 @@ public function test_evaluate_function_should_execute_functions() {
'anonymous_function' => function( $store ) {
return $store['state']['count'] + $store['context']['count'];
},
// Other types of callables should not be executed.
'function_name' => 'gutenberg_test_process_directives_helper_increment',
'class_method' => array( $helper, 'increment' ),
'class_static_method' => 'Helper_Class::static_increment',
Expand All @@ -149,8 +150,21 @@ public function test_evaluate_function_should_execute_functions() {
);

$this->assertSame( 5, gutenberg_interactivity_evaluate_reference( 'selectors.anonymous_function', $context->get_context() ) );
$this->assertSame( 5, gutenberg_interactivity_evaluate_reference( 'selectors.function_name', $context->get_context() ) );
$this->assertSame( 5, gutenberg_interactivity_evaluate_reference( 'selectors.class_static_method', $context->get_context() ) );
$this->assertSame( 5, gutenberg_interactivity_evaluate_reference( 'selectors.class_static_method_as_array', $context->get_context() ) );
$this->assertSame(
'gutenberg_test_process_directives_helper_increment',
gutenberg_interactivity_evaluate_reference( 'selectors.function_name', $context->get_context() )
);
$this->assertSame(
array( $helper, 'increment' ),
gutenberg_interactivity_evaluate_reference( 'selectors.class_method', $context->get_context() )
);
$this->assertSame(
'Helper_Class::static_increment',
gutenberg_interactivity_evaluate_reference( 'selectors.class_static_method', $context->get_context() )
);
$this->assertSame(
array( 'Helper_Class', 'static_increment' ),
gutenberg_interactivity_evaluate_reference( 'selectors.class_static_method_as_array', $context->get_context() )
);
}
}