Add onSetValue plugin hook

[aiwenar]

Is this adding or improving a feature or fixing a bug?

feature

What's the new behavior?

Currently plugins can only observe changes in value via the onChange hook. This presents two problems for plugins which derive their state from value:

• If they use that state to influence rendering (e.g. in renderEditor), the editor will render incorrectly until it's first focused, which is the first time onChange is run, and the first time the plugin can derive state

• If value is changed via setValue they have no way of detecting this change and updating their state.

This PR adds a new plugin hook, onSetValue(editor, next), which is invoked with the new value when it is changed via setValue. This addresses both points:

• Since Editor's constructor uses setValue, plugins will receive onSetValue right after onConstruct, allowing them to inspect value before editor is first rendered;

• Since the only ways to set value are setValue and applyOperation (I'm assuming setting editor.value manually is not a supported use case), plugins can now observe all changes to value.

How does this change work?

Have you checked that...?

• The new code matches the existing patterns and styles.
• The tests pass with yarn test — all existing tests pass, but I probably should add some and I'm not sure how to test this.
• The linter passes with yarn lint. (Fix errors with yarn prettier.)
• The relevant examples still work. (Run examples with yarn watch.)

Does this fix any issues or need any specific reviewers?

Fixes: #
Reviewers: @

[ianstormtaylor]

What's a use case where this is required? In renderEditor why not use the existing editor's value instead of one that is cached from onChange?

[jas7457]

@ianstormtaylor - a use case that I've wanted for this is the following case:
We have special "placeholder" syntax in our strings - "You must answer {number} correct to continue." We are searching for this text and adding an isVoid mark around the text. It would be great if we had a way to do this once, up front, and not on each change or normalize event. The regex search, especially over a large document, can add up, and we really only care about doing this check initially after the value has been read in.

[ianstormtaylor]

@jas7457 if that's the case then why not do it when the value is first serialized?

[jas7457]

Do you have an example of how to do this with slate-html-serializer? My understanding was deserialize took one html node and returned one block/mark/etc, but now my one html node (say, paragraph), can have 0-n placeholder marks, which aren't html elements

[adjourn]

@jas7457 Good timing, Im working on similar issue right now.

My use case is to replace all emojis with custom inline elements.

Main task is to take a single text node e.g 'Lit AF 🔥😂👨🏼‍🚒 emojis next to each other' and split it into several parts, in this case: [TextNode, EmojiInline, EmojiInline, EmojiInline, TextNode].

The main point Im (trying) to make here is that you can build nodes yourself and pass them to objects instead of el.childNodes, even mix of both. That's how you escape one-on-one mapping.

Please note that this is WIP code, but it's not just a theory, I've already got a working solution.
Im sure there's plenty to improve, Im still exploring myself, e.g just learned this.

{
  // rule example (blocks)
  deserialize(el, next) {
    const block = BLOCK_TAGS[el.tagName.toLowerCase()];
    if (block) {
      // call "processChildren" in every deserializer that could
      // have text as direct children (could contain emojis)

      // you can find example of "nodes" below
      const nodes = processChildren(el.childNodes);
      return {
        object: "block",
        type: block,
        // pass processed nodes instead of "el.childNodes"
        // use "next" because "nodes" could contain DOM nodes (in my case)
        nodes: next(nodes)
      };
    }
  }
},

// ...

// function that recieves original child nodes
// and returns what ever nodes you need
function processChildren(children) {
  let nodes = [];
  for (let i = 0; i < children.length; i++) {
    if (children[i].nodeType === Node.TEXT_NODE) {
      // text node
      if (children[i].length > 0) {
        // returns "false" if no emojis present
        // otherwise, example: [TextNode, EmojiInline, TextNode]
        const parts = splitTextWithEmojis(children[i].wholeText);
        if (parts) {
          nodes.push(...parts);
        } else {
          // if contains no emojis, push text DOM
          // TODO: might be better to push Slate text
          // object directly and save few "deserialize" calls
          nodes.push(children[i]);
        }
      }
    } else {
      // not text node
      // TODO: requires more testing ..
      // safe to push DOM node because it's handed over to next
      // round of deserializers which also call "processChildren", 
      // whole tree/every node gets processed recursively
      nodes.push(children[i]);
    }
  }
  return nodes;
}

// "nodes" example
[
  { // TextNode
    object: "text",
    leaves: [
      {
        object: "leaf",
        text: 'Lit AF '
      }
    ]
  },
  { // EmojiInline
    object: "inline",
    type: "emoji",
    nodes: [
      {
        object: "text",
        leaves: [
          {
            object: "leaf",
            text: '😂'
          }
        ]
      }
    ]
  }
  // etc ..
]

// because "nodes" could contain DOM nodes and already resolved
// Slate objects, I have last deserializer to sort things out
// everything works but this is not my final iteration of this
{
  // case for items that fell through other rules
  deserialize(el, next) {
    if (el.tagName) {
      // spans don't match other cases but could contain emojis
      if (el.tagName.toLowerCase() === "span") {
        const nodes = processChildren(el.childNodes);
        return next(nodes);
      }
      // TODO: other tags that don't match previous
      // deserialize cases but could contain emojis
    } 

    if (el.type === "emoji") {
      // emoji inline, done with it
      return el;
    } else if (el.object === "text") {
      // already processed text, done with it
      return el;
    } else {
      // haven't checked if and what fells here
      // but Im guessing DOM nodes in which case
      // hand them over to new deserialization loop
      return next();
    }
  }
}

[jas7457]

@adjourn that is great timing! I'll have to toy around with this for my use case, but it shouldn't be hard to adopt this to work. Thanks for the great example + comments!

[aiwenar]

@ianstormtaylor Sorry for the delay.

We're working on a book editor, and one of features we have to support are numbered elements (figures, tables, etc.) and references to them (e.g. Figure 15 in as portrayed on Figure 15.). This is something that would be easily done using CSS Generated Content for Paged Media Module's target-counter, but there are no browsers implementing this standard, and I don't know of any polyfills.

Instead I implemented katalysteducation/slate-counters, which works somewhat like CSS counters and target-counter, except it operates on Slate's model instead of HTML DOM. It works by first calculating state of counters on every node in the document, and after each change recalculating counters in the changed subtree, and propagating changes to other nodes which may be affected.

There are three issues with it that prompted me to suggest onSetValue:

1. Since we don't have access to value until first onChange we can't display references correctly until user focuses the editor;

2. Calculating initial counter state is very expensive compared to just updating it, taking around 2 seconds on larger documents (though I haven't benchmarked it yet, so there may be other things contributing). Such slowdown is acceptable when document is being loaded, but not in response to user's action (right now it happens when user focuses the editor, see point 1.).

3. If editor's value changes for any reason other that due to an operation(s) being applied (Editor#setValue), it'll either lead to it being incorrectly displayed (best case) or our plugin outright crashing, as it'll still think it's operating on the previous document.

[ianstormtaylor]

@aiwenar that use case sounds like something to solve by any other possible means. Needing to re-process all nodes when any changes is not great. And keeping presentation-only data that is order-specific in the Slate data model isn’t really advised.

It seems to me like something you could handle with React-and-DOM-level effects instead.

[ianstormtaylor]

As of #3093 (which was just merged), I believe this issue is no longer applicable, because a lot has changed. I'm going through and closing out any potential issues that are not out of date with the overhaul. Thanks for understanding.