Skip to content

Commit

Permalink
Block Bindings: Improve how the context needed by sources is extended…
Browse files Browse the repository at this point in the history
… in the editor (WordPress#63513)

* Move bindings registration to editor provider

* Remove sources from editor provider

* Create `registerCoreBlockBindingsSources` function

* Add `updateBlockBindingsSource` action

* Bootstrap sources defined in the server

* Change registration to allow server bootstrap

* Remove label from post meta and pattern overrides

* Remove `updateBlockBindingsSource`

* Use `bootstrapBlockBindingsSource` instead

* Wrap server registration in the same function

* Change how label is managed

* Remove type from object

* Add usesContext to the store

* Handle usesContext validation

* Merge usesContext and add tests

* Read `usesContext` during bindings processing

* Adapt code after rebase

* More changes after rebase

* Adapt unit test

* Adapt unit tests

Co-authored-by: SantosGuillamot <[email protected]>
Co-authored-by: artemiomorales <[email protected]>
Co-authored-by: gziolo <[email protected]>
  • Loading branch information
4 people authored Jul 24, 2024
1 parent 85db58f commit ab9e132
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 6 deletions.
28 changes: 24 additions & 4 deletions packages/block-editor/src/hooks/use-bindings-attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@
import { store as blocksStore } from '@wordpress/blocks';
import { createHigherOrderComponent } from '@wordpress/compose';
import { useRegistry, useSelect } from '@wordpress/data';
import { useCallback, useMemo } from '@wordpress/element';
import { useCallback, useMemo, useContext } from '@wordpress/element';
import { addFilter } from '@wordpress/hooks';

/**
* Internal dependencies
*/
import { unlock } from '../lock-unlock';
import BlockContext from '../components/block-context';

/** @typedef {import('@wordpress/compose').WPHigherOrderComponent} WPHigherOrderComponent */
/** @typedef {import('@wordpress/blocks').WPBlockSettings} WPBlockSettings */
Expand Down Expand Up @@ -93,10 +94,11 @@ export function canBindAttribute( blockName, attributeName ) {
export const withBlockBindingSupport = createHigherOrderComponent(
( BlockEdit ) => ( props ) => {
const registry = useRegistry();
const blockContext = useContext( BlockContext );
const sources = useSelect( ( select ) =>
unlock( select( blocksStore ) ).getAllBlockBindingsSources()
);
const { name, clientId, context } = props;
const { name, clientId } = props;
const hasParentPattern = !! props.context[ 'pattern/overrides' ];
const hasPatternOverridesDefaultBinding =
props.attributes.metadata?.bindings?.[ DEFAULT_ATTRIBUTE ]
Expand Down Expand Up @@ -145,6 +147,15 @@ export const withBlockBindingSupport = createHigherOrderComponent(

if ( blockBindingsBySource.size ) {
for ( const [ source, bindings ] of blockBindingsBySource ) {
// Populate context.
const context = {};

if ( source.usesContext?.length ) {
for ( const key of source.usesContext ) {
context[ key ] = blockContext[ key ];
}
}

// Get values in batch if the source supports it.
const values = source.getValues( {
registry,
Expand Down Expand Up @@ -177,7 +188,7 @@ export const withBlockBindingSupport = createHigherOrderComponent(
}

return attributes;
}, [ blockBindings, name, clientId, context, registry, sources ] );
}, [ blockBindings, name, clientId, blockContext, registry, sources ] );

const { setAttributes } = props;

Expand Down Expand Up @@ -223,6 +234,15 @@ export const withBlockBindingSupport = createHigherOrderComponent(
source,
bindings,
] of blockBindingsBySource ) {
// Populate context.
const context = {};

if ( source.usesContext?.length ) {
for ( const key of source.usesContext ) {
context[ key ] = blockContext[ key ];
}
}

source.setValues( {
registry,
context,
Expand Down Expand Up @@ -255,7 +275,7 @@ export const withBlockBindingSupport = createHigherOrderComponent(
blockBindings,
name,
clientId,
context,
blockContext,
setAttributes,
sources,
hasPatternOverridesDefaultBinding,
Expand Down
8 changes: 8 additions & 0 deletions packages/blocks/src/api/registration.js
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,7 @@ export const unregisterBlockVariation = ( blockName, variationName ) => {
* @param {Object} source Properties of the source to be registered.
* @param {string} source.name The unique and machine-readable name.
* @param {string} [source.label] Human-readable label.
* @param {Array} [source.usesContext] Array of context needed by the source only in the editor.
* @param {Function} [source.getValues] Function to get the values from the source.
* @param {Function} [source.setValues] Function to update multiple values connected to the source.
* @param {Function} [source.getPlaceholder] Function to get the placeholder when the value is undefined.
Expand All @@ -794,6 +795,7 @@ export const registerBlockBindingsSource = ( source ) => {
const {
name,
label,
usesContext,
getValues,
setValues,
getPlaceholder,
Expand Down Expand Up @@ -867,6 +869,12 @@ export const registerBlockBindingsSource = ( source ) => {
return;
}

// Check the `usesContext` property is correct.
if ( usesContext && ! Array.isArray( usesContext ) ) {
warning( 'Block bindings source usesContext must be an array.' );
return;
}

// Check the `getValues` property is correct.
if ( getValues && typeof getValues !== 'function' ) {
warning( 'Block bindings source getValues must be a function.' );
Expand Down
82 changes: 80 additions & 2 deletions packages/blocks/src/api/test/registration.js
Original file line number Diff line number Diff line change
Expand Up @@ -1513,8 +1513,10 @@ describe( 'blocks', () => {
} );

it( 'should not override label from the server', () => {
// Simulate bootstrapping a source from the server registration.
registerBlockBindingsSource( {
// Bootstrap source from the server.
unlock(
dispatch( blocksStore )
).addBootstrappedBlockBindingsSource( {
name: 'core/server',
label: 'Server label',
} );
Expand All @@ -1528,6 +1530,80 @@ describe( 'blocks', () => {
);
} );

// Check the `usesContext` array is correct.
it( 'should reject invalid usesContext property', () => {
registerBlockBindingsSource( {
name: 'core/testing',
label: 'testing',
usesContext: 'should be an array',
} );
expect( console ).toHaveWarnedWith(
'Block bindings source usesContext must be an array.'
);
} );

it( 'should add usesContext when only defined in the server', () => {
// Bootstrap source from the server.
unlock(
dispatch( blocksStore )
).addBootstrappedBlockBindingsSource( {
name: 'core/testing',
label: 'testing',
usesContext: [ 'postId', 'postType' ],
} );
// Register source in the client without usesContext.
registerBlockBindingsSource( {
name: 'core/testing',
getValue: () => 'value',
} );
const source = getBlockBindingsSource( 'core/testing' );
unregisterBlockBindingsSource( 'core/testing' );
expect( source.usesContext ).toEqual( [ 'postId', 'postType' ] );
} );

it( 'should add usesContext when only defined in the client', () => {
// Bootstrap source from the server.
unlock(
dispatch( blocksStore )
).addBootstrappedBlockBindingsSource( {
name: 'core/testing',
label: 'testing',
} );
// Register source in the client with usesContext.
registerBlockBindingsSource( {
name: 'core/testing',
usesContext: [ 'postId', 'postType' ],
getValue: () => 'value',
} );
const source = getBlockBindingsSource( 'core/testing' );
unregisterBlockBindingsSource( 'core/testing' );
expect( source.usesContext ).toEqual( [ 'postId', 'postType' ] );
} );

it( 'should merge usesContext from server and client without duplicates', () => {
// Bootstrap source from the server.
unlock(
dispatch( blocksStore )
).addBootstrappedBlockBindingsSource( {
name: 'core/testing',
label: 'testing',
usesContext: [ 'postId', 'postType' ],
} );
// Register source in the client with usesContext.
registerBlockBindingsSource( {
name: 'core/testing',
usesContext: [ 'postType', 'clientContext' ],
getValue: () => 'value',
} );
const source = getBlockBindingsSource( 'core/testing' );
unregisterBlockBindingsSource( 'core/testing' );
expect( source.usesContext ).toEqual( [
'postId',
'postType',
'clientContext',
] );
} );

// Check the `getValues` callback is correct.
it( 'should reject invalid getValues callback', () => {
registerBlockBindingsSource( {
Expand Down Expand Up @@ -1584,6 +1660,7 @@ describe( 'blocks', () => {
it( 'should register a valid source', () => {
const sourceProperties = {
label: 'Valid Source',
usesContext: [ 'postId' ],
getValues: () => 'value',
setValues: () => 'new values',
getPlaceholder: () => 'placeholder',
Expand All @@ -1605,6 +1682,7 @@ describe( 'blocks', () => {
label: 'Valid Source',
} );
const source = getBlockBindingsSource( 'core/valid-source' );
expect( source.usesContext ).toBeUndefined();
expect( source.getValues ).toBeUndefined();
expect( source.setValues ).toBeUndefined();
expect( source.getPlaceholder ).toBeUndefined();
Expand Down
1 change: 1 addition & 0 deletions packages/blocks/src/store/private-actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export function addBlockBindingsSource( source ) {
type: 'ADD_BLOCK_BINDINGS_SOURCE',
name: source.name,
label: source.label,
usesContext: source.usesContext,
getValues: source.getValues,
setValues: source.setValues,
getPlaceholder: source.getPlaceholder,
Expand Down
12 changes: 12 additions & 0 deletions packages/blocks/src/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -374,11 +374,23 @@ export function collections( state = {}, action ) {
export function blockBindingsSources( state = {}, action ) {
switch ( action.type ) {
case 'ADD_BLOCK_BINDINGS_SOURCE':
// Merge usesContext with existing values, potentially defined in the server registration.
let mergedUsesContext = [
...( state[ action.name ]?.usesContext || [] ),
...( action.usesContext || [] ),
];
// Remove duplicates.
mergedUsesContext =
mergedUsesContext.length > 0
? [ ...new Set( mergedUsesContext ) ]
: undefined;

return {
...state,
[ action.name ]: {
// Don't override the label if it's already set.
label: state[ action.name ]?.label || action.label,
usesContext: mergedUsesContext,
getValues: action.getValues,
setValues: action.setValues,
getPlaceholder: action.getPlaceholder,
Expand Down

0 comments on commit ab9e132

Please sign in to comment.