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.

Sign up for GitHub

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

Automatically use a subregistry when using the block editor provider #14678

Merged
merged 3 commits into from
Mar 29, 2019
Merged

Automatically use a subregistry when using the block editor provider #14678

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0dac41d
Automatically use a subregistry when using the block editor provider
youknowriad Mar 28, 2019
92bd78b
Defer the hooks usage
youknowriad Mar 28, 2019
4cb7c09
set sub registry instead of update registry
youknowriad Mar 29, 2019
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
9 changes: 7 additions & 2 deletions packages/block-editor/src/components/provider/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,14 @@
*/
import { Component } from '@wordpress/element';
import { DropZoneProvider, SlotFillProvider } from '@wordpress/components';
import { withDispatch, withRegistry } from '@wordpress/data';
import { withDispatch } from '@wordpress/data';
import { compose } from '@wordpress/compose';

/**
* Internal dependencies
*/
import withRegistryProvider from './with-registry-provider';

class BlockEditorProvider extends Component {
componentDidMount() {
this.props.updateSettings( this.props.settings );
Expand Down Expand Up @@ -115,6 +120,7 @@ class BlockEditorProvider extends Component {
}

export default compose( [
withRegistryProvider,
withDispatch( ( dispatch ) => {
const {
updateSettings,
Expand All @@ -126,5 +132,4 @@ export default compose( [
resetBlocks,
};
} ),
withRegistry,
] )( BlockEditorProvider );
41 changes: 41 additions & 0 deletions packages/block-editor/src/components/provider/with-registry-provider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/**
* WordPress dependencies
*/
import { useState, useEffect } from '@wordpress/element';
import { withRegistry, createRegistry, RegistryProvider } from '@wordpress/data';
import { createHigherOrderComponent } from '@wordpress/compose';

/**
* Internal dependencies
*/
import { storeConfig } from '../../store';
import applyMiddlewares from '../../store/middlewares';

const withRegistryProvider = createHigherOrderComponent( ( WrappedComponent ) => {
return withRegistry( ( { useSubRegistry = true, registry, ...props } ) => {
if ( ! useSubRegistry ) {
return <WrappedComponent registry={ registry } { ...props } />;
}

const [ subRegistry, setSubRegistry ] = useState( null );
useEffect( () => {
const newRegistry = createRegistry( {}, registry );
Copy link
Member

Choose a reason for hiding this comment

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

It took me a while to understand what's happening here:

  • so it creates a new registry and points the passed registry as a parent
  • overrides core/block-editor
  • does some magic with middlewares

Copy link
Member

Choose a reason for hiding this comment

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

Would it work as well?

Suggested change
const newRegistry = createRegistry( {}, registry );
const newRegistry = createRegistry( { 'core/block-editor': storeConfig }, registry );

This is what I read from:
https://github.com/WordPress/gutenberg/blob/master/packages/data/src/registry.js#L181

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, previously this was not possible because the controls plugins was not built-in but we can do this now. Good suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I guess we still can't do that yet because we need the returned "store" object to apply middlewares.

Copy link
Member

Choose a reason for hiding this comment

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

we need the returned "store" object to apply middlewares.

For now, anyways. @nerrad has been working to eliminate this (e.g. #14594).

(No action implied to be needed here)

const store = newRegistry.registerStore( 'core/block-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 registry={ subRegistry } { ...props } />
</RegistryProvider>
);
} );
}, 'withRegistryProvider' );

export default withRegistryProvider;
6 changes: 5 additions & 1 deletion packages/block-editor/src/store/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,15 @@ import controls from './controls';
*/
const MODULE_KEY = 'core/block-editor';

const store = registerStore( MODULE_KEY, {
export const storeConfig = {
reducer,
selectors,
actions,
controls,
};

const store = registerStore( MODULE_KEY, {
...storeConfig,
persist: [ 'preferences' ],
} );
applyMiddlewares( store );
Expand Down
1 change: 1 addition & 0 deletions packages/editor/src/components/provider/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ class EditorProvider extends Component {
onInput={ resetEditorBlocksWithoutUndoLevel }
onChange={ resetEditorBlocks }
settings={ editorSettings }
useSubRegistry={ false }
>
{ children }
<ReusableBlocksButtons />
Expand Down