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

DAreRodz · 2023-06-19T09:01:06Z

What?

Follow-up of #51229. Small bunch of code improvements following @dmsnell's feedback provided in that PR.

Tracking issue: #51056

Why?

To have a better, beautiful code. ✨

How?

Following the suggestions @dmsnell made in #51229.

github-actions · 2023-06-19T10:09:59Z

Flaky tests detected in ae67933.
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/5310182967
📝 Reported issues:

[Flaky Test] shouldn't conflict with server-side autosave #35176 in specs/editor/various/autosave.test.js

dmsnell · 2023-06-19T10:16:28Z

lib/experimental/interactivity-api/directive-processing.php

- 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


tiny nitpick: "ignored in on purpose"

Fixed. ✅ Thanks for the review, Dennis. 😄

dmsnell

Noted a typo, but this reads clearer than before, and I think it's good that values which might be common (e.g. "file", "system", "date", "current") are not called.

If you tested and it works, and nobody else objects, this seems good to go. (Though you might want to correct the typo first)

luisherranz

LGTM as well! Thanks for taking care of this, David.

luisherranz · 2023-06-20T07:51:20Z

lib/experimental/interactivity-api/directives/wp-context.php

- // TODO: Error handling.
+ if ( null === $new_context ) {
+ // Invalid JSON defined in the directive.
+ return;
+ }


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

DAreRodz · 2023-06-20T09:43:51Z

The Playwright 1 tests are consistently failing. Did I break something? 👀

dmsnell · 2023-06-20T10:02:01Z

Not sure @DAreRodz - I didn't see universal failure for these tests in the repo, but it does seem odd. These are related to the navigation blocks - could we be relying on the is_callable() somewhere that didn't get updated to a Closure?

luisherranz · 2023-06-20T10:17:27Z

could we be relying on the is_callable() somewhere that didn't get updated to a Closure?

It doesn't use is_callable. But maybe there's something broken because Playwright 1 contains the tests of the Navigation block (and Lightbox?), which is using directives.

DAreRodz · 2023-06-20T11:17:09Z

I see some of the tests are also failing on trunk:

> gutenberg@16.0.0 test:e2e:playwright /Users/darerodz/Code/gutenberg
> playwright test --config test/e2e/playwright.config.ts "navigation.spec.js" "-g" "Frontend interactivity"


Running 4 tests using 1 worker

  ✓  1 …on block - Frontend interactivity › Overlay menu › Overlay menu interactions (10.8s)
  ✘  2 …teractivity › Submenu mouse and keyboard interactions › Submenu interactions (11.1s)
  ✓  3 …nteractivity › Submenus (Arrow setting) › submenu opens on click in the arrow (5.9s)
  ✘  4 …ontend interactivity › Page list block › page-list submenu user interactions (11.6s)

luisherranz · 2023-06-20T11:32:52Z

What about the interactivity branch? (this PR branches from interactivity)

DAreRodz · 2023-06-20T11:47:08Z

What about the interactivity branch? (this PR branches from interactivity)

Running 4 tests using 1 worker

  ✘  1 …on block - Frontend interactivity › Overlay menu › Overlay menu interactions (11.8s)
  ✘  2 …teractivity › Submenu mouse and keyboard interactions › Submenu interactions (11.6s)
  ✓  3 …nteractivity › Submenus (Arrow setting) › submenu opens on click in the arrow (5.9s)
  ✘  4 …ontend interactivity › Page list block › page-list submenu user interactions (11.5s)

luisherranz · 2023-06-20T13:21:51Z

Oh, you merged trunk in interactivity last week. So maybe those problems come from trunk indeed.

luisherranz · 2023-06-20T16:24:34Z

I've merged trunk in interactivity to see if the tests pass there:

Create @wordpress/interactivity with the Interactivity API #50906

dmsnell · 2023-06-21T10:34:49Z

It doesn't use is_callable.

Right, I didn't mean to communicate it is; I was just guessing that maybe something in the other code was depending on that behavior, passing something like array( 'WP_Directive_Processor', 'handle_show' ) or something that needs to be updated to a Closure and was missed.

Given that these are failing in trunk this is ruled out anyway, so it doesn't matter.

Maybe some timing issue with the lightbox opening? We didn't merge over failing tests before did we? So maybe they were flakey at first but now are failing reliably?

luisherranz · 2023-06-21T15:32:09Z

It looks like it was a problem in the interactivity branch (#50906). I've already fixed it and once the tests finish, I'll run these ones again to see if everything is back to green.

luisherranz · 2023-06-22T09:04:56Z

Playwright - 1 tests still fail in this branch, so we need to take a look at it.

DAreRodz · 2023-06-22T09:35:38Z

Playwright - 1 tests still fail in this branch, so we need to take a look at it.

I'm on it 🕵️

DAreRodz · 2023-06-22T11:20:23Z

It seems that the failing tests were flaky tests also failing in trunk. I created a PR to fix them.

Fix flaky tests in navigation.spec.js and other tests related to the Post Editor Template mode #51790

…ents

luisherranz · 2023-06-23T08:25:14Z

All the e2e tests pass now. Thanks for your work, @DAreRodz! Feel free to merge 🙂

* Start with package.json and README * Add new package to docs/manifest.json * Copy-paste runtime files from block-library * Add .npmrc file * Add interactivity package to dependencies * Create a custom webpack config for interactivity * Expose interactivity runtime in `wp.interactivity` * Update package-lock * Add `@wordpress/interactivity` to block-library deps * Rename entry point to index * Remove vendors chunk * Add oddly required aliases * Add view prefix to interactivity.js files * Use view-interactivity files when enabled * Stop adding defer to Interactivity scripts * Remove webpack config for interactivity.js files * Remove interactivity runtime from block-library * Remove interactivity runtime from sideEffects * Undo temporary fix for Interactivity API in dependency-extraction-webpack-plugin * Remove script loader for Interactivity API runtime * Remove block-librar/interactivity from build_files * Add src/index.js to Interactivity API entry file * Remove unnecessary aliases * Restore data-wp-body directive * Interactivity API: add `wp_store` (#51191) * Add `wp_store` to the Interactivity API * Rename WP_Interactivity_Store and move filter to scripts file * Remove todos to change the store id * Rename syntax to -- and data-wp-interactive (#51241) * Interactivity API: initial support for SSR (#51229) * Initial version working with basic support for wp-bind * Add wp-context * Add wp-class * Add wp-style * Add wp-text * Add directive processing tests * Add WP_Directive_Processor class tests * Add wp-bind tests * Add wp-context tests * Add wp-class tests * Add wp-style tests * Add wp-text tests * Add evaluate tests * Fix PHP lint * Prevent errors with incorrect JSON objects * Add support for functions in the server * Remove require for missing script-loader.php * Remove missing PHP file * Rename view file and fix block.json * Add "interactivity" to supports and fix renaming * Code improvements for the SSR part of the Interactivity API (#51640) * Fix multi-line comments and add examples * Add parse_attribute_name static method to WP_Directive_Processor * Replace array functions with a foreach loop * Add explanatory comment for the negation operator check * Replace $array with $path_segments * Minor fix for the negation operator comment * Call only instances of Closure * Improve negation operator code style * Do not lower-case tags * Use static parse_attribute_name inside directive processors * Add basic error handling in wp-context * Fix hidden identation errors * Use the correct variable name * Fix test for evaluating functions * Remove references to "attribute" directives * Remove emtpy lines in multi-line function calls * Fix typo --------- Co-authored-by: Luis Herranz <luisherranz@gmail.com> * Add the full Interactivity API runtime (but removing the client-side navigation). (#51194) * Add show and text directives * Move directive bind tests * Move the rest of e2e tests (except csn-related) * Add interactive-blocks plugin for e2e tests * Move test plugins one folder up * Add plugin to .wp-env.json * Change directive-bind spec file to use new plugin * Move plugin to e2e-tests package * Move HTML for directive-bind to plugin * Update exposed properties from preact * Refactor directive-bind spec file * Create directive-effect block for e2e testing * Update directive-effect spec file * Remove unnecessary files * Fix e2e tests for bind and effect directives * Refactor fixtures and use them for bind and effect * Remove unnecessary editorScript * Fix e2e test for directive priorities * Remove unnecessary files * Fix negation operator * Refactor store-tag e2e tests * Refactor directive-class e2e tests * Remove extra spaces * Add util for removing all created posts * Add block for context directive * Add block for directive show testing * Remove unintentionally added artifact * Ignore artifacts generated inside /test/e2e * Remove unused html * Add block for directive text testing * Add blocks for tovdom testing * Update directives syntax in e2e tests * Add getLink to InteractivityUtils * Fix php lint errors * Add disable_directives_ssr param * Fix phpcs errors * Fix missing phpcs error and warnings * Remove `wp-interactivity` from `viewScript` --------- Co-authored-by: Luis Herranz <luisherranz@gmail.com> * Remove custom watchOptions in interactivity webpack config * Update version and description of interactivity package --------- Co-authored-by: Luis Herranz <luisherranz@gmail.com>

…#51962) * Start with package.json and README * Add new package to docs/manifest.json * Copy-paste runtime files from block-library * Add .npmrc file * Add interactivity package to dependencies * Create a custom webpack config for interactivity * Expose interactivity runtime in `wp.interactivity` * Update package-lock * Add `@wordpress/interactivity` to block-library deps * Rename entry point to index * Remove vendors chunk * Add oddly required aliases * Add view prefix to interactivity.js files * Use view-interactivity files when enabled * Stop adding defer to Interactivity scripts * Remove webpack config for interactivity.js files * Remove interactivity runtime from block-library * Remove interactivity runtime from sideEffects * Undo temporary fix for Interactivity API in dependency-extraction-webpack-plugin * Remove script loader for Interactivity API runtime * Remove block-librar/interactivity from build_files * Add src/index.js to Interactivity API entry file * Remove unnecessary aliases * Restore data-wp-body directive * Interactivity API: add `wp_store` (#51191) * Add `wp_store` to the Interactivity API * Rename WP_Interactivity_Store and move filter to scripts file * Remove todos to change the store id * Rename syntax to -- and data-wp-interactive (#51241) * Interactivity API: initial support for SSR (#51229) * Initial version working with basic support for wp-bind * Add wp-context * Add wp-class * Add wp-style * Add wp-text * Add directive processing tests * Add WP_Directive_Processor class tests * Add wp-bind tests * Add wp-context tests * Add wp-class tests * Add wp-style tests * Add wp-text tests * Add evaluate tests * Fix PHP lint * Prevent errors with incorrect JSON objects * Add support for functions in the server * Remove require for missing script-loader.php * Remove missing PHP file * Rename view file and fix block.json * Add "interactivity" to supports and fix renaming * Code improvements for the SSR part of the Interactivity API (#51640) * Fix multi-line comments and add examples * Add parse_attribute_name static method to WP_Directive_Processor * Replace array functions with a foreach loop * Add explanatory comment for the negation operator check * Replace $array with $path_segments * Minor fix for the negation operator comment * Call only instances of Closure * Improve negation operator code style * Do not lower-case tags * Use static parse_attribute_name inside directive processors * Add basic error handling in wp-context * Fix hidden identation errors * Use the correct variable name * Fix test for evaluating functions * Remove references to "attribute" directives * Remove emtpy lines in multi-line function calls * Fix typo --------- Co-authored-by: Luis Herranz <luisherranz@gmail.com> * Add the full Interactivity API runtime (but removing the client-side navigation). (#51194) * Add show and text directives * Move directive bind tests * Move the rest of e2e tests (except csn-related) * Add interactive-blocks plugin for e2e tests * Move test plugins one folder up * Add plugin to .wp-env.json * Change directive-bind spec file to use new plugin * Move plugin to e2e-tests package * Move HTML for directive-bind to plugin * Update exposed properties from preact * Refactor directive-bind spec file * Create directive-effect block for e2e testing * Update directive-effect spec file * Remove unnecessary files * Fix e2e tests for bind and effect directives * Refactor fixtures and use them for bind and effect * Remove unnecessary editorScript * Fix e2e test for directive priorities * Remove unnecessary files * Fix negation operator * Refactor store-tag e2e tests * Refactor directive-class e2e tests * Remove extra spaces * Add util for removing all created posts * Add block for context directive * Add block for directive show testing * Remove unintentionally added artifact * Ignore artifacts generated inside /test/e2e * Remove unused html * Add block for directive text testing * Add blocks for tovdom testing * Update directives syntax in e2e tests * Add getLink to InteractivityUtils * Fix php lint errors * Add disable_directives_ssr param * Fix phpcs errors * Fix missing phpcs error and warnings * Remove `wp-interactivity` from `viewScript` --------- Co-authored-by: Luis Herranz <luisherranz@gmail.com> * Check that modal exists before using `contains` --------- Co-authored-by: David Arenas <david.arenas@automattic.com>

…#50906) * Start with package.json and README * Add new package to docs/manifest.json * Copy-paste runtime files from block-library * Add .npmrc file * Add interactivity package to dependencies * Create a custom webpack config for interactivity * Expose interactivity runtime in `wp.interactivity` * Update package-lock * Add `@wordpress/interactivity` to block-library deps * Rename entry point to index * Remove vendors chunk * Add oddly required aliases * Add view prefix to interactivity.js files * Use view-interactivity files when enabled * Stop adding defer to Interactivity scripts * Remove webpack config for interactivity.js files * Remove interactivity runtime from block-library * Remove interactivity runtime from sideEffects * Undo temporary fix for Interactivity API in dependency-extraction-webpack-plugin * Remove script loader for Interactivity API runtime * Remove block-librar/interactivity from build_files * Add src/index.js to Interactivity API entry file * Remove unnecessary aliases * Restore data-wp-body directive * Interactivity API: add `wp_store` (WordPress#51191) * Add `wp_store` to the Interactivity API * Rename WP_Interactivity_Store and move filter to scripts file * Remove todos to change the store id * Rename syntax to -- and data-wp-interactive (WordPress#51241) * Interactivity API: initial support for SSR (WordPress#51229) * Initial version working with basic support for wp-bind * Add wp-context * Add wp-class * Add wp-style * Add wp-text * Add directive processing tests * Add WP_Directive_Processor class tests * Add wp-bind tests * Add wp-context tests * Add wp-class tests * Add wp-style tests * Add wp-text tests * Add evaluate tests * Fix PHP lint * Prevent errors with incorrect JSON objects * Add support for functions in the server * Remove require for missing script-loader.php * Remove missing PHP file * Rename view file and fix block.json * Add "interactivity" to supports and fix renaming * Code improvements for the SSR part of the Interactivity API (WordPress#51640) * Fix multi-line comments and add examples * Add parse_attribute_name static method to WP_Directive_Processor * Replace array functions with a foreach loop * Add explanatory comment for the negation operator check * Replace $array with $path_segments * Minor fix for the negation operator comment * Call only instances of Closure * Improve negation operator code style * Do not lower-case tags * Use static parse_attribute_name inside directive processors * Add basic error handling in wp-context * Fix hidden identation errors * Use the correct variable name * Fix test for evaluating functions * Remove references to "attribute" directives * Remove emtpy lines in multi-line function calls * Fix typo --------- Co-authored-by: Luis Herranz <luisherranz@gmail.com> * Add the full Interactivity API runtime (but removing the client-side navigation). (WordPress#51194) * Add show and text directives * Move directive bind tests * Move the rest of e2e tests (except csn-related) * Add interactive-blocks plugin for e2e tests * Move test plugins one folder up * Add plugin to .wp-env.json * Change directive-bind spec file to use new plugin * Move plugin to e2e-tests package * Move HTML for directive-bind to plugin * Update exposed properties from preact * Refactor directive-bind spec file * Create directive-effect block for e2e testing * Update directive-effect spec file * Remove unnecessary files * Fix e2e tests for bind and effect directives * Refactor fixtures and use them for bind and effect * Remove unnecessary editorScript * Fix e2e test for directive priorities * Remove unnecessary files * Fix negation operator * Refactor store-tag e2e tests * Refactor directive-class e2e tests * Remove extra spaces * Add util for removing all created posts * Add block for context directive * Add block for directive show testing * Remove unintentionally added artifact * Ignore artifacts generated inside /test/e2e * Remove unused html * Add block for directive text testing * Add blocks for tovdom testing * Update directives syntax in e2e tests * Add getLink to InteractivityUtils * Fix php lint errors * Add disable_directives_ssr param * Fix phpcs errors * Fix missing phpcs error and warnings * Remove `wp-interactivity` from `viewScript` --------- Co-authored-by: Luis Herranz <luisherranz@gmail.com> * Remove custom watchOptions in interactivity webpack config * Update version and description of interactivity package --------- Co-authored-by: Luis Herranz <luisherranz@gmail.com>

DAreRodz added 15 commits June 16, 2023 12:30

Fix multi-line comments and add examples

f3cac84

Add parse_attribute_name static method to WP_Directive_Processor

250e7b1

Replace array functions with a foreach loop

bd8e120

Add explanatory comment for the negation operator check

92a2b6f

Replace $array with $path_segments

f3a59a2

Minor fix for the negation operator comment

7e02740

Call only instances of Closure

8a1795e

Improve negation operator code style

0f4c0f7

Do not lower-case tags

29619eb

Use static parse_attribute_name inside directive processors

5c651e1

Add basic error handling in wp-context

fc7ab88

Fix hidden identation errors

10abf30

Use the correct variable name

364e073

Fix test for evaluating functions

b0252c3

Remove references to "attribute" directives

63fe0ff

DAreRodz added [Type] Enhancement A suggestion for improvement. [Type] Experimental Experimental feature or API. [Feature] Interactivity API API to add frontend interactivity to blocks. labels Jun 19, 2023

DAreRodz requested review from luisherranz and dmsnell June 19, 2023 09:01

DAreRodz requested a review from spacedmonkey as a code owner June 19, 2023 09:01

Remove emtpy lines in multi-line function calls

ae67933

dmsnell reviewed Jun 19, 2023

View reviewed changes

dmsnell approved these changes Jun 19, 2023

View reviewed changes

Fix typo

06620e9

luisherranz approved these changes Jun 20, 2023

View reviewed changes

Merge branch 'interactivity' into interactivity-api-ssr-code-improvem…

9e28311

…ents

DAreRodz merged commit 8758865 into interactivity Jun 23, 2023
50 checks passed

DAreRodz deleted the interactivity-api-ssr-code-improvements branch June 23, 2023 14:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

DAreRodz commented Jun 19, 2023

github-actions bot commented Jun 19, 2023

dmsnell Jun 19, 2023

DAreRodz Jun 19, 2023

dmsnell left a comment

luisherranz left a comment •

edited

Loading

luisherranz Jun 20, 2023

DAreRodz commented Jun 20, 2023 •

edited

Loading

dmsnell commented Jun 20, 2023

luisherranz commented Jun 20, 2023 •

edited

Loading

DAreRodz commented Jun 20, 2023

luisherranz commented Jun 20, 2023 •

edited

Loading

DAreRodz commented Jun 20, 2023

luisherranz commented Jun 20, 2023

luisherranz commented Jun 20, 2023 •

edited

Loading

dmsnell commented Jun 21, 2023

luisherranz commented Jun 21, 2023

luisherranz commented Jun 22, 2023

DAreRodz commented Jun 22, 2023

DAreRodz commented Jun 22, 2023

luisherranz commented Jun 23, 2023

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

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

Conversation

DAreRodz commented Jun 19, 2023

What?

Why?

How?

github-actions bot commented Jun 19, 2023

dmsnell Jun 19, 2023

Choose a reason for hiding this comment

DAreRodz Jun 19, 2023

Choose a reason for hiding this comment

dmsnell left a comment

Choose a reason for hiding this comment

luisherranz left a comment • edited Loading

Choose a reason for hiding this comment

luisherranz Jun 20, 2023

Choose a reason for hiding this comment

DAreRodz commented Jun 20, 2023 • edited Loading

dmsnell commented Jun 20, 2023

luisherranz commented Jun 20, 2023 • edited Loading

DAreRodz commented Jun 20, 2023

luisherranz commented Jun 20, 2023 • edited Loading

DAreRodz commented Jun 20, 2023

luisherranz commented Jun 20, 2023

luisherranz commented Jun 20, 2023 • edited Loading

dmsnell commented Jun 21, 2023

luisherranz commented Jun 21, 2023

luisherranz commented Jun 22, 2023

DAreRodz commented Jun 22, 2023

DAreRodz commented Jun 22, 2023

luisherranz commented Jun 23, 2023

luisherranz left a comment •

edited

Loading

DAreRodz commented Jun 20, 2023 •

edited

Loading

luisherranz commented Jun 20, 2023 •

edited

Loading

luisherranz commented Jun 20, 2023 •

edited

Loading

luisherranz commented Jun 20, 2023 •

edited

Loading