Try: Extensibility: Enable hook-based Slot/Fill registration

[aduth]

Related: #1352

This pull request seeks to explore an pattern for plugin UI extensibility by exposing slots on a global hooks registry.

Implementation notes:

These changes make use of @wordpress/hooks@0.1.0-beta.5 to add a new hooks registry to the editor returned instance, available at wp._wpGutenbergEditor.addFilter. The behavior here of adding a hooks registry specific to editor (vs. a global registry) is not settled, but enabled as part of changes introduced to the hooks module in WordPress/packages#40.

With these changes, a registerSlot action is monitored by a PluginSlots component after mounting. A plugin can fire this action to register a fill component by slot name. For example, entering the following in the browser developer tools console while running this branch will cause a message to be shown in the publish menu dropdown:

_wpGutenbergEditor.registerFill( 'publish-dropdown', () => 'Hello!' );

A few alternative approaches were considered:

• Not leveraging hooks behavior and instead creating a separate API for registering fills, exposed by the editor, i.e. editor.registerFill( name, FillComponent );
  • It was cumbersome to try to maintain communication between the top-level API and the component responsible for rendering the fills. PluginSlots here can instead leverage hooks context provided by HooksProvider.
• Treating fills as a filter triggered during PluginSlots mount, i.e. instead of this.state = { fills: {} };, doing this.state = { fills: applyFilters( 'registerFills', {} ) };
  • Ensuring that plugins manipulate the object as expected is error-prone (must concatenate with or create a default array on slot name)
  • Requires that plugins register to this filter by the time the component mounts, meaning it is not possible to register after page load
• Maintaining Fills in Redux state
  • Possible, and not too dissimilar to current approach, but this would require storing component objects in Redux state, and was decided against for the goal of keeping Redux state as a plain object.

The included HooksProvider component (and withHooks higher-order component) enable a hooks registry scoped to a specific React tree, rather than on a single global. Therefore, each editor instance returned by wp.editor.createEditorInstance will have its own hooks registry (similar to having its own Redux store via ReactReduxProvider).

To-Do:

• Document added components
• Test added components

Testing instructions:

Enter the following into your browser developer tools console after loading the editor and verify that a "Hello!" message is shown when then toggling the Publish dropdown menu:

_wpGutenbergEditor.doAction( 'registerFill', 'publish-dropdown', () => 'Hello!' );

[codecov]

Codecov Report

Merging #3321 into master will decrease coverage by 0.04%.
The diff coverage is 19.23%.

@@            Coverage Diff             @@
##           master    #3321      +/-   ##
==========================================
- Coverage   34.16%   34.12%   -0.05%     
==========================================
  Files         258      261       +3     
  Lines        6755     6779      +24     
  Branches     1224     1225       +1     
==========================================
+ Hits         2308     2313       +5     
- Misses       3754     3773      +19     
  Partials      693      693

Impacted Files	Coverage Δ
editor/index.js	0% <ø> (ø)	⬆️
editor/header/publish-dropdown/index.js	0% <0%> (ø)	⬆️
editor/components/plugin-fills/index.js	0% <0%> (ø)
editor/components/provider/index.js	11.11% <0%> (-1.39%)	⬇️
editor/layout/index.js	0% <0%> (ø)	⬆️
components/higher-order/with-hooks/provider.js	16.66% <16.66%> (ø)
components/higher-order/with-hooks/index.js	80% <80%> (ø)
... and 1 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2ce97d...2e89a58. Read the comment docs.

[gziolo]

I tried the following:

const H1 = wp.element.createElement( 'h1', {}, 'Hello World!' );
_wpGutenbergEditor.doAction( 'registerFill', 'publish-dropdown', () => H1 );

it worked like a charm:

I like the proposed solution. It's very clean and elegant. It would be a great start to make Gutenberg extendable. It has some limitations at the moment:

• You can only append an item.
• You can't replace already appended items.
• You can't inject an item on a specific position.

Is there plan to support more advanced use cases in this PR?

[jorgefilipecosta]

I really like this solution it is simple and works! I think it would be good to return some kind of identifier to the registerFill caller. Or receive a unique identifier from the caller. Using this identifier the artifact registering the component should be able to query if the component is registered or not (we would provide a mechanism to check), and remove or replace the component registered. I also think we should provide a way to check if the hook we want to register is available for us at the time or not. E.g there may be situations where we don't have a publish dropdown.

[youknowriad]

Do you think we could avoid having to register the hooks to the editor's instance. What if we have multiple instances?. I'm thinking a global like wp.postEditor.hooks could be better.

[aduth]

Do you think we could avoid having to register the hooks to the editor's instance. What if we have multiple instances?. I'm thinking a global like wp.postEditor.hooks could be better.

Right, this is a bigger open question about hooks generally. It seems in this case we have a few options:

1. A single hooks registry, e.g. wp.hooks.doAction
2. A hooks registry common to all editors, e.g. wp.postEditor.doAction
3. A hooks registry specific to a single instance of an editor, e.g. myEditor.doAction

On the last two, it depends if we think it would be common for multiple instances of an editor to exist in the same screen and, if so, whether it's sensible that each have distinct or share common extension behaviors. And generally whether we think the scoping adds value (clarity? predictable flow?)

It's not too dissimilar to as in Redux, we could have a module exporting a global shared instance of the store (import store from '../store';) rather than passing it around as an argument when initialized (usually through the React Redux Provider context).

[youknowriad]

Do we really need to update the fills after the initial render? I'm thinking the hooks could be global and just provide them in the editorSettings object. Could simplify things and avoid another provider.

[aduth]

Do we really need to update the fills after the initial render? I'm thinking the hooks could be global and just provide them in the editorSettings object. Could simplify things and avoid another provider.

This could work, and is similar to the second "alternative" option described in the original comment (with some downsides mentioned).

[aduth]

It has some limitations at the moment:

• You can only append an item.
• You can't replace already appended items.
• You can't inject an item on a specific position.

Is there plan to support more advanced use cases in this PR?

These limitations are to be expected as the behavior of Slot / Fill to be a mechanism for "insert thing at this specific position". Arguably it's even desirable, if per #1352 we want to be conscious of specifically where we enable extension points. Beyond what exists currently, it would be fairly easy to create multiple slot areas: I could imagine "Before" and "After" pairings, so it's not merely appending.

You may also be interested in the early exploration at #1023 which was more along the lines of extending or replacing the rendering behavior of a component. Another advantage that Slot / Fill has is that it's a much simpler concept for plugin authors to implement.

[aduth]

Also some familiarity between Slots and a add_action PHP callback where a user or plugin author can inject HTML, with some patterns (proposed and in some themes) for similar before/after content actions:

• https://core.trac.wordpress.org/ticket/19709
• https://core.trac.wordpress.org/ticket/10976
• https://core.trac.wordpress.org/ticket/18561

[gziolo]

Beyond what exists currently, it would be fairly easy to create multiple slot areas: I could imagine "Before" and "After" pairings, so it's not merely appending.

I have still a concern that we don't have control over which item gets appended first if multiple plugins compete to extend the same slot. I think this is why PHP hooks provide this $priority argument to give some level of control in such case.

You may also be interested in the early exploration at #1023 which was more along the lines of extending or replacing the rendering behavior of a component. Another advantage that Slot / Fill has is that it's a much simpler concept for plugin authors to implement.

Oh, that's interesting. I will look into it tomorrow. This is exactly what I would need to implement for Collaborative Editing. There is this use case where you need to freeze block when another user is editing it. So it seems like the easiest way would be to wrap a component with a parent component that would render a different state when a block is being edited or simply render a default state otherwise.

[aduth]

I have still a concern that we don't have control over which item gets appended first if multiple plugins compete to extend the same slot. I think this is why PHP hooks provide this $priority argument to give some level of control in such case.

This priority argument exists in the JavaScript implementation as well:

https://www.npmjs.com/package/@wordpress/hooks

[gziolo]

This priority argument exists in the JavaScript implementation as well:

https://www.npmjs.com/package/@wordpress/hooks

Nice 👍 In that case we only need to finalize how developers could consume this API. I think it's worth referencing here how it works for blocks at the moment:

window.wp.blocks.registerBlockType( 'block-name', {
    ...
} );

I'm seeing the following options proposed:

1. A single hooks registry, e.g. wp.hooks.doAction
2. A hooks registry common to all editors, e.g. wp.postEditor.doAction
3. A hooks registry specific to a single instance of an editor, e.g. myEditor.doAction

So the question is if we want to hide the fact that we use hooks API behind the scenes and follow the pattern started with blocks, which would translate into something like:

const H1 = wp.element.createElement( 'h1', {}, 'Hello World!' );
window.wp.editor.registerFill( 'publish-dropdown', () => H1 );

which would be close to the option (2) that updates all editor instances. Whatever we pick here, it seems like registering a new block type should follow the pattern.

[aduth]

@gziolo Yes, there's a general overarching question as to whether we want to expose the raw actions/filters interface, or abstract them with specific APIs that may or may not use hooks as the implementation. I think it may come down to whether want to encourage a proliferation of actions and filters as we see in PHP, to the extent where creating specific APIs for each would be more confusing/redundant than helpful.

A conservative approach would be to start with the specific APIs, avoid exposing the hooks, and reconsider later if we find that we are encountering said confusion. In the case of registering fills, since Slot/Fill gives us a good amount of flexibility, it should have a broad impact in allowing UI extensibility.

#3330 can serve as a working overview of where we expect extensibility to be needed.

[gziolo]

Personally, I’m fine with exposing it as an experimental feature with API like wp.editor.__experimental_registerFill to battle tests it with real code. We can refine it later once we confirm it works nicely with the existing plugins.

[youknowriad]

Let's get this shipped to try it in the Colloborative Editing Plugin.

I still have a preference for wp.postEditor.hooks.doAction but I don't mind us trying the instance hook.

We should make sure to document this as experimental, with Docs or my renaming this to myEditor.experimentalHooks.doAction

[aduth]

Rebased to account for folder restructuring of #3389.

Small refactor in a448cb7 to no longer expose the full hooks registry, but rather only currently a singular slot registry API, e.g. _wpGutenbergEditor.registerFill( 'fillName', Component )

[mcsf]

This looks really good as a pattern and as a first step to extensibility.

I favor a conservative roll-out of the API, so the recent addition of the editor.registerFill gatekeeper is 👍 .

I also think we should provide a way to check if the hook we want to register is available for us at the time or not. E.g there may be situations where we don't have a publish dropdown.

I think we can explore this as we go, along with starting to expose app state to 3rd-party consumers and seeing where that takes us.

[gziolo]

Was it tested with the recently added recreate editor instance feature? createHooks() makes me think that there is going to be a new object created with a fresh state, so we might lose all registered hooks.

[gziolo]

2nd thing we should figure out pretty soon is how it relates to createHooks() call for blocks: https://github.com/WordPress/gutenberg/blob/master/blocks/hooks/index.js#L11.

Unless createHooks uses singletone pattern and it isn't an issue at all :)

[adamsilverstein]

I don't think it does @gziolo - did you ever look into this?

[gziolo]

It doesn't use singletone . It's fine as it is, we took a completely different approach anyways :)

[gziolo]

Okey, I can confirm that whenever an error is thrown inside the editor, it is not possible to recover using the newly introduced Attempt Recovery feature. The easiest way to do it is to register the following fill:

_wpGutenbergEditor.registerFill( 'publish-dropdown', () => { throw new Error() } );

It's something that doesn't make too much sense but allows to test it with easy

Once you execute this code snippet on your JS console, you need to click Publish button in the top toolbar. You will see The editor has encountered an unexpected error. message. When I click Attempt Recovery Gutenberg turns into the white screen and I see the following message on the console:

TypeError: Cannot read property 'hooks' of null

[gziolo]

I'm not opposed to merging it as it is, but wanted to make sure we keep track of it.

[gziolo]

I think we discovered other ways to achieve a similar goal. I'm closing this for now. We can also cycle back if we find out that the hooks package doesn't allow us to extend everything we need.