Interactivity API: Fix null and number strings as namespaces runtime …

…error. (WordPress#61960) * Fix null and number strings as namespaces runtime error * Update with suggestions * Update changelog * added wrong css changes * Revert move isObject to utils * Rename url vars Co-authored-by: cbravobernal <[email protected]> Co-authored-by: sirreal <[email protected]>
carstingaxion · Jun 4, 2024 · bccf48e · bccf48e
1 parent 4a7bf32
commit bccf48e
Show file tree

Hide file tree

Showing 6 changed files with 130 additions and 10 deletions.
diff --git a/packages/e2e-tests/plugins/interactive-blocks/namespace/render.php b/packages/e2e-tests/plugins/interactive-blocks/namespace/render.php
@@ -18,6 +18,28 @@
 	<a data-wp-bind--href="state.url" data-testid="object namespace"></a>
 </div>
 
+<div data-wp-interactive="null">
+	<a data-wp-bind--href="state.url" data-testid="null namespace"></a>
+</div>
+
+<div data-wp-interactive="2">
+	<a data-wp-bind--href="state.url" data-testid="number namespace"></a>
+</div>
+
 <div data-wp-interactive>
 	<a data-wp-bind--href="other::state.url" data-testid="other namespace"></a>
 </div>
+
+<div data-wp-interactive="true">
+	<a data-wp-bind--href="state.url" data-testid="true namespace"></a>
+</div>
+
+<div data-wp-interactive="false">
+	<a data-wp-bind--href="state.url" data-testid="false namespace"></a>
+</div>
+<div data-wp-interactive="[]">
+	<a data-wp-bind--href="state.url" data-testid="[] namespace"></a>
+</div>
+<div data-wp-interactive='"quoted string"'>
+	<a data-wp-bind--href="state.url" data-testid="quoted namespace"></a>
+</div>
diff --git a/packages/e2e-tests/plugins/interactive-blocks/namespace/view.js b/packages/e2e-tests/plugins/interactive-blocks/namespace/view.js
@@ -3,9 +3,16 @@
  */
 import { store } from '@wordpress/interactivity';
 
+
+store( '', {
+	state: {
+		url: '/empty-string-url',
+	},
+} );
+
 store( 'namespace', {
 	state: {
-		url: '/some-url',
+		url: '/namespace-url',
 	},
 } );
 
@@ -14,3 +21,50 @@ store( 'other', {
 		url: '/other-store-url',
 	},
 } );
+
+store( 'null', {
+	state: {
+		url: '/null-url',
+	},
+} );
+
+store( '2', {
+	state: {
+		url: '/number-url',
+	},
+} );
+
+store( '{}', {
+	state: {
+		url: '/object-url',
+	},
+} );
+
+store( 'true', {
+	state: {
+		url: '/true-url',
+	},
+} );
+
+store( 'false', {
+	state: {
+		url: '/false-url',
+	},
+} );
+
+store( '[]', {
+	state: {
+		url: '/array-url',
+	},
+} );
+
+store( '"quoted string"', {
+	state: {
+		url: '/quoted-url',
+	},
+} );
+
+
+
+
+
diff --git a/packages/interactivity/CHANGELOG.md b/packages/interactivity/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### Bug fixes
+
+-	Fix null and number strings as namespaces runtime error. ([#61960](https://github.com/WordPress/gutenberg/pull/61960/))
+
 ### Breaking Changes
 
 -   Variables like `process.env.IS_GUTENBERG_PLUGIN` have been replaced by `globalThis.IS_GUTENBERG_PLUGIN`. Build systems using `process.env` should be updated ([#61486](https://github.com/WordPress/gutenberg/pull/61486)).

diff --git a/packages/interactivity/src/store.ts b/packages/interactivity/src/store.ts
@@ -15,7 +15,6 @@ import {
 	setNamespace,
 	resetNamespace,
 } from './hooks';
-
 const isObject = ( item: unknown ): item is Record< string, unknown > =>
 	Boolean( item && typeof item === 'object' && item.constructor === Object );
 

diff --git a/packages/interactivity/src/vdom.ts b/packages/interactivity/src/vdom.ts
@@ -13,6 +13,8 @@ const islandAttr = `data-${ p }-interactive`;
 const fullPrefix = `data-${ p }-`;
 const namespaces: Array< string | null > = [];
 const currentNamespace = () => namespaces[ namespaces.length - 1 ] ?? null;
+const isObject = ( item: unknown ): item is Record< string, unknown > =>
+	Boolean( item && typeof item === 'object' && item.constructor === Object );
 
 // Regular expression for directive parsing.
 const directiveParser = new RegExp(
@@ -100,8 +102,9 @@ export function toVdom( root: Node ): Array< ComponentChild > {
 					const namespace = regexResult?.[ 1 ] ?? null;
 					let value: any = regexResult?.[ 2 ] ?? attributeValue;
 					try {
-						value = value && JSON.parse( value );
-					} catch ( e ) {}
+						const parsedValue = JSON.parse( value );
+						value = isObject( parsedValue ) ? parsedValue : value;
+					} catch {}
 					if ( attributeName === islandAttr ) {
 						island = true;
 						const islandNamespace =

diff --git a/test/e2e/specs/interactivity/namespace.spec.ts b/test/e2e/specs/interactivity/namespace.spec.ts
@@ -20,30 +20,68 @@ test.describe( 'Namespaces', () => {
 
 	test( 'Empty string as namespace should not work', async ( { page } ) => {
 		const el = page.getByTestId( 'empty namespace' );
-		await expect( el ).not.toHaveAttribute( 'href', '/some-url' );
+		await expect( el ).not.toHaveAttribute( 'href', '/empty-string-url' );
 	} );
 
 	test( 'A string as namespace should work', async ( { page } ) => {
 		const el = page.getByTestId( 'correct namespace' );
-		await expect( el ).toHaveAttribute( 'href', '/some-url' );
+		await expect( el ).toHaveAttribute( 'href', '/namespace-url' );
 	} );
 
-	test( 'An empty object as namespace should work', async ( { page } ) => {
+	test( 'An empty object as namespace should not work', async ( {
+		page,
+	} ) => {
 		const el = page.getByTestId( 'object namespace' );
-		await expect( el ).not.toHaveAttribute( 'href', '/some-url' );
+		await expect( el ).not.toHaveAttribute( 'href', '/object-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' );
+		await expect( el ).not.toHaveAttribute( 'href', '/namespace-url' );
 		const correct = page.getByTestId( 'correct namespace' );
-		await expect( correct ).toHaveAttribute( 'href', '/some-url' );
+		await expect( correct ).toHaveAttribute( 'href', '/namespace-url' );
 	} );
 
 	test( 'A different store namespace should work', async ( { page } ) => {
 		const el = page.getByTestId( 'other namespace' );
 		await expect( el ).toHaveAttribute( 'href', '/other-store-url' );
 	} );
+
+	test( 'A number as a string as namespace should work', async ( {
+		page,
+	} ) => {
+		const el = page.getByTestId( 'number namespace' );
+		await expect( el ).toHaveAttribute( 'href', '/number-url' );
+	} );
+
+	test( 'A null as a string as namespace should work', async ( { page } ) => {
+		const el = page.getByTestId( 'null namespace' );
+		await expect( el ).toHaveAttribute( 'href', '/null-url' );
+	} );
+
+	test( 'A true as a string as namespace should work', async ( { page } ) => {
+		const el = page.getByTestId( 'true namespace' );
+		await expect( el ).toHaveAttribute( 'href', '/true-url' );
+	} );
+
+	test( 'A false as a string as namespace should work', async ( {
+		page,
+	} ) => {
+		const el = page.getByTestId( 'false namespace' );
+		await expect( el ).toHaveAttribute( 'href', '/false-url' );
+	} );
+
+	test( 'A [] as a string as namespace should work', async ( { page } ) => {
+		const el = page.getByTestId( '[] namespace' );
+		await expect( el ).toHaveAttribute( 'href', '/array-url' );
+	} );
+
+	test( 'A "quoted string" as a string as namespace should work', async ( {
+		page,
+	} ) => {
+		const el = page.getByTestId( 'quoted namespace' );
+		await expect( el ).toHaveAttribute( 'href', '/quoted-url' );
+	} );
 } );