Server directive processing: Improve how block references are saved

[cbravobernal]

What?

A small refactor to process only root blocks, deleting the root block reference after processing instead of using arrays comparison.

Why?

Although previous version works, this one is easier to read and have better performance.

[github-actions]

This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress.

If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged.

If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack.

Thank you! ❤️

View changed files

❔ lib/experimental/interactivity-api/class-wp-directive-processor.php
❔ lib/experimental/interactivity-api/directive-processing.php
❔ phpunit/experimental/interactivity-api/directive-processing-test.php

[luisherranz]

Hey, Carlos, can you please add these tests:

• Check how many times process_rendered_html has been called for different blocks, including one that has a Pattern block.
• Check that even if a block with the same md5 is found, it's not processed if it's not a root block.

One question: Why did you decide not to create a different class than WP_Directive_Processor? Didn't it make sense?

[luisherranz]

We can rename this filter again because it's not processing the directives anymore, only marking the root blocks.

Also, the DocBlock description needs to be updated.

[luisherranz]

Passing $parsed_block three times is confusing.

Suggested change

	gutenberg_interactivity_process_directives( $parsed_block, $parsed_block, $parsed_block );
	$fake_parent_block = array();
	$source_block = $parsed_block;
	gutenberg_interactivity_process_directives( $parsed_block, $source_block, $fake_parent_block );

[luisherranz]

You are not testing root blocks if you are passing a parent.

Suggested change

	// Test that a root block is not added if there is already a root block defined.
	gutenberg_interactivity_process_directives( $parsed_block_second, $parsed_block_second, $parsed_block_second );
	// Test that a root block is not added if there is already a root block defined.
	gutenberg_interactivity_process_directives( $parsed_block_second, $parsed_block_second, null );

[cbravobernal]

One question: Why did you decide not to create a different class than WP_Directive_Processor? Didn't it make sense?

Not yet, I think we should keep working on that class and then split it if we consider necessary.

[cbravobernal]

Check how many times process_rendered_html has been called for different blocks, including one that has a Pattern block.

I'm afraid we cannot do this as a Unit test (is more an e2e test). In order to count how many times a function is called, you need a similar code to this one (provided by Copilot)

public function testProcessRenderedHtmlCalledXTimes()
    {
        // Create a mock for the class containing the process_rendered_html method
        $tagsMock = $this->getMockBuilder('TheClassName') // replace 'TheClassName' with the actual class name
                         ->setMethods(['process_rendered_html'])
                         ->getMock();

        // Specify the number of times the method should be called
        $tagsMock->expects($this->exactly(3)) // replace 3 with the number of times the method should be called
                 ->method('process_rendered_html')
                 ->willReturn('...'); // replace '...' with the value you want the method to return

        // Replace the real object with the mock in your function
        $tags = $tagsMock;
        $directives = array(
            'data-wp-bind'    => 'gutenberg_interactivity_process_wp_bind',
            'data-wp-context' => 'gutenberg_interactivity_process_wp_context',
            'data-wp-class'   => 'gutenberg_interactivity_process_wp_class',
            'data-wp-style'   => 'gutenberg_interactivity_process_wp_style',
            'data-wp-text'    => 'gutenberg_interactivity_process_wp_text',
        );

        // Call the method the specified number of times
        $tags->process_rendered_html($tags, 'data-wp-', $directives);
        $tags->process_rendered_html($tags, 'data-wp-', $directives);
        $tags->process_rendered_html($tags, 'data-wp-', $directives);
    }

You are "duplicating" the class, so using it in different functions won't work.

Also, that way, we are testing different things rather than the "root blocks code" of the PR, we would be testing:

• That a filter is executed.
• That render_block_data filter provides a $parent_block variable.

We can instead provide an mocked $parent_block just to check that function gutenberg_interactivity_process_directives does what it is required.

Hope it helps!

[cbravobernal]

@luisherranz I've addressed most of the changes requested, thanks for reviewing!

[luisherranz]

Perfect. Thanks @c4rl0sbr4v0!!

[youknowriad]

Has this been backported to Core already?

[luisherranz]

Yesss.