Slate: remove the "stack", use HOCs instead

[ianstormtaylor]

@rockymeza had the very interesting idea, in our Slack, of writing plugins as React components. Potentially even using HOCs to re-implement all of the logic that is currently handled by the custom plugin system.

I think this is really interesting, and has the potential to simplify some things in core, and kinda seems obvious now that I think about it, which good ideas often do.

Haven't fully fleshed this out yet, but...

• We could get rid of the Stack concept, since HOCs would imply the ordering, and they can easily override or passthrough to the Editor as they please. This would also eliminate the potential hidden performance impact to re-creating the stack on every render, since it would be obvious that re-creating the Component on every render would have negative performance impacts.
• We could remove the render and renderPortal properties of plugins, since they would be supported automatically with HOCs.
• We might be able to remove storing state in this.state.state, and instead just use this.props.state, simplifying some logic.

If you have thoughts, please add! Especially if there are holes that aren't possible with HOCs that we'd need to figure out.

[oyeanuj]

@ianstormtaylor this sounds good as I followed the Slack discussion a little bit.

One other pro that comes to mind is that it really simplifies passing props to plugins.

On the other hand, a con that comes to mind is that if one has a lot of plugins that don't render (and act onPaste, or onKeyDown), then every re-render of the editor might needlessly render more than a dozen HoCs. Likewise, if we have a prop passed to a plugin and it changes, it causes the re-render of the whole nest. But then again, I am not sure if that is already how stack behaves though.

[rockymeza]

@ianstormtaylor so I did play around with some implementations. The easiest implementation is really a no-brainer.

const withDisableKeys = keys => Editor =>
  class DisableKeyPlugin extends React.Component {
    handleKeyDown = (event) => {
      if (keys.includes(event.key)) {
        event.preventDefault();
        console.log('DisableKeyPlugin disabled key!');
        return;
      }
      if (this.props.onKeyDown) {
        return this.props.onKeyDown(event);
      }
    };

    render() {
      return (
        <Editor
          {...this.props}
          onKeyDown={this.handleKeyDown}
        />
      );
    }
  };

const MyFancyEditor = withDisableKeys(['a', 'e', 'i', 'o', 'u'])(Editor);

This just works by itself, I think the only downside to it is that you have to call this.props.onKeyDown in your own onKeyDown. This is not very DRY, and doesn't work like the plugins worked. I'm not sure if that's a problem. I did find a way to work around it, but it was by creating something very similar to a stack underneath. That definitely feels kind of hacky though.

As for how to pass props down to plugins, I was thinking that it could sort of work like this.

• LinkExtractorPlugin takes a onFoundURL callback.
• all plugins forward all props they receive except for ones that they know are just for them.

// LinkExtractorPlugin.render
const { onFoundURL, ...otherProps } = this.props;
return <Editor {...otherProps} />;

// Usage
const MyFancyEditor = withLinkExtractor(Editor);

// render
<MyFancyEditor
  state={this.state.state}
  onChange={this.handleEditorChange}
  onFoundURL={this.handleFoundURL}
/>

wrt @oyeanuj's comment, if the components below LinkExtractorPlugin are PureComponents and onFoundURL changes, they won't re-render because they aren't being passed that prop anyway. This is also the standard React way of optimizing code.

[ms88privat]

I think doing this with HOC is a very good idea. I somebody actively working on it right now?

[kgdev]

How far are we from this HOC idea?

[AlbertHilb]

Editor plugins in the form of High Order Components are much convenient when dealing with rendering the editor and managing its state changes.
In particular when a plugin relies on other plugins and must pass data to them; passing data as props is much more slick than using the State data field.
However, as already noted, when treating event handlers or schema objects they are less comfortable for the authors.

For that reason I propose a mixed approach.

Plugins remain objects with event handlers and schema entries, but render and onChange properties (and possibly some others) are replaced by an HOC property.
Then, those objects are passed to an editor factory function (see issue #1013) that generates the actual editor component.

[danburzo]

In terms of event handling, working with a HOC pattern offers a few advantages:

• Deciding the order in which to execute handlers: each plugin can either call its own handler _before_ or _after_ its parent HOC; fallback-type plugins may choose to invoke this.props[method] before this[method].
• Augmenting the handler data — a clipboard reader may supplement the handlers with ready-to-use data in various formats; a CompositionListener may provide isComposing to other handlers.

To address the loss of brevity and make it easy to migrate the existing userland code, we could even devise a LegacyPlugin HOC that takes the plugin definition and makes it conform to the new way of doing things:

_(Pseudo-JS/Pseudo-React pseudocode, might be using wrong lifecycle methods here)_

const legacyPlugin = opts => Editor => 
    class LegacyPlugin extends React.Component {

        constructor(props) {
            super(props);

            // Deal with event listeners

            this.handlers = {};

            [
                //...
                'onKeyDown',
                'onKeyUp',
                //...
            ].forEach(method => {
                if (opts[method]) {
                    this.handlers[method] = (...args) => {
                        var ret = null;
                        if (this.props[method]) {
                            var ret = this.props[method].call(...args);
                        }
                        return ret || opts[method].call(...args);
                    }
                }
            });

            if (opts.schema) {
                // append its own schema
            }
        }

        render() {
            return <Editor {...this.props} {...this.handlers}></Editor>
        }
    }

[danburzo]

It sounds like it's essential for HOCs to be PureComponents to achieve maximum performance, but I do wonder, as @oyeanuj, what the performance penalty will be when chaining a dozen HOCs. Having something like this.props.state propagate through the entire chain will still cause some non-trivial computation, no?

[AlbertHilb]

Hi @danburzo.

Deciding the order in which to execute handlers

This is precisely what must be avoided.
The logic of the plugin system is the following: higher order plugins must be able to override the behavior of lower. To achieve this we must ensure, for example, that event handlers of higher order plugins are executed before handlers of lower order plugins.
Enforcing this rule is one of the problems of the HOC pattern.

Augmenting the handler data

For this there is not need for HOC pattern, it can be achieved right now, just add fields to the data object parameter passed to the handler.

[danburzo]

The logic of the plugin system is the following: higher order plugins must be able to override the behavior of lower.

Yes, that sounds right, I see how this could complicate things. I was thinking of the handles that are currently in the Content component (onBeforeInput, onInput etc.) that could benefit from refactoring as a plugin (or series of plugins), but they contain the sort of behavior that needs to happen _before_ all other plugins.

[rockymeza]

I have managed to get some time from work to work on this the next two weeks. I'm going to put a base together and port some of our internal plugins to it. I'm going to try to come with something as ergonomic as possible first and then think about how to port the legacy plugins to it. I will post back here when I have something to look at.

[ms88privat]

@danburzo About performance of multiple HOC's. There are attempts to combine multiple HOC's into one HOC automatically by various "recompose" libraries

[AlbertHilb]

A way to make writing event handlers for HOC Slate plugins easier is using decorators:

With a decorator like this at their disposal

const handler = (target, name, descriptor) => {
  const original = descriptor.value;

  if (typeof original !== 'function') return descriptor;

  descriptor.value = function extendedCallback(...args) {
    const propsCallback = this.props[name];

    if (propsCallback) {
      const res = propsCallback.apply(this, args);
      if (res) return res;
    }

    return original.apply(this, args);
  };

  return descriptor;
};

plugin authors just have to take care of the actual logic of their handlers.

const Plugin = Editor =>
  class ExtendedEditor extends React.Component {
    @handler
    onKeyDown(e, data, state) {
      ...
    }

    render() {
      const { onKeyDown, ...otherProps } = this.props;

      return (
        <Editor onKeyDown={this.onKeyDown} {...otherProps} />
      );
    }
  }

Decorators are not yet part of the javascript standard but are on the way; in the mean time one can use babel decorators transform.

[ianstormtaylor]

Closing because this didn't really seem to get us anywhere. And HOCs are kind of becoming extent with render props these days. Our plugin system so far seems better than either for the things it needs to be able to achieve.