Interactivity API: Prevent wrong written directives from killing the runtime

[cbravobernal]

What?

Prevent wrong written directives killing the runtime of the Interactivity API.

Why?

A directive like data-wp-on-window-- can kill the runtime process in the Interactivity API.

How?

Refactored the code that gets the directives and parse them.

Testing Instructions

1. Create an Interactive block with a wrong directive like data-wp-on-document--
2. Check that the rest of directives are processed.

Screenshots or screencast

Before:

After:

[github-actions]

Flaky tests detected in 03c093d.
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/8895797255
📝 Reported issues:

• [Flaky Test] Redirection after template creation #60678 in /test/e2e/specs/site-editor/site-editor-url-navigation.spec.js

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: cbravobernal <cbravobernal@git.wordpress.org>
Co-authored-by: sirreal <jonsurrell@git.wordpress.org>
Co-authored-by: DAreRodz <darerodz@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[sirreal]

Should we update the regexp instead so it doesn't match the bad suffix, requiring an alphanumeric character to start?

gutenberg/packages/interactivity/src/vdom.ts

Lines 16 to 28 in 204c420

	// Regular expression for directive parsing.
	const directiveParser = new RegExp(
	`^data-${ p }-` + // ${p} must be a prefix string, like 'wp'.
	// Match alphanumeric characters including hyphen-separated
	// segments. It excludes underscore intentionally to prevent confusion.
	// E.g., "custom-directive".
	'([a-z0-9]+(?:-[a-z0-9]+)*)' +
	// (Optional) Match '--' followed by any alphanumeric charachters. It
	// excludes underscore intentionally to prevent confusion, but it can
	// contain multiple hyphens. E.g., "--custom-prefix--with-more-info".
	'(?:--([a-z0-9_-]+))?$',
	'i' // Case insensitive.
	);

This line matching the suffix:

gutenberg/packages/interactivity/src/vdom.ts

Line 26 in 204c420

'(?:--([a-z0-9_-]+))?$',

Could add a required alphanumeric initial character to the suffix like this:

'(?:--([a-z0-9][a-z0-9_-]*))?$',

---

For the warning, then we could check early if .exec() returns null, show the warning, and just skip the rest of the processing for a bad directive?

[DAreRodz]

Should we update the regexp instead so it doesn't match the bad suffix, requiring an alphanumeric character to start?

Hey @sirreal, we need to support dashes at the beginning of "suffixes". 😅 For instance, the data-wp-style directive accepts CSS variables:

<button
  style="--button-color:blue;"
  data-wp-style----button-color="state.buttonColor"
>A fancy button</button>

[sirreal]

Hey @sirreal, we need to support dashes at the beginning of "suffixes". 😅 For instance, the data-wp-style directive accepts CSS variables:

Good spot 👍

[sirreal]

The bugfix is good, but I would like to see some better naming generally for the parts of a directive. It doesn't need to happen in this PR.

[sirreal]

This needs to go up under the unreleased section.

[sirreal]

This section could probably use some comments about how strings are being split up and what we expect to find in each part. We've even got a constant wp that's the directivePrefix (that also appears in the middle of the data attribute).

Honestly, some kind of description of what the parts of a directive are and a refactor to rename things accordingly would be helpful. prefix and suffix don't really make sense:

nothing?       suffix is everything after `prefix--`… I guess this makes sense
   |              |
vvvvvvv       vvvvvvvvvv   
data-wp-bind--on--change
        ^^^^
          |
      "prefix" in the middle?

Maybe we could talk about these things like the "directive name" or "directive type", then the rest of it could be some kind of input to the directive? I think we use these prefix/suffix names across the package and it seems confusing.

---

The bug fix here where we try to access a null match is good 👍