slate icon indicating copy to clipboard operation
slate copied to clipboard

Replace / Rewrap node Transform

Open JosNun opened this issue 2 years ago • 1 comments

Problem Changing a node of one type into a node of another type is a fairly common task. The examples use setNodes to change the type of a node, but there's a subtle problem with this approach; any additional fields on the initial node are also present on the new node. image

In and of itself, this is a fairly benign issue. Extra properties don't usually cause any unexpected behavior. What becomes interesting, though, is how slate-history relies on the superfluous properties to undo an operation. The implementation of setNodes doesn't add the extra properties to the properties field on the undo item, and undoing requires that they be in the node for the undo to be successful.

Again, in isolation, this works. The problems begin to present themselves when serializing the nodes, or when dealing with TS.

When serializing the nodes, you need to account for the fact that there may be extra properties on the nodes. If you don't include the extra properties when rehydrating the editor with the nodes, the undo stack is unable to properly restore the correct fields onto the node, resulting in a potentially invalid node state.

Additionally, typing nodes becomes... challenging. When writing types for your nodes, it's easy to assume that a node of a given type will always have a given set of properties. In reality, however, that's not the case. There may be additional properties on a node if setNodes was used used to change the type of an entry. This can lead to edge cases when generating nodes from code, since there may be editor state contained in the nodes (in the form of extra properties) that is easy to miss when generating nodes yourself.

Solution A replaceNode function that allows you to replace a node with another node, and properly track property changes in history, so undo / redo isn't dependent on state stored within the nodes.

Alternatives Alternative workarounds in this case include removing the node to replace, then inserting a new node (requires updating the selection, and handle it in the undo case). Another option (the one I ultimately implemented) is to insert the new node as a child of the initial node, then use unwrapNodes to make it replace the original node.

Context Admittedly, the replaceNode solution seems like a fairly insignificant shim to workaround the fact that slate nodes are mutated, and aren't necessarily the same as their types would suggest. This may just be my ignorance of the internals of Slate, but the extra properties on objects was very unexpected for me, and lead to a case where my serialization and deserialization of the editor content was lossy (again, my error).

I guess a larger question tangentially related is whether the reliance on mutation (and extra fields in nodes) is good behavior? I'm sure there was a reason Slate was initially implemented like this, but I can't help but think that this pattern doesn't lend itself well to types, nor the immutability patterns that react uses? More research suggests that this less of a new issue, and more of a continuation of the discussion in https://github.com/ianstormtaylor/slate/issues/4173.

JosNun avatar Sep 20 '22 22:09 JosNun

Any update about it?

filippozanfini avatar Feb 25 '23 15:02 filippozanfini