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: Bootstrap sources defined in the server #63470

Merged
merged 25 commits into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
44d86f0
Move bindings registration to editor provider
SantosGuillamot Jul 19, 2024
0ca3e00
Remove sources from editor provider
SantosGuillamot Jul 19, 2024
758ede2
Create `registerCoreBlockBindingsSources` function
SantosGuillamot Jul 19, 2024
b575369
Add `updateBlockBindingsSource` action
SantosGuillamot Jul 19, 2024
0e29fcd
Bootstrap sources defined in the server
SantosGuillamot Jul 19, 2024
f67ff90
Change registration to allow server bootstrap
SantosGuillamot Jul 19, 2024
a8fa943
Remove label from post meta and pattern overrides
SantosGuillamot Jul 19, 2024
8742066
Remove `updateBlockBindingsSource`
SantosGuillamot Jul 19, 2024
429ebe7
Use `bootstrapBlockBindingsSource` instead
SantosGuillamot Jul 19, 2024
575a2a9
Wrap server registration in the same function
SantosGuillamot Jul 19, 2024
9a56150
Ensure label is provided when source is not bootstrapped
SantosGuillamot Jul 19, 2024
a98c45a
Remove old import
SantosGuillamot Jul 19, 2024
bde87d2
Add compat filter to expose server sources
SantosGuillamot Jul 19, 2024
7cdf395
Add e2e test
SantosGuillamot Jul 19, 2024
18f7227
Check if the sources are already added
SantosGuillamot Jul 19, 2024
a05caf6
Change how label is managed
SantosGuillamot Jul 19, 2024
f330a6a
Remove type from object
SantosGuillamot Jul 19, 2024
c1ebd6b
Use blockBindingsSources name
SantosGuillamot Jul 19, 2024
33603d3
Fixes from rebase
SantosGuillamot Jul 19, 2024
facd94e
Add return back and warning
SantosGuillamot Jul 19, 2024
b3b0a38
Add backport
SantosGuillamot Jul 19, 2024
cf2a8e3
Improve comments
SantosGuillamot Jul 19, 2024
227890f
Fix getValues/setValues after rebase
SantosGuillamot Jul 19, 2024
50cfa09
Adapt reducer and fixes
SantosGuillamot Jul 19, 2024
8389d37
Revert args used in `addBlockBindingsSource`
SantosGuillamot Jul 22, 2024
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
3 changes: 3 additions & 0 deletions backport-changelog/6.7/7020.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
https://github.com/WordPress/wordpress-develop/pull/7020

* https://github.com/WordPress/gutenberg/pull/63470
40 changes: 40 additions & 0 deletions lib/compat/wordpress-6.7/block-bindings.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php
/**
* Temporary compatibility code for new functionalitites/changes related to block bindings APIs present in Gutenberg.
*
* @package gutenberg
*/

/**
* Adds the block bindings sources registered in the server to the editor settings.
*
* This allows them to be bootstrapped in the editor.
*
* @param array $settings The block editor settings from the `block_editor_settings_all` filter.
* @return array The editor settings including the block bindings sources.
*/
function gutenberg_add_server_block_bindings_sources_to_editor_settings( $editor_settings ) {
// Check if the sources are already exposed in the editor settings.
if ( isset( $editor_settings['blockBindingsSources'] ) ) {
return $editor_settings;
}

$registered_block_bindings_sources = get_all_registered_block_bindings_sources();
if ( ! empty( $registered_block_bindings_sources ) ) {
// Initialize array.
$editor_settings['blockBindingsSources'] = array();
foreach ( $registered_block_bindings_sources as $source_name => $source_properties ) {
// Add source with the label to editor settings.
$editor_settings['blockBindingsSources'][ $source_name ] = array(
'label' => $source_properties->label,
);
// Add `usesContext` property if exists.
if ( ! empty( $source_properties->uses_context ) ) {
$editor_settings['blockBindingsSources'][ $source_name ]['usesContext'] = $source_properties->uses_context;
}
}
}
return $editor_settings;
}

add_filter( 'block_editor_settings_all', 'gutenberg_add_server_block_bindings_sources_to_editor_settings', 10 );
1 change: 1 addition & 0 deletions lib/load.php
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ function gutenberg_is_experiment_enabled( $name ) {

// WordPress 6.7 compat.
require __DIR__ . '/compat/wordpress-6.7/blocks.php';
require __DIR__ . '/compat/wordpress-6.7/block-bindings.php';

// Experimental features.
require __DIR__ . '/experimental/block-editor-settings-mobile.php';
Expand Down
23 changes: 18 additions & 5 deletions packages/blocks/src/api/registration.js
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,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 {string} [source.label] Human-readable label.
* @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 Down Expand Up @@ -800,11 +800,15 @@ export const registerBlockBindingsSource = ( source ) => {
canUserEditValue,
} = source;

// Check if the source is already registered.
const existingSource = unlock(
select( blocksStore )
).getBlockBindingsSource( name );
if ( existingSource ) {

/*
* Check if the source has been already registered on the client.
* If the `getValues` property is defined, it could be assumed the source is already registered.
*/
if ( existingSource?.getValues ) {
warning(
'Block bindings source "' + name + '" is already registered.'
);
Expand Down Expand Up @@ -844,12 +848,21 @@ export const registerBlockBindingsSource = ( source ) => {
}

// Check the `label` property is correct.
if ( ! label ) {
if ( label && existingSource?.label ) {
gziolo marked this conversation as resolved.
Show resolved Hide resolved
warning(
'Block bindings "' +
name +
'" source label is already defined in the server.'
);
return;
}

if ( ! label && ! existingSource?.label ) {
warning( 'Block bindings source must contain a label.' );
return;
}

if ( typeof label !== 'string' ) {
if ( label && typeof label !== 'string' ) {
warning( 'Block bindings source label must be a string.' );
return;
}
Expand Down
17 changes: 17 additions & 0 deletions packages/blocks/src/api/test/registration.js
Original file line number Diff line number Diff line change
Expand Up @@ -1512,6 +1512,22 @@ describe( 'blocks', () => {
expect( getBlockBindingsSource( 'core/testing' ) ).toBeUndefined();
} );

it( 'should not override label from the server', () => {
// Simulate bootstrapping a source from the server registration.
registerBlockBindingsSource( {
name: 'core/server',
label: 'Server label',
} );
SantosGuillamot marked this conversation as resolved.
Show resolved Hide resolved
// Override the source with a different label in the client.
registerBlockBindingsSource( {
name: 'core/server',
label: 'Client label',
} );
expect( console ).toHaveWarnedWith(
'Block bindings "core/server" source label is already defined in the server.'
);
} );

// Check the `getValues` callback is correct.
it( 'should reject invalid getValues callback', () => {
registerBlockBindingsSource( {
Expand Down Expand Up @@ -1600,6 +1616,7 @@ describe( 'blocks', () => {
const source = {
name: 'core/test-source',
label: 'Test Source',
getValues: () => 'value',
};
registerBlockBindingsSource( source );
registerBlockBindingsSource( source );
Expand Down
14 changes: 14 additions & 0 deletions packages/blocks/src/store/private-actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,17 @@ export function removeBlockBindingsSource( name ) {
name,
};
}

/**
* Add bootstrapped block bindings sources, usually initialized from the server.
*
* @param {string} source Name of the source to bootstrap.
*/
export function addBootstrappedBlockBindingsSource( source ) {
return {
type: 'ADD_BOOTSTRAPPED_BLOCK_BINDINGS_SOURCE',
name: source.name,
label: source.label,
usesContext: source.usesContext,
};
}
11 changes: 10 additions & 1 deletion packages/blocks/src/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -377,13 +377,22 @@ export function blockBindingsSources( state = {}, action ) {
return {
...state,
[ action.name ]: {
label: action.label,
// Don't override the label if it's already set.
label: state[ action.name ]?.label || action.label,
getValues: action.getValues,
setValues: action.setValues,
getPlaceholder: action.getPlaceholder,
canUserEditValue: action.canUserEditValue,
},
};
case 'ADD_BOOTSTRAPPED_BLOCK_BINDINGS_SOURCE':
return {
...state,
[ action.name ]: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my #63470 (comment). I predict it’s the missing edge case to cover. In the case bootstrapping happens after registration, it would replace the entry with data from the server. A unit test would also help here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. I'll work on a follow-up pull request to cover that use case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have started this pull request trying to address this use case: link.

label: action.label,
usesContext: action.usesContext,
},
};
case 'REMOVE_BLOCK_BINDINGS_SOURCE':
return omit( state, action.name );
}
Expand Down
16 changes: 13 additions & 3 deletions packages/e2e-tests/plugins/block-bindings.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,19 @@
*/

/**
* Register custom fields.
* Register custom fields and custom block bindings sources.
*/
function gutenberg_test_block_bindings_register_custom_fields() {
function gutenberg_test_block_bindings_registration() {
// Register custom block bindings sources.
register_block_bindings_source(
'core/server-source',
array(
'label' => 'Server Source',
'get_value_callback' => function () {},
)
);

// Register custom fields.
register_meta(
'post',
'text_custom_field',
Expand Down Expand Up @@ -51,4 +61,4 @@ function gutenberg_test_block_bindings_register_custom_fields() {
)
);
}
add_action( 'init', 'gutenberg_test_block_bindings_register_custom_fields' );
add_action( 'init', 'gutenberg_test_block_bindings_registration' );
2 changes: 2 additions & 0 deletions packages/edit-post/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const {
BackButton: __experimentalMainDashboardButton,
registerDefaultActions,
registerCoreBlockBindingsSources,
bootstrapBlockBindingsSourcesFromServer,
} = unlock( editorPrivateApis );

/**
Expand Down Expand Up @@ -87,6 +88,7 @@ export function initializeEditor(
}

registerCoreBlocks();
bootstrapBlockBindingsSourcesFromServer( settings?.blockBindingsSources );
registerCoreBlockBindingsSources();
registerLegacyWidgetBlock( { inserter: false } );
registerWidgetGroupBlock( { inserter: false } );
Expand Down
8 changes: 6 additions & 2 deletions packages/edit-site/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,11 @@ import { store as editSiteStore } from './store';
import { unlock } from './lock-unlock';
import App from './components/app';

const { registerDefaultActions, registerCoreBlockBindingsSources } =
unlock( editorPrivateApis );
const {
registerDefaultActions,
registerCoreBlockBindingsSources,
bootstrapBlockBindingsSourcesFromServer,
} = unlock( editorPrivateApis );

/**
* Initializes the site editor screen.
Expand All @@ -46,6 +49,7 @@ export function initializeEditor( id, settings ) {
( { name } ) => name !== 'core/freeform'
);
registerCoreBlocks( coreBlocks );
bootstrapBlockBindingsSourcesFromServer( settings?.blockBindingsSources );
registerCoreBlockBindingsSources();
dispatch( blocksStore ).setFreeformFallbackBlockName( 'core/html' );
registerLegacyWidgetBlock( { inserter: false } );
Expand Down
32 changes: 31 additions & 1 deletion packages/editor/src/bindings/api.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
/**
* WordPress dependencies
*/
import { privateApis as blocksPrivateApis } from '@wordpress/blocks';
import {
privateApis as blocksPrivateApis,
store as blocksStore,
} from '@wordpress/blocks';
import { dispatch } from '@wordpress/data';

/**
* Internal dependencies
Expand All @@ -25,3 +29,29 @@ export function registerCoreBlockBindingsSources() {
registerBlockBindingsSource( patternOverrides );
registerBlockBindingsSource( postMeta );
}

/**
* Function to bootstrap core block bindings sources defined in the server.
*
* @param {Object} sources Object containing the sources to bootstrap.
*
* @example
* ```js
* import { bootstrapBlockBindingsSourcesFromServer } from '@wordpress/editor';
*
* bootstrapBlockBindingsSourcesFromServer( sources );
* ```
*/
export function bootstrapBlockBindingsSourcesFromServer( sources ) {
if ( sources ) {
const { addBootstrappedBlockBindingsSource } = unlock(
dispatch( blocksStore )
);
for ( const [ name, args ] of Object.entries( sources ) ) {
addBootstrappedBlockBindingsSource( {
name,
...args,
} );
}
}
}
2 changes: 0 additions & 2 deletions packages/editor/src/bindings/pattern-overrides.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
/**
* WordPress dependencies
*/
import { _x } from '@wordpress/i18n';
import { store as blockEditorStore } from '@wordpress/block-editor';

const CONTENT = 'content';

export default {
name: 'core/pattern-overrides',
label: _x( 'Pattern Overrides', 'block bindings source' ),
getValues( { registry, clientId, context, bindings } ) {
const patternOverridesContent = context[ 'pattern/overrides' ];
const { getBlockAttributes } = registry.select( blockEditorStore );
Expand Down
2 changes: 0 additions & 2 deletions packages/editor/src/bindings/post-meta.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
* WordPress dependencies
*/
import { store as coreDataStore } from '@wordpress/core-data';
import { _x } from '@wordpress/i18n';

/**
* Internal dependencies
Expand All @@ -11,7 +10,6 @@ import { store as editorStore } from '../store';

export default {
name: 'core/post-meta',
label: _x( 'Post Meta', 'block bindings source' ),
getPlaceholder( { args } ) {
return args.key;
},
Expand Down
6 changes: 5 additions & 1 deletion packages/editor/src/private-apis.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ import {
GlobalStylesProvider,
} from './components/global-styles-provider';
import registerDefaultActions from './dataviews/actions';
import { registerCoreBlockBindingsSources } from './bindings/api';
import {
registerCoreBlockBindingsSources,
bootstrapBlockBindingsSourcesFromServer,
} from './bindings/api';

const { store: interfaceStore, ...remainingInterfaceApis } = interfaceApis;

Expand All @@ -45,6 +48,7 @@ lock( privateApis, {
ResizableEditor,
registerDefaultActions,
registerCoreBlockBindingsSources,
bootstrapBlockBindingsSourcesFromServer,

// This is a temporary private API while we're updating the site editor to use EditorProvider.
useBlockEditorSettings,
Expand Down
29 changes: 29 additions & 0 deletions test/e2e/specs/editor/various/block-bindings.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2172,4 +2172,33 @@ test.describe( 'Block bindings', () => {
} );
} );
} );

test.describe( 'Sources registration', () => {
test.beforeEach( async ( { admin } ) => {
await admin.createNewPost( { title: 'Test bindings' } );
} );

test( 'should show the label of a source only registered in the server', async ( {
editor,
page,
} ) => {
await editor.insertBlock( {
name: 'core/paragraph',
attributes: {
metadata: {
bindings: {
content: {
source: 'core/server-source',
},
},
},
},
} );

const bindingLabel = page.locator(
'.components-item__block-bindings-source'
);
await expect( bindingLabel ).toHaveText( 'Server Source' );
} );
} );
} );
Loading