Decorate Nested Leaf Unsuccessful

[jolanglinais]

Do you want to request a feature or report a bug?

Bug 🐛

What's the current behavior?

Context

There are 3 leaf nodes and a React state hovering which is being updated. It seems like leaves are not re-rendering on React state change, or that we are losing leaf nodes or they are not being recognized as leaf nodes. This may be some optimization or normalization under the hood?

However, I have managed it to work as expected in a very specific circumstance, IFF onChange is triggered on the target leaf, the selection is in the target leaf node, and React state is updated. Just changing the React state does not trigger it, typing in the leaf node is required.

Sandbox of issue reproduction

https://codesandbox.io/s/slate-repo-irmerk-25-06-2020-sq8r0

Steps to reproduce

1. Hover over italicized text, note that 25 is not bold
2. Click and type within the italicized text, note that 25 is not bold
3. Click and type within 25, note that 25 is not bold
4. Click in 25
5. Hover over italicized text
6. Type anything
7. Note the bold

Slate: 0.58.0 Browser: Chrome OS: Mac

What's the expected behavior?

React hovering state change should re-render all 3 leaves. It should decorate the middle (variable) leaf node.

[jolanglinais]

Note that I am currently attempting Transforms.setNodes() as a different approach from decorate to the same objective.

[pbirsinger]

YES. This is a huge thing that does not work for me either. Using Transforms.setNodes() works, but I want to use a decorator. What gives??

[arimah]

I've run into what I'm pretty sure is the same bug! I believe the error is in the MemoizedText's dirty check:

const MemoizedText = React.memo(Text, (prev, next) => {
  return (
    next.parent === prev.parent &&
    next.isLast === prev.isLast &&
    next.renderLeaf === prev.renderLeaf &&
    next.text === prev.text
  )
})

As you can see, the decorations prop is missing, so React will dutifully reuse the component's last state and not re-render it. That, of course, is why typing updates the leaf: it changes the text prop.

Compare that with MemoizedElement's dirty check:

const MemoizedElement = React.memo(Element, (prev, next) => {
  return (
    prev.decorate === next.decorate &&
    prev.element === next.element &&
    prev.renderElement === next.renderElement &&
    prev.renderLeaf === next.renderLeaf &&
    isRangeListEqual(prev.decorations, next.decorations) && // <-- HERE
    (prev.selection === next.selection ||
      (!!prev.selection &&
        !!next.selection &&
        Range.equals(prev.selection, next.selection)))
  )
})

I think it should be sufficient to add a check for decorations to MemoizedText. I'll experiment a bit and possibly produce a PR.

[hdsuperman]

@arimah I have the same problem, solved by:

var MemoizedText = React.memo(Text, (prev, next) => {
  return next.parent === prev.parent &&
      next.isLast === prev.isLast &&
      next.renderLeaf === prev.renderLeaf &&
      next.text === prev.text &&
      isRangeListEqual(prev.decorations, next.decorations);
});

add isRangeListEqual(prev.decorations, next.decorations) in file: node_modules/slate-react/dist/index.es.js