-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Extensibility: Make Block Bindings work with editor.BlockEdit
hook (2nd try)
#67523
Changes from all commits
9cd9bac
1eec4e6
535bbd5
414436d
0b1eaf4
245eae5
a87251e
47e700c
dcb21ad
da3988f
f192c04
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -6,18 +6,27 @@ import clsx from 'clsx'; | |||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||
* WordPress dependencies | ||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||
import { withFilters } from '@wordpress/components'; | ||||||||||||||||||||||||||||
import { | ||||||||||||||||||||||||||||
getBlockDefaultClassName, | ||||||||||||||||||||||||||||
hasBlockSupport, | ||||||||||||||||||||||||||||
getBlockType, | ||||||||||||||||||||||||||||
hasBlockSupport, | ||||||||||||||||||||||||||||
store as blocksStore, | ||||||||||||||||||||||||||||
} from '@wordpress/blocks'; | ||||||||||||||||||||||||||||
import { useContext, useMemo } from '@wordpress/element'; | ||||||||||||||||||||||||||||
import { withFilters } from '@wordpress/components'; | ||||||||||||||||||||||||||||
import { useRegistry, useSelect } from '@wordpress/data'; | ||||||||||||||||||||||||||||
import { useCallback, useContext, useMemo } from '@wordpress/element'; | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||
* Internal dependencies | ||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||
import BlockContext from '../block-context'; | ||||||||||||||||||||||||||||
import isURLLike from '../link-control/is-url-like'; | ||||||||||||||||||||||||||||
import { | ||||||||||||||||||||||||||||
canBindAttribute, | ||||||||||||||||||||||||||||
hasPatternOverridesDefaultBinding, | ||||||||||||||||||||||||||||
replacePatternOverridesDefaultBinding, | ||||||||||||||||||||||||||||
} from '../../utils/block-bindings'; | ||||||||||||||||||||||||||||
import { unlock } from '../../lock-unlock'; | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||
* Default value used for blocks which do not define their own context needs, | ||||||||||||||||||||||||||||
|
@@ -48,27 +57,223 @@ const Edit = ( props ) => { | |||||||||||||||||||||||||||
const EditWithFilters = withFilters( 'editor.BlockEdit' )( Edit ); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
const EditWithGeneratedProps = ( props ) => { | ||||||||||||||||||||||||||||
const { attributes = {}, name } = props; | ||||||||||||||||||||||||||||
const { name, clientId, attributes, setAttributes } = props; | ||||||||||||||||||||||||||||
const registry = useRegistry(); | ||||||||||||||||||||||||||||
const blockType = getBlockType( name ); | ||||||||||||||||||||||||||||
const blockContext = useContext( BlockContext ); | ||||||||||||||||||||||||||||
const registeredSources = useSelect( | ||||||||||||||||||||||||||||
( select ) => | ||||||||||||||||||||||||||||
unlock( select( blocksStore ) ).getAllBlockBindingsSources(), | ||||||||||||||||||||||||||||
[] | ||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||
gziolo marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
// Assign context values using the block type's declared context needs. | ||||||||||||||||||||||||||||
const context = useMemo( () => { | ||||||||||||||||||||||||||||
return blockType && blockType.usesContext | ||||||||||||||||||||||||||||
const { blockBindings, context, hasPatternOverrides } = useMemo( () => { | ||||||||||||||||||||||||||||
// Assign context values using the block type's declared context needs. | ||||||||||||||||||||||||||||
const computedContext = blockType?.usesContext | ||||||||||||||||||||||||||||
? Object.fromEntries( | ||||||||||||||||||||||||||||
Object.entries( blockContext ).filter( ( [ key ] ) => | ||||||||||||||||||||||||||||
blockType.usesContext.includes( key ) | ||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||
: DEFAULT_BLOCK_CONTEXT; | ||||||||||||||||||||||||||||
}, [ blockType, blockContext ] ); | ||||||||||||||||||||||||||||
// Add context requested by Block Bindings sources. | ||||||||||||||||||||||||||||
if ( attributes?.metadata?.bindings ) { | ||||||||||||||||||||||||||||
Object.values( attributes?.metadata?.bindings || {} ).forEach( | ||||||||||||||||||||||||||||
( binding ) => { | ||||||||||||||||||||||||||||
registeredSources[ binding?.source ]?.usesContext?.forEach( | ||||||||||||||||||||||||||||
( key ) => { | ||||||||||||||||||||||||||||
computedContext[ key ] = blockContext[ key ]; | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
return { | ||||||||||||||||||||||||||||
blockBindings: replacePatternOverridesDefaultBinding( | ||||||||||||||||||||||||||||
name, | ||||||||||||||||||||||||||||
attributes?.metadata?.bindings | ||||||||||||||||||||||||||||
), | ||||||||||||||||||||||||||||
context: computedContext, | ||||||||||||||||||||||||||||
hasPatternOverrides: hasPatternOverridesDefaultBinding( | ||||||||||||||||||||||||||||
attributes?.metadata?.bindings | ||||||||||||||||||||||||||||
), | ||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||
}, [ | ||||||||||||||||||||||||||||
name, | ||||||||||||||||||||||||||||
blockType?.usesContext, | ||||||||||||||||||||||||||||
blockContext, | ||||||||||||||||||||||||||||
attributes?.metadata?.bindings, | ||||||||||||||||||||||||||||
registeredSources, | ||||||||||||||||||||||||||||
] ); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
const computedAttributes = useSelect( | ||||||||||||||||||||||||||||
( select ) => { | ||||||||||||||||||||||||||||
if ( ! blockBindings ) { | ||||||||||||||||||||||||||||
return attributes; | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
const attributesFromSources = {}; | ||||||||||||||||||||||||||||
const blockBindingsBySource = new Map(); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
for ( const [ attributeName, binding ] of Object.entries( | ||||||||||||||||||||||||||||
blockBindings | ||||||||||||||||||||||||||||
) ) { | ||||||||||||||||||||||||||||
const { source: sourceName, args: sourceArgs } = binding; | ||||||||||||||||||||||||||||
const source = registeredSources[ sourceName ]; | ||||||||||||||||||||||||||||
if ( ! source || ! canBindAttribute( name, attributeName ) ) { | ||||||||||||||||||||||||||||
continue; | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
blockBindingsBySource.set( source, { | ||||||||||||||||||||||||||||
...blockBindingsBySource.get( source ), | ||||||||||||||||||||||||||||
[ attributeName ]: { | ||||||||||||||||||||||||||||
args: sourceArgs, | ||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||
} ); | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
if ( blockBindingsBySource.size ) { | ||||||||||||||||||||||||||||
for ( const [ source, bindings ] of blockBindingsBySource ) { | ||||||||||||||||||||||||||||
// Get values in batch if the source supports it. | ||||||||||||||||||||||||||||
let values = {}; | ||||||||||||||||||||||||||||
if ( ! source.getValues ) { | ||||||||||||||||||||||||||||
Object.keys( bindings ).forEach( ( attr ) => { | ||||||||||||||||||||||||||||
// Default to the the source label when `getValues` doesn't exist. | ||||||||||||||||||||||||||||
values[ attr ] = source.label; | ||||||||||||||||||||||||||||
} ); | ||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||
values = source.getValues( { | ||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Mamaduka, as I'm looking at: I'm wondering what the pattern should be for observing store changes when you expect that for the same set of dependencies, there are expected different results. In this particular case, gutenberg/packages/editor/src/bindings/post-meta.js Lines 77 to 89 in 5e3d575
In effect, where |
||||||||||||||||||||||||||||
select, | ||||||||||||||||||||||||||||
context, | ||||||||||||||||||||||||||||
clientId, | ||||||||||||||||||||||||||||
bindings, | ||||||||||||||||||||||||||||
} ); | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
for ( const [ attributeName, value ] of Object.entries( | ||||||||||||||||||||||||||||
values | ||||||||||||||||||||||||||||
) ) { | ||||||||||||||||||||||||||||
if ( | ||||||||||||||||||||||||||||
attributeName === 'url' && | ||||||||||||||||||||||||||||
( ! value || ! isURLLike( value ) ) | ||||||||||||||||||||||||||||
) { | ||||||||||||||||||||||||||||
// Return null if value is not a valid URL. | ||||||||||||||||||||||||||||
attributesFromSources[ attributeName ] = null; | ||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||
attributesFromSources[ attributeName ] = value; | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
return { | ||||||||||||||||||||||||||||
...attributes, | ||||||||||||||||||||||||||||
...attributesFromSources, | ||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||
[ | ||||||||||||||||||||||||||||
attributes, | ||||||||||||||||||||||||||||
blockBindings, | ||||||||||||||||||||||||||||
clientId, | ||||||||||||||||||||||||||||
context, | ||||||||||||||||||||||||||||
name, | ||||||||||||||||||||||||||||
registeredSources, | ||||||||||||||||||||||||||||
] | ||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
const setBoundAttributes = useCallback( | ||||||||||||||||||||||||||||
( nextAttributes ) => { | ||||||||||||||||||||||||||||
if ( ! blockBindings ) { | ||||||||||||||||||||||||||||
setAttributes( nextAttributes ); | ||||||||||||||||||||||||||||
return; | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
registry.batch( () => { | ||||||||||||||||||||||||||||
const keptAttributes = { ...nextAttributes }; | ||||||||||||||||||||||||||||
const blockBindingsBySource = new Map(); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
// Loop only over the updated attributes to avoid modifying the bound ones that haven't changed. | ||||||||||||||||||||||||||||
for ( const [ attributeName, newValue ] of Object.entries( | ||||||||||||||||||||||||||||
keptAttributes | ||||||||||||||||||||||||||||
) ) { | ||||||||||||||||||||||||||||
if ( | ||||||||||||||||||||||||||||
! blockBindings[ attributeName ] || | ||||||||||||||||||||||||||||
! canBindAttribute( name, attributeName ) | ||||||||||||||||||||||||||||
) { | ||||||||||||||||||||||||||||
continue; | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
const binding = blockBindings[ attributeName ]; | ||||||||||||||||||||||||||||
const source = registeredSources[ binding?.source ]; | ||||||||||||||||||||||||||||
if ( ! source?.setValues ) { | ||||||||||||||||||||||||||||
continue; | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
blockBindingsBySource.set( source, { | ||||||||||||||||||||||||||||
...blockBindingsBySource.get( source ), | ||||||||||||||||||||||||||||
[ attributeName ]: { | ||||||||||||||||||||||||||||
args: binding.args, | ||||||||||||||||||||||||||||
newValue, | ||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||
} ); | ||||||||||||||||||||||||||||
delete keptAttributes[ attributeName ]; | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
if ( blockBindingsBySource.size ) { | ||||||||||||||||||||||||||||
for ( const [ | ||||||||||||||||||||||||||||
source, | ||||||||||||||||||||||||||||
bindings, | ||||||||||||||||||||||||||||
] of blockBindingsBySource ) { | ||||||||||||||||||||||||||||
source.setValues( { | ||||||||||||||||||||||||||||
select: registry.select, | ||||||||||||||||||||||||||||
dispatch: registry.dispatch, | ||||||||||||||||||||||||||||
context, | ||||||||||||||||||||||||||||
clientId, | ||||||||||||||||||||||||||||
bindings, | ||||||||||||||||||||||||||||
} ); | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
const hasParentPattern = !! context[ 'pattern/overrides' ]; | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
if ( | ||||||||||||||||||||||||||||
// Don't update non-connected attributes if the block is using pattern overrides | ||||||||||||||||||||||||||||
// and the editing is happening while overriding the pattern (not editing the original). | ||||||||||||||||||||||||||||
! ( hasPatternOverrides && hasParentPattern ) && | ||||||||||||||||||||||||||||
Object.keys( keptAttributes ).length | ||||||||||||||||||||||||||||
) { | ||||||||||||||||||||||||||||
// Don't update caption and href until they are supported. | ||||||||||||||||||||||||||||
if ( hasPatternOverrides ) { | ||||||||||||||||||||||||||||
delete keptAttributes.caption; | ||||||||||||||||||||||||||||
delete keptAttributes.href; | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
setAttributes( keptAttributes ); | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
} ); | ||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||
[ | ||||||||||||||||||||||||||||
blockBindings, | ||||||||||||||||||||||||||||
clientId, | ||||||||||||||||||||||||||||
context, | ||||||||||||||||||||||||||||
hasPatternOverrides, | ||||||||||||||||||||||||||||
setAttributes, | ||||||||||||||||||||||||||||
registeredSources, | ||||||||||||||||||||||||||||
name, | ||||||||||||||||||||||||||||
registry, | ||||||||||||||||||||||||||||
] | ||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
if ( ! blockType ) { | ||||||||||||||||||||||||||||
return null; | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
if ( blockType.apiVersion > 1 ) { | ||||||||||||||||||||||||||||
return <EditWithFilters { ...props } context={ context } />; | ||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||
<EditWithFilters | ||||||||||||||||||||||||||||
{ ...props } | ||||||||||||||||||||||||||||
attributes={ computedAttributes } | ||||||||||||||||||||||||||||
context={ context } | ||||||||||||||||||||||||||||
setAttributes={ setBoundAttributes } | ||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
// Generate a class name for the block's editable form. | ||||||||||||||||||||||||||||
|
@@ -77,15 +282,17 @@ const EditWithGeneratedProps = ( props ) => { | |||||||||||||||||||||||||||
: null; | ||||||||||||||||||||||||||||
const className = clsx( | ||||||||||||||||||||||||||||
generatedClassName, | ||||||||||||||||||||||||||||
attributes.className, | ||||||||||||||||||||||||||||
attributes?.className, | ||||||||||||||||||||||||||||
props.className | ||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||
<EditWithFilters | ||||||||||||||||||||||||||||
{ ...props } | ||||||||||||||||||||||||||||
context={ context } | ||||||||||||||||||||||||||||
attributes={ computedAttributes } | ||||||||||||||||||||||||||||
className={ className } | ||||||||||||||||||||||||||||
context={ context } | ||||||||||||||||||||||||||||
setAttributes={ setBoundAttributes } | ||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the previous declaration,
attributes
had a default empty object. Could this cause any issues if it is undefined? Maybe some developers are relying on it being an object inBlockEdit
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there was ever guaranteed that
attributes
is going to be an object due to historical reasons.computedAttributes
gets passed down instead, which is always an object when there are no bindings. Otherwise, it will be the same value as defined by the block. I'm not entirely sure why it would get mapped to an empty object in this place, but if we want to keep that, then we should use a stable reference instead of overriding it with a new empty object instance on every re-render.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the server, attributes can be set to
null
:https://github.com/WordPress/wordpress-develop/blob/32880ec4e910837cd267e1685733867b243e22fa/src/wp-includes/class-wp-block-type.php#L169-L175
The same applies to REST API response:
https://github.com/WordPress/wordpress-develop/blob/32880ec4e910837cd267e1685733867b243e22fa/src/wp-includes/rest-api/endpoints/class-wp-rest-block-types-controller.php#L503-L513
It was hardened in
block.json
schema:gutenberg/schemas/json/block.json
Lines 107 to 110 in af34d1e
However, when not provided on the server, that would still mean it's
null
.On the client, it defaults to an empty object:
gutenberg/packages/blocks/src/store/process-block-type.js
Lines 235 to 239 in af34d1e
However, I'm not entirely sure what happens if the server sets
null
when bootstraping the value.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for the block type processing,
attributes
will never benull
because it defines some attributes that are common to all blocks:metadata
andlock
: https://github.com/WordPress/wordpress-develop/blob/32880ec4e910837cd267e1685733867b243e22fa/src/wp-includes/class-wp-block-type.php#L282-L285.If
null
is a valid value for theBlockEdit
, I'm fine keeping it this way.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, it's probably impossible to see
attributes
set tonull
on the client based on the fact it always contains global attributes on the server:https://github.com/WordPress/wordpress-develop/blob/32880ec4e910837cd267e1685733867b243e22fa/src/wp-includes/class-wp-block-type.php#L553-L563