-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Editor: Create own sub-registry in default EditorProvider use #15989
Changes from all commits
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 |
---|---|---|
@@ -0,0 +1,46 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { useState, useEffect } from '@wordpress/element'; | ||
import { withRegistry, createRegistry, RegistryProvider } from '@wordpress/data'; | ||
import { createHigherOrderComponent } from '@wordpress/compose'; | ||
import { storeConfig as blockEditorStoreConfig } from '@wordpress/block-editor'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import { storeConfig } from '../../store'; | ||
import applyMiddlewares from '../../store/middlewares'; | ||
|
||
const withRegistryProvider = createHigherOrderComponent( | ||
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. I feel like I already wrote this code before. Do you think there's value in exposing a generic version of this in the data module? (I'm on the fence personally) 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.
It is identical to the one in |
||
( WrappedComponent ) => withRegistry( ( props ) => { | ||
const { useSubRegistry = true, registry, ...additionalProps } = props; | ||
if ( ! useSubRegistry ) { | ||
return <WrappedComponent { ...additionalProps } />; | ||
} | ||
|
||
const [ subRegistry, setSubRegistry ] = useState( null ); | ||
useEffect( () => { | ||
const newRegistry = createRegistry( { | ||
'core/block-editor': blockEditorStoreConfig, | ||
}, registry ); | ||
const store = newRegistry.registerStore( 'core/editor', storeConfig ); | ||
// This should be removed after the refactoring of the effects to controls. | ||
applyMiddlewares( store ); | ||
setSubRegistry( newRegistry ); | ||
}, [ registry ] ); | ||
|
||
if ( ! subRegistry ) { | ||
return null; | ||
} | ||
|
||
return ( | ||
<RegistryProvider value={ subRegistry }> | ||
<WrappedComponent { ...additionalProps } /> | ||
</RegistryProvider> | ||
); | ||
} ), | ||
'withRegistryProvider' | ||
); | ||
|
||
export default withRegistryProvider; |
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 feel this prop should be passed down from
editor/provider
toblock-editor/provider
as well right (unless we access theblock-editor
store from theeditor
components (which we probably do on a second thought but maybe we shouldn't) ?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.
Right now we just explicitly pass
false
:gutenberg/packages/editor/src/components/provider/index.js
Line 159 in 42b2748
Which I think is how we want it to be:
useSubRegistry={ false }
to its BlockEditorProvider, but that block editor still operates in the same subregistry as created for the EditorProviderIn the end, we get a flatter hierarchy of registries. For the main post editor, we still only have a single registry.
For what becomes of the embedded editor in #14715, there is a subregistry which is used both by the EditorProvider and its BlockEditorProvider.
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.
Makes sense, thanks for the explanation.