From f5df725630cffa7d76774cb347d57fbf5c9df1b2 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 7 Oct 2025 15:30:12 +0100 Subject: [PATCH 1/6] Fully clear binding when clearing --- packages/block-editor/README.md | 7 ++- .../block-editor/src/utils/block-bindings.js | 47 ++++++++++++++++++- .../shared/use-entity-binding.js | 22 +++++---- 3 files changed, 64 insertions(+), 12 deletions(-) diff --git a/packages/block-editor/README.md b/packages/block-editor/README.md index 8fe2c5f1179dcd..de5eed3d59b13a 100644 --- a/packages/block-editor/README.md +++ b/packages/block-editor/README.md @@ -894,13 +894,15 @@ Retrieves the existing utils needed to update the block `bindings` metadata. The It contains the following utils: - `updateBlockBindings`: Updates the value of the bindings connected to block attributes. It can be used to remove a specific binding by setting the value to `undefined`. +- `removeBlockBinding`: Removes the binding for a specific attribute. - `removeAllBlockBindings`: Removes the bindings property of the `metadata` attribute. _Usage_ ```js import { useBlockBindingsUtils } from '@wordpress/block-editor'; -const { updateBlockBindings, removeAllBlockBindings } = useBlockBindingsUtils(); +const { updateBlockBindings, removeBlockBinding, removeAllBlockBindings } = + useBlockBindingsUtils(); // Update url and alt attributes. updateBlockBindings( { @@ -921,6 +923,9 @@ updateBlockBindings( { // Remove binding from url attribute. updateBlockBindings( { url: undefined } ); +// Alternative: Remove binding from url attribute. +removeBlockBinding( 'url' ); + // Remove bindings from all attributes. removeAllBlockBindings(); ``` diff --git a/packages/block-editor/src/utils/block-bindings.js b/packages/block-editor/src/utils/block-bindings.js index 57c58560021293..0d78f8225ce8d6 100644 --- a/packages/block-editor/src/utils/block-bindings.js +++ b/packages/block-editor/src/utils/block-bindings.js @@ -73,6 +73,7 @@ export function replacePatternOverridesDefaultBinding( * @typedef {Object} WPBlockBindingsUtils * * @property {Function} updateBlockBindings Updates the value of the bindings connected to block attributes. + * @property {Function} removeBlockBinding Removes the binding for a specific attribute. * @property {Function} removeAllBlockBindings Removes the bindings property of the `metadata` attribute. */ @@ -82,6 +83,7 @@ export function replacePatternOverridesDefaultBinding( * * It contains the following utils: * - `updateBlockBindings`: Updates the value of the bindings connected to block attributes. It can be used to remove a specific binding by setting the value to `undefined`. + * - `removeBlockBinding`: Removes the binding for a specific attribute. * - `removeAllBlockBindings`: Removes the bindings property of the `metadata` attribute. * * @since 6.7.0 Introduced in WordPress core. @@ -93,7 +95,7 @@ export function replacePatternOverridesDefaultBinding( * @example * ```js * import { useBlockBindingsUtils } from '@wordpress/block-editor' - * const { updateBlockBindings, removeAllBlockBindings } = useBlockBindingsUtils(); + * const { updateBlockBindings, removeBlockBinding, removeAllBlockBindings } = useBlockBindingsUtils(); * * // Update url and alt attributes. * updateBlockBindings( { @@ -114,6 +116,9 @@ export function replacePatternOverridesDefaultBinding( * // Remove binding from url attribute. * updateBlockBindings( { url: undefined } ); * + * // Alternative: Remove binding from url attribute. + * removeBlockBinding( 'url' ); + * * // Remove bindings from all attributes. * removeAllBlockBindings(); * ``` @@ -180,6 +185,44 @@ export function useBlockBindingsUtils( clientId ) { } ); }; + /** + * Removes the binding for a specific attribute. + * + * @param {string} attribute The attribute name to remove the binding from. + * + * @example + * ```js + * import { useBlockBindingsUtils } from '@wordpress/block-editor' + * + * const { removeBlockBinding } = useBlockBindingsUtils(); + * removeBlockBinding( 'url' ); + * ``` + */ + const removeBlockBinding = ( attribute ) => { + const { metadata: { bindings, ...metadata } = {} } = + getBlockAttributes( blockClientId ); + + if ( ! bindings || ! bindings[ attribute ] ) { + return; + } + + const newBindings = { ...bindings }; + delete newBindings[ attribute ]; + + const newMetadata = { + ...metadata, + bindings: newBindings, + }; + + if ( isObjectEmpty( newMetadata.bindings ) ) { + delete newMetadata.bindings; + } + + updateBlockAttributes( blockClientId, { + metadata: isObjectEmpty( newMetadata ) ? undefined : newMetadata, + } ); + }; + /** * Removes the bindings property of the `metadata` attribute. * @@ -199,5 +242,5 @@ export function useBlockBindingsUtils( clientId ) { } ); }; - return { updateBlockBindings, removeAllBlockBindings }; + return { updateBlockBindings, removeBlockBinding, removeAllBlockBindings }; } diff --git a/packages/block-library/src/navigation-link/shared/use-entity-binding.js b/packages/block-library/src/navigation-link/shared/use-entity-binding.js index a07e77b3e84ddb..3e9b5971397887 100644 --- a/packages/block-library/src/navigation-link/shared/use-entity-binding.js +++ b/packages/block-library/src/navigation-link/shared/use-entity-binding.js @@ -16,19 +16,23 @@ import { useBlockBindingsUtils } from '@wordpress/block-editor'; * @return {Object} Hook return value */ export function useEntityBinding( { clientId, attributes } ) { - const { updateBlockBindings } = useBlockBindingsUtils( clientId ); + const { updateBlockBindings, removeBlockBinding } = + useBlockBindingsUtils( clientId ); const { metadata, id } = attributes; - const hasUrlBinding = !! metadata?.bindings?.url && !! id; + // Check if there's a URL binding with a valid source (not null) + const hasUrlBinding = !! metadata?.bindings?.url?.source && !! id; + + // Check if there's ANY binding metadata (even if cleared/null) + const hasBindingMetadata = !! metadata?.bindings?.url; const clearBinding = useCallback( () => { - updateBlockBindings( { - url: { - source: null, - args: null, - }, - } ); - }, [ updateBlockBindings ] ); + // Only clear if there's actually a binding to clear + if ( hasBindingMetadata ) { + // Remove the URL binding + removeBlockBinding( 'url' ); + } + }, [ hasBindingMetadata, removeBlockBinding ] ); const createBinding = useCallback( () => { updateBlockBindings( { From ed18184f0bec9d7944acc1be79aea197b3adb55c Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 7 Oct 2025 15:45:31 +0100 Subject: [PATCH 2/6] Go with a simpler approach --- packages/block-editor/README.md | 7 +-- .../block-editor/src/utils/block-bindings.js | 47 +------------------ .../shared/use-entity-binding.js | 9 ++-- 3 files changed, 7 insertions(+), 56 deletions(-) diff --git a/packages/block-editor/README.md b/packages/block-editor/README.md index de5eed3d59b13a..8fe2c5f1179dcd 100644 --- a/packages/block-editor/README.md +++ b/packages/block-editor/README.md @@ -894,15 +894,13 @@ Retrieves the existing utils needed to update the block `bindings` metadata. The It contains the following utils: - `updateBlockBindings`: Updates the value of the bindings connected to block attributes. It can be used to remove a specific binding by setting the value to `undefined`. -- `removeBlockBinding`: Removes the binding for a specific attribute. - `removeAllBlockBindings`: Removes the bindings property of the `metadata` attribute. _Usage_ ```js import { useBlockBindingsUtils } from '@wordpress/block-editor'; -const { updateBlockBindings, removeBlockBinding, removeAllBlockBindings } = - useBlockBindingsUtils(); +const { updateBlockBindings, removeAllBlockBindings } = useBlockBindingsUtils(); // Update url and alt attributes. updateBlockBindings( { @@ -923,9 +921,6 @@ updateBlockBindings( { // Remove binding from url attribute. updateBlockBindings( { url: undefined } ); -// Alternative: Remove binding from url attribute. -removeBlockBinding( 'url' ); - // Remove bindings from all attributes. removeAllBlockBindings(); ``` diff --git a/packages/block-editor/src/utils/block-bindings.js b/packages/block-editor/src/utils/block-bindings.js index 0d78f8225ce8d6..57c58560021293 100644 --- a/packages/block-editor/src/utils/block-bindings.js +++ b/packages/block-editor/src/utils/block-bindings.js @@ -73,7 +73,6 @@ export function replacePatternOverridesDefaultBinding( * @typedef {Object} WPBlockBindingsUtils * * @property {Function} updateBlockBindings Updates the value of the bindings connected to block attributes. - * @property {Function} removeBlockBinding Removes the binding for a specific attribute. * @property {Function} removeAllBlockBindings Removes the bindings property of the `metadata` attribute. */ @@ -83,7 +82,6 @@ export function replacePatternOverridesDefaultBinding( * * It contains the following utils: * - `updateBlockBindings`: Updates the value of the bindings connected to block attributes. It can be used to remove a specific binding by setting the value to `undefined`. - * - `removeBlockBinding`: Removes the binding for a specific attribute. * - `removeAllBlockBindings`: Removes the bindings property of the `metadata` attribute. * * @since 6.7.0 Introduced in WordPress core. @@ -95,7 +93,7 @@ export function replacePatternOverridesDefaultBinding( * @example * ```js * import { useBlockBindingsUtils } from '@wordpress/block-editor' - * const { updateBlockBindings, removeBlockBinding, removeAllBlockBindings } = useBlockBindingsUtils(); + * const { updateBlockBindings, removeAllBlockBindings } = useBlockBindingsUtils(); * * // Update url and alt attributes. * updateBlockBindings( { @@ -116,9 +114,6 @@ export function replacePatternOverridesDefaultBinding( * // Remove binding from url attribute. * updateBlockBindings( { url: undefined } ); * - * // Alternative: Remove binding from url attribute. - * removeBlockBinding( 'url' ); - * * // Remove bindings from all attributes. * removeAllBlockBindings(); * ``` @@ -185,44 +180,6 @@ export function useBlockBindingsUtils( clientId ) { } ); }; - /** - * Removes the binding for a specific attribute. - * - * @param {string} attribute The attribute name to remove the binding from. - * - * @example - * ```js - * import { useBlockBindingsUtils } from '@wordpress/block-editor' - * - * const { removeBlockBinding } = useBlockBindingsUtils(); - * removeBlockBinding( 'url' ); - * ``` - */ - const removeBlockBinding = ( attribute ) => { - const { metadata: { bindings, ...metadata } = {} } = - getBlockAttributes( blockClientId ); - - if ( ! bindings || ! bindings[ attribute ] ) { - return; - } - - const newBindings = { ...bindings }; - delete newBindings[ attribute ]; - - const newMetadata = { - ...metadata, - bindings: newBindings, - }; - - if ( isObjectEmpty( newMetadata.bindings ) ) { - delete newMetadata.bindings; - } - - updateBlockAttributes( blockClientId, { - metadata: isObjectEmpty( newMetadata ) ? undefined : newMetadata, - } ); - }; - /** * Removes the bindings property of the `metadata` attribute. * @@ -242,5 +199,5 @@ export function useBlockBindingsUtils( clientId ) { } ); }; - return { updateBlockBindings, removeBlockBinding, removeAllBlockBindings }; + return { updateBlockBindings, removeAllBlockBindings }; } diff --git a/packages/block-library/src/navigation-link/shared/use-entity-binding.js b/packages/block-library/src/navigation-link/shared/use-entity-binding.js index 3e9b5971397887..4ee85be35a1ab9 100644 --- a/packages/block-library/src/navigation-link/shared/use-entity-binding.js +++ b/packages/block-library/src/navigation-link/shared/use-entity-binding.js @@ -16,8 +16,7 @@ import { useBlockBindingsUtils } from '@wordpress/block-editor'; * @return {Object} Hook return value */ export function useEntityBinding( { clientId, attributes } ) { - const { updateBlockBindings, removeBlockBinding } = - useBlockBindingsUtils( clientId ); + const { updateBlockBindings } = useBlockBindingsUtils( clientId ); const { metadata, id } = attributes; // Check if there's a URL binding with a valid source (not null) @@ -29,10 +28,10 @@ export function useEntityBinding( { clientId, attributes } ) { const clearBinding = useCallback( () => { // Only clear if there's actually a binding to clear if ( hasBindingMetadata ) { - // Remove the URL binding - removeBlockBinding( 'url' ); + // Remove the URL binding by setting it to undefined + updateBlockBindings( { url: undefined } ); } - }, [ hasBindingMetadata, removeBlockBinding ] ); + }, [ hasBindingMetadata, updateBlockBindings ] ); const createBinding = useCallback( () => { updateBlockBindings( { From ff1f48c13c02d4b97f32ec1f2ac0cb150d3217f8 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 7 Oct 2025 15:50:47 +0100 Subject: [PATCH 3/6] Simplify even further --- .../src/navigation-link/shared/use-entity-binding.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/block-library/src/navigation-link/shared/use-entity-binding.js b/packages/block-library/src/navigation-link/shared/use-entity-binding.js index 4ee85be35a1ab9..273017595f29a4 100644 --- a/packages/block-library/src/navigation-link/shared/use-entity-binding.js +++ b/packages/block-library/src/navigation-link/shared/use-entity-binding.js @@ -19,19 +19,16 @@ export function useEntityBinding( { clientId, attributes } ) { const { updateBlockBindings } = useBlockBindingsUtils( clientId ); const { metadata, id } = attributes; - // Check if there's a URL binding with a valid source (not null) + // Check if there's a URL binding with a valid source const hasUrlBinding = !! metadata?.bindings?.url?.source && !! id; - // Check if there's ANY binding metadata (even if cleared/null) - const hasBindingMetadata = !! metadata?.bindings?.url; - const clearBinding = useCallback( () => { - // Only clear if there's actually a binding to clear - if ( hasBindingMetadata ) { + // Only clear if there's actually a valid binding to clear + if ( hasUrlBinding ) { // Remove the URL binding by setting it to undefined updateBlockBindings( { url: undefined } ); } - }, [ hasBindingMetadata, updateBlockBindings ] ); + }, [ hasUrlBinding, updateBlockBindings ] ); const createBinding = useCallback( () => { updateBlockBindings( { From b14da1fc2eb0961fc4430d53060b8c3209bb0251 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 7 Oct 2025 15:59:59 +0100 Subject: [PATCH 4/6] Add test coverage --- .../shared/test/use-entity-binding.js | 52 +++++++++++++++++-- 1 file changed, 48 insertions(+), 4 deletions(-) diff --git a/packages/block-library/src/navigation-link/shared/test/use-entity-binding.js b/packages/block-library/src/navigation-link/shared/test/use-entity-binding.js index 74bb603a05a699..93dcdb319141a4 100644 --- a/packages/block-library/src/navigation-link/shared/test/use-entity-binding.js +++ b/packages/block-library/src/navigation-link/shared/test/use-entity-binding.js @@ -72,7 +72,7 @@ describe( 'useEntityBinding', () => { expect( result.current.hasUrlBinding ).toBe( true ); } ); - it( 'should clear binding when clearBinding is called', () => { + it( 'should clear binding when clearBinding is called and binding exists', () => { const attributes = { metadata: { bindings: { @@ -97,11 +97,55 @@ describe( 'useEntityBinding', () => { } ); expect( mockUpdateBlockBindings ).toHaveBeenCalledWith( { - url: { - source: null, - args: null, + url: undefined, + } ); + } ); + + it( 'should NOT clear binding when clearBinding is called and no binding exists', () => { + const attributes = { + metadata: {}, + id: null, + }; + + const { result } = renderHook( () => + useEntityBinding( { + clientId: 'test-client-id', + attributes, + } ) + ); + + act( () => { + result.current.clearBinding(); + } ); + + expect( mockUpdateBlockBindings ).not.toHaveBeenCalled(); + } ); + + it( 'should NOT clear binding when binding metadata exists but source is null', () => { + const attributes = { + metadata: { + bindings: { + url: { + source: null, + args: null, + }, + }, }, + id: 123, + }; + + const { result } = renderHook( () => + useEntityBinding( { + clientId: 'test-client-id', + attributes, + } ) + ); + + act( () => { + result.current.clearBinding(); } ); + + expect( mockUpdateBlockBindings ).not.toHaveBeenCalled(); } ); it( 'should create binding when createBinding is called', () => { From c3d79b963523383d5f04728d7b11ea9a47d60a53 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 7 Oct 2025 16:00:56 +0100 Subject: [PATCH 5/6] Add tests for conditional clearBinding and optimize test performance --- .../shared/test/use-entity-binding.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/block-library/src/navigation-link/shared/test/use-entity-binding.js b/packages/block-library/src/navigation-link/shared/test/use-entity-binding.js index 93dcdb319141a4..d9ffc81081b14d 100644 --- a/packages/block-library/src/navigation-link/shared/test/use-entity-binding.js +++ b/packages/block-library/src/navigation-link/shared/test/use-entity-binding.js @@ -7,22 +7,21 @@ */ import { renderHook, act } from '@testing-library/react'; -/** - * WordPress dependencies - */ -import { useBlockBindingsUtils } from '@wordpress/block-editor'; - /** * Internal dependencies */ import { useEntityBinding } from '../use-entity-binding'; -// Mock the useBlockBindingsUtils hook +// Mock the entire @wordpress/block-editor module jest.mock( '@wordpress/block-editor', () => ( { - ...jest.requireActual( '@wordpress/block-editor' ), useBlockBindingsUtils: jest.fn(), } ) ); +/** + * WordPress dependencies + */ +import { useBlockBindingsUtils } from '@wordpress/block-editor'; + describe( 'useEntityBinding', () => { const mockUpdateBlockBindings = jest.fn(); From 768c2ec011b4d61d33ba123eb557e7828f448da0 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 7 Oct 2025 16:06:24 +0100 Subject: [PATCH 6/6] Fix check for specific binding source --- .../shared/test/use-entity-binding.js | 131 ++++++++++++++---- .../shared/use-entity-binding.js | 5 +- 2 files changed, 104 insertions(+), 32 deletions(-) diff --git a/packages/block-library/src/navigation-link/shared/test/use-entity-binding.js b/packages/block-library/src/navigation-link/shared/test/use-entity-binding.js index d9ffc81081b14d..62513e1d55b178 100644 --- a/packages/block-library/src/navigation-link/shared/test/use-entity-binding.js +++ b/packages/block-library/src/navigation-link/shared/test/use-entity-binding.js @@ -32,43 +32,114 @@ describe( 'useEntityBinding', () => { } ); } ); - it( 'should return hasUrlBinding as false when no binding exists', () => { - const attributes = { - metadata: {}, - id: null, - }; + describe( 'hasUrlBinding', () => { + it( 'should return false when no binding exists', () => { + const attributes = { + metadata: {}, + id: null, + }; + + const { result } = renderHook( () => + useEntityBinding( { + clientId: 'test-client-id', + attributes, + } ) + ); + + expect( result.current.hasUrlBinding ).toBe( false ); + } ); - const { result } = renderHook( () => - useEntityBinding( { - clientId: 'test-client-id', - attributes, - } ) - ); + it( 'should return true when core/entity binding exists with id', () => { + const attributes = { + metadata: { + bindings: { + url: { + source: 'core/entity', + args: { key: 'url' }, + }, + }, + }, + id: 123, + }; - expect( result.current.hasUrlBinding ).toBe( false ); - } ); + const { result } = renderHook( () => + useEntityBinding( { + clientId: 'test-client-id', + attributes, + } ) + ); - it( 'should return hasUrlBinding as true when binding exists', () => { - const attributes = { - metadata: { - bindings: { - url: { - source: 'core/entity', - args: { key: 'url' }, + expect( result.current.hasUrlBinding ).toBe( true ); + } ); + + it( 'should return false when source is not core/entity', () => { + const attributes = { + metadata: { + bindings: { + url: { + source: 'some-other-source', + args: { key: 'url' }, + }, }, }, - }, - id: 123, - }; + id: 123, + }; - const { result } = renderHook( () => - useEntityBinding( { - clientId: 'test-client-id', - attributes, - } ) - ); + const { result } = renderHook( () => + useEntityBinding( { + clientId: 'test-client-id', + attributes, + } ) + ); + + expect( result.current.hasUrlBinding ).toBe( false ); + } ); - expect( result.current.hasUrlBinding ).toBe( true ); + it( 'should return false when core/entity binding exists but no id', () => { + const attributes = { + metadata: { + bindings: { + url: { + source: 'core/entity', + args: { key: 'url' }, + }, + }, + }, + id: null, + }; + + const { result } = renderHook( () => + useEntityBinding( { + clientId: 'test-client-id', + attributes, + } ) + ); + + expect( result.current.hasUrlBinding ).toBe( false ); + } ); + + it( 'should return false when binding source is null', () => { + const attributes = { + metadata: { + bindings: { + url: { + source: null, + args: null, + }, + }, + }, + id: 123, + }; + + const { result } = renderHook( () => + useEntityBinding( { + clientId: 'test-client-id', + attributes, + } ) + ); + + expect( result.current.hasUrlBinding ).toBe( false ); + } ); } ); it( 'should clear binding when clearBinding is called and binding exists', () => { diff --git a/packages/block-library/src/navigation-link/shared/use-entity-binding.js b/packages/block-library/src/navigation-link/shared/use-entity-binding.js index 273017595f29a4..a24127d1004a31 100644 --- a/packages/block-library/src/navigation-link/shared/use-entity-binding.js +++ b/packages/block-library/src/navigation-link/shared/use-entity-binding.js @@ -19,8 +19,9 @@ export function useEntityBinding( { clientId, attributes } ) { const { updateBlockBindings } = useBlockBindingsUtils( clientId ); const { metadata, id } = attributes; - // Check if there's a URL binding with a valid source - const hasUrlBinding = !! metadata?.bindings?.url?.source && !! id; + // Check if there's a URL binding with the core/entity source + const hasUrlBinding = + metadata?.bindings?.url?.source === 'core/entity' && !! id; const clearBinding = useCallback( () => { // Only clear if there's actually a valid binding to clear