Skip to content

Commit

Permalink
Interactivity API: Fix null and number strings as namespaces runtime …
Browse files Browse the repository at this point in the history
…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 <cbravobernal@git.wordpress.org>
Co-authored-by: sirreal <jonsurrell@git.wordpress.org>
  • Loading branch information
3 people authored and patil-vipul committed Jun 17, 2024
1 parent 77c09ad commit d43d98b
Show file tree
Hide file tree
Showing 6 changed files with 130 additions and 10 deletions.
22 changes: 22 additions & 0 deletions packages/e2e-tests/plugins/interactive-blocks/namespace/render.php
Original file line number Diff line number Diff line change
Expand Up @@ -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>
56 changes: 55 additions & 1 deletion packages/e2e-tests/plugins/interactive-blocks/namespace/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,16 @@
*/
import { store } from '@wordpress/interactivity';


store( '', {
state: {
url: '/empty-string-url',
},
} );

store( 'namespace', {
state: {
url: '/some-url',
url: '/namespace-url',
},
} );

Expand All @@ -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',
},
} );





4 changes: 4 additions & 0 deletions packages/interactivity/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)).
Expand Down
1 change: 0 additions & 1 deletion packages/interactivity/src/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 );

Expand Down
7 changes: 5 additions & 2 deletions packages/interactivity/src/vdom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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 =
Expand Down
50 changes: 44 additions & 6 deletions test/e2e/specs/interactivity/namespace.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' );
} );
} );

0 comments on commit d43d98b

Please sign in to comment.