Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Block Bindings: Improve how the context needed by sources is extended in the editor #63513

Merged
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
70 changes: 70 additions & 0 deletions packages/blocks/src/api/test/registration.js
Original file line number Diff line number Diff line change
Expand Up @@ -1528,6 +1528,74 @@ 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 client', () => {
artemiomorales marked this conversation as resolved.
Show resolved Hide resolved
// Simulate server bootstrap.
registerBlockBindingsSource( {
name: 'core/testing',
label: 'testing',
usesContext: [ 'postId', 'postType' ],
} );
artemiomorales marked this conversation as resolved.
Show resolved Hide resolved
// 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 keep usesContext when it is not defined in the client', () => {
artemiomorales marked this conversation as resolved.
Show resolved Hide resolved
// Simulate server bootstrap.
registerBlockBindingsSource( {
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', () => {
// Simulate server bootstrap.
registerBlockBindingsSource( {
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 +1652,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 +1674,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
Loading