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

Server directive processing: Stop processing non-interactive blocks #56302

Merged
merged 42 commits into from Dec 28, 2023

Conversation

cbravobernal
Copy link
Contributor

@cbravobernal cbravobernal commented Nov 19, 2023

What?

Use render_block_dataand render_blockfilters to differentiate between interactive, non-interactive children of an interactive block and interactive children of an interactive block to minimize the directive processing as much as possible.

Then it stacks the non interactive parts and process only interactive ones. We are replacing interactive inner blocks by a custom tag. Processing them (as they may need data from their parents) and later replacing again the processed inner blocks content by their respective tags. This is done due to how the HTML Tag Processor iterates through every tag present in the HTML.

  • Process $interactive_block (innerContent) to return a string like: `
    static_content... ....more _static_content
  • Start a WP_Directive_Processor instance with the previous string.
  • Process the content before . Process directives if present.
  • Process all inner blocks and save them into a variable.
  • Come back to the previous processing and finish it, starting at the end of the
  • Replace all wp-inner-blocks tags with their processed content, respectively.

Why?

The fewer HTML elements we process, the more performant it will be.

Copy link

github-actions bot commented Nov 19, 2023

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
❔ lib/experimental/interactivity-api/directives/wp-style.php
❔ phpunit/experimental/interactivity-api/directive-processing-test.php
❔ phpunit/experimental/interactivity-api/directives/wp-style-test.php

@cbravobernal cbravobernal added [Type] Enhancement A suggestion for improvement. [Feature] Interactivity API API to add frontend interactivity to blocks. [Packages] Interactivity /packages/interactivity labels Nov 19, 2023
@cbravobernal cbravobernal self-assigned this Nov 19, 2023
Copy link
Member

@luisherranz luisherranz left a comment

Choose a reason for hiding this comment

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

Great work 🙂

Next step: start integrating this with the HTML API 🎉

@cbravobernal cbravobernal force-pushed the update/interactivity-mark-interactive-parts branch from 182e6ba to ca33458 Compare November 28, 2023 20:45
@cbravobernal cbravobernal marked this pull request as ready for review November 29, 2023 10:05
@cbravobernal cbravobernal marked this pull request as draft November 29, 2023 16:48
@cbravobernal cbravobernal force-pushed the update/interactivity-mark-interactive-parts branch from dbb736b to e0b8edc Compare December 1, 2023 15:05
Copy link

github-actions bot commented Dec 1, 2023

Flaky tests detected in 22f6131.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7330411357
📝 Reported issues:

@cbravobernal cbravobernal force-pushed the update/interactivity-mark-interactive-parts branch 2 times, most recently from 16e5150 to 1ed8b04 Compare December 4, 2023 17:18
@cbravobernal cbravobernal force-pushed the update/interactivity-mark-interactive-parts branch from 1ed8b04 to af9e04c Compare December 11, 2023 09:56
@cbravobernal cbravobernal force-pushed the update/interactivity-mark-interactive-parts branch from af9e04c to a3f5aa7 Compare December 13, 2023 23:27
@cbravobernal cbravobernal changed the title Server directive processing: Differentiate between interactive and non interactive parts. Server directive processing: Stop processing non-interactive blocks Dec 19, 2023
@cbravobernal cbravobernal marked this pull request as ready for review December 19, 2023 10:43
Copy link
Member

@luisherranz luisherranz left a comment

Choose a reason for hiding this comment

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

Nice work!! 🎉

Codewise everything looks great, although I haven't looked at the tests yet because they are failing in my local environment due to some HTML differences.

I tried running npx wp-env start --update but it didn't fix the issue. I could look into it, but I also think that this way of testing is not very reliable and could break in the future when something unrelated changes (i.e., a change in the class names of the group block).

I think we should switch to the HTML Tag Processor and inspect only the tags that we are interested in. To do so, I think the easiest way would be to use data-wp-bind instead of data-wp-text, and add some custom class names to the custom blocks.

Imagine the interactive blocks with something like this:

<div
  data-wp-interactive='{"namespace": "level-1"}'
  data-wp-context='{"text": "level-1" }'
>
  <input class="level-1-input-1" data-wp-bind--value="context.text">
  <wp-inner-blocks></wp-inner-blocks>
  <input class="level-1-input-2" data-wp-bind--value="context.text">
</div>;

The testing could be something like this, only checking the specific attributes of the specific tags:

$post_content    = '...';
$rendered_blocks = do_blocks( $post_content );
$p = new WP_HTML_Tag_Processor( $rendered_blocks );

$p->next_tag( array( 'class' => 'level-1-input-1' ) );
$this->assertSame( "level-1", $p->get_attribute( 'value' ) );

$p->next_tag( array( 'class' => 'level-1-input-2' ) );
$this->assertSame( "level-1", $p->get_attribute( 'value' ) );

@cbravobernal cbravobernal force-pushed the update/interactivity-mark-interactive-parts branch from ec22d09 to 4843cb7 Compare December 26, 2023 14:34
@cbravobernal
Copy link
Contributor Author

I updated the branck with trunk and had to cherry pick @DAreRodz and @luisherranz commits. All tests are green now!

Copy link
Member

@luisherranz luisherranz left a comment

Choose a reason for hiding this comment

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

Ok, I think this is good enough now 🙂

Let's merge!

@luisherranz luisherranz enabled auto-merge (squash) December 28, 2023 10:33
@luisherranz luisherranz merged commit 1f9b753 into trunk Dec 28, 2023
55 checks passed
@luisherranz luisherranz deleted the update/interactivity-mark-interactive-parts branch December 28, 2023 12:24
@github-actions github-actions bot added this to the Gutenberg 17.5 milestone Dec 28, 2023
@getdave getdave added the Needs PHP backport Needs PHP backport to Core label Jan 15, 2024
@youknowriad
Copy link
Contributor

Has this been backported to Core already?

@luisherranz luisherranz added Backported to WP Core Pull request that has been successfully merged into WP Core and removed Needs PHP backport Needs PHP backport to Core labels Feb 8, 2024
@luisherranz
Copy link
Member

Yesss.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backported to WP Core Pull request that has been successfully merged into WP Core [Feature] Interactivity API API to add frontend interactivity to blocks. [Packages] Interactivity /packages/interactivity [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants