Interactivity API: Prevent empty namespace or different namespaces fr…

…om killing the runtime (#61409)

* Prevent empty namespace or different namespaces from killing the runtime

* Update changelog

* Move changelog

* Update with isdebug, rephrase error

* Prevent warning duplication

* Remove not needed check

* Move warn to same resolve function

* Add some tests

* Remove not needed warn

* Fix typos

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

packages/block-library/src/embed/util.js

@@ -177,7 +177,7 @@ export const removeAspectRatioClasses = ( existingClassNames ) => {
 	if ( ! existingClassNames ) {
 		// Avoids extraneous work and also, by returning the same value as
 		// received, ensures the post is not dirtied by a change of the block
-		// attribute from `undefined` to an emtpy string.
+		// attribute from `undefined` to an empty string.
 		return existingClassNames;
 	}
 	const aspectRatioClassNames = ASPECT_RATIOS.reduce(

packages/e2e-tests/plugins/interactive-blocks/namespace/block.json

@@ -0,0 +1,15 @@
+{
+	"$schema": "https://schemas.wp.org/trunk/block.json",
+	"apiVersion": 2,
+	"name": "test-namespace/directive-bind",
+	"title": "E2E Interactivity tests - directive bind",
+	"category": "text",
+	"icon": "heart",
+	"description": "",
+	"supports": {
+		"interactivity": true
+	},
+	"textdomain": "e2e-interactivity",
+	"viewScriptModule": "file:./view.js",
+	"render": "file:./render.php"
+}

packages/e2e-tests/plugins/interactive-blocks/namespace/render.php

@@ -0,0 +1,23 @@
+<?php
+/**
+ * HTML for testing the directive `data-wp-bind`.
+ *
+ * @package gutenberg-test-interactive-blocks
+ */
+?>
+
+<div data-wp-interactive="">
+	<a data-wp-bind--href="state.url" data-testid="empty namespace"></a>
+</div>
+
+<div data-wp-interactive="namespace">
+	<a data-wp-bind--href="state.url" data-testid="correct namespace"></a>
+</div>
+
+<div data-wp-interactive="{}">
+	<a data-wp-bind--href="state.url" data-testid="object namespace"></a>
+</div>
+
+<div data-wp-interactive>
+	<a data-wp-bind--href="other::state.url" data-testid="other namespace"></a>
+</div>

packages/e2e-tests/plugins/interactive-blocks/namespace/view.asset.php

@@ -0,0 +1 @@
+<?php return array( 'dependencies' => array( '@wordpress/interactivity' ) );

packages/e2e-tests/plugins/interactive-blocks/namespace/view.js

@@ -0,0 +1,16 @@
+/**
+ * WordPress dependencies
+ */
+import { store } from '@wordpress/interactivity';
+
+store( 'namespace', {
+	state: {
+		url: '/some-url',
+	},
+} );
+
+store( 'other', {
+	state: {
+		url: '/other-store-url',
+	},
+} );

packages/interactivity/CHANGELOG.md

@@ -7,6 +7,7 @@
 -	Allow multiple event handlers for the same type with `data-wp-on-document` and `data-wp-on-window`. ([#61009](https://github.com/WordPress/gutenberg/pull/61009))
 
 -	Prevent wrong written directives from killing the runtime ([#61249](https://github.com/WordPress/gutenberg/pull/61249))
+-	Prevent empty namespace or different namespaces from killing the runtime ([#61409](https://github.com/WordPress/gutenberg/pull/61409))
 
 ## 5.6.0 (2024-05-02)
 

packages/interactivity/src/directives.js

@@ -13,6 +13,7 @@ import { deepSignal, peek } from 'deepsignal';
 import { useWatch, useInit } from './utils';
 import { directive, getScope, getEvaluate } from './hooks';
 import { kebabToCamelCase } from './utils/kebab-to-camelcase';
+import { warn } from './utils/warn';
 
 // Assigned objects should be ignore during proxification.
 const contextAssignedObjects = new WeakMap();
@@ -242,13 +243,8 @@ export default () => {
 				if ( defaultEntry ) {
 					const { namespace, value } = defaultEntry;
 					// Check that the value is a JSON object. Send a console warning if not.
-					if (
-						typeof SCRIPT_DEBUG !== 'undefined' &&
-						SCRIPT_DEBUG === true &&
-						! isPlainObject( value )
-					) {
-						// eslint-disable-next-line no-console
-						console.warn(
+					if ( ! isPlainObject( value ) ) {
+						warn(
 							`The value of data-wp-context in "${ namespace }" store must be a valid stringified JSON object.`
 						);
 					}

packages/interactivity/src/hooks.tsx

@@ -16,6 +16,7 @@ import type { VNode, Context, RefObject } from 'preact';
  * Internal dependencies
  */
 import { store, stores, universalUnlock } from './store';
+import { warn } from './utils/warn';
 interface DirectiveEntry {
 	value: string | Object;
 	namespace: string;
@@ -260,18 +261,26 @@ export const directive = (
 
 // Resolve the path to some property of the store object.
 const resolve = ( path, namespace ) => {
+	if ( ! namespace ) {
+		warn(
+			`The "namespace" cannot be "{}", "null" or an empty string. Path: ${ path }`
+		);
+		return;
+	}
 	let resolvedStore = stores.get( namespace );
 	if ( typeof resolvedStore === 'undefined' ) {
 		resolvedStore = store( namespace, undefined, {
 			lock: universalUnlock,
 		} );
 	}
-	let current = {
+	const current = {
 		...resolvedStore,
 		context: getScope().context[ namespace ],
 	};
-	path.split( '.' ).forEach( ( p ) => ( current = current[ p ] ) );
-	return current;
+	try {
+		// TODO: Support lazy/dynamically initialized stores
+		return path.split( '.' ).reduce( ( acc, key ) => acc[ key ], current );
+	} catch ( e ) {}
 };
 
 // Generate the evaluate function.

packages/interactivity/src/utils/warn.ts

@@ -0,0 +1,21 @@
+const logged = new Set();
+
+export const warn = ( message ) => {
+	// @ts-expect-error
+	if ( typeof SCRIPT_DEBUG !== 'undefined' && SCRIPT_DEBUG === true ) {
+		if ( logged.has( message ) ) {
+			return;
+		}
+
+		// eslint-disable-next-line no-console
+		console.warn( message );
+
+		// Adding a stack trace to the warning message to help with debugging.
+		try {
+			throw Error( message );
+		} catch ( e ) {
+			// Do nothing.
+		}
+		logged.add( message );
+	}
+};

packages/interactivity/src/vdom.ts

@@ -6,6 +6,7 @@ import { h } from 'preact';
  * Internal dependencies
  */
 import { directivePrefix as p } from './constants';
+import { warn } from './utils/warn';
 
 const ignoreAttr = `data-${ p }-ignore`;
 const islandAttr = `data-${ p }-interactive`;
@@ -120,15 +121,7 @@ export function toVdom( root ) {
 				( obj, [ name, ns, value ] ) => {
 					const directiveMatch = directiveParser.exec( name );
 					if ( directiveMatch === null ) {
-						if (
-							// @ts-expect-error This is a debug-only warning.
-							typeof SCRIPT_DEBUG !== 'undefined' &&
-							// @ts-expect-error This is a debug-only warning.
-							SCRIPT_DEBUG === true
-						) {
-							// eslint-disable-next-line no-console
-							console.warn( `Invalid directive: ${ name }.` );
-						}
+						warn( `Invalid directive: ${ name }.` );
 						return obj;
 					}
 					const prefix = directiveMatch[ 1 ] || '';

test/e2e/specs/interactivity/namespace.spec.ts

@@ -0,0 +1,49 @@
+/**
+ * Internal dependencies
+ */
+import { test, expect } from './fixtures';
+
+test.describe( 'Namespaces', () => {
+	test.beforeAll( async ( { interactivityUtils: utils } ) => {
+		await utils.activatePlugins();
+		await utils.addPostWithBlock( 'test-namespace/directive-bind' );
+	} );
+
+	test.beforeEach( async ( { interactivityUtils: utils, page } ) => {
+		await page.goto( utils.getLink( 'test-namespace/directive-bind' ) );
+	} );
+
+	test.afterAll( async ( { interactivityUtils: utils } ) => {
+		await utils.deactivatePlugins();
+		await utils.deleteAllPosts();
+	} );
+
+	test( 'Empty string as namespace should not work', async ( { page } ) => {
+		const el = page.getByTestId( 'empty namespace' );
+		await expect( el ).not.toHaveAttribute( 'href', '/some-url' );
+	} );
+
+	test( 'A string as namespace should work', async ( { page } ) => {
+		const el = page.getByTestId( 'correct namespace' );
+		await expect( el ).toHaveAttribute( 'href', '/some-url' );
+	} );
+
+	test( 'An empty object as namespace should work', async ( { page } ) => {
+		const el = page.getByTestId( 'object namespace' );
+		await expect( el ).not.toHaveAttribute( 'href', '/some-url' );
+	} );
+
+	test( 'A wrong namespace should not break the runtime', async ( {
+		page,
+	} ) => {
+		const el = page.getByTestId( 'object namespace' );
+		await expect( el ).not.toHaveAttribute( 'href', '/some-url' );
+		const correct = page.getByTestId( 'correct namespace' );
+		await expect( correct ).toHaveAttribute( 'href', '/some-url' );
+	} );
+
+	test( 'A different store namespace should work', async ( { page } ) => {
+		const el = page.getByTestId( 'other namespace' );
+		await expect( el ).toHaveAttribute( 'href', '/other-store-url' );
+	} );
+} );