I think I found a bit of a better solution for this. As best I can tell, decorations themselves never get normalized. This is why we end up with null values on the points even though they should be getting normalized away.
There are two entry points for creating decorations setDecorations() and decorateNode(). The approach I'm thinking is that Decoration should have a normalization method that looks like:
normalize(node) {
return this.merge({
anchor: this.anchor.normalize(node),
focus: this.focus.normalize(node),
mark: this.mark.normalize(node),
})
}
And then we should normalize when we have access to a node. So in getDecorations(), we normalize before they get used anywhere:
return decorations.map(dec => dec.normalize(this))
We need to do it here, because getDecorations() gets called when the document is being rendered already, and we can't wait for a top level normalization. This seems to work pretty well locally but I havne't done extensive testing. I'm most worried about performance.
We also need to normalize at the top level in case someone passes decorations in directly through something like setDecorations(). I think this needs to happen in Editor#normalize() along with the document normalization, but still wrapping my head around how this would work. Probably needs to happen after the document finishes normalizing, and just another map to normalization like the example above.
@ianstormtaylor What do you think?
This is by far the most common error coming out of our Slate editor… @ericedem still thinking of taking on this fix?
@tommoor Ahh sorry, this one fell off my radar. This is the branch I was working out of is here if you want to take a look at the code for your own PR. I haven't been working on Slate much recently but will probably be digging back in in the next few weeks and might be able to tackle this.
Fixed it here https://github.com/ianstormtaylor/slate/pull/2698
Most helpful comment
I think I found a bit of a better solution for this. As best I can tell, decorations themselves never get normalized. This is why we end up with
nullvalues on the points even though they should be getting normalized away.There are two entry points for creating decorations
setDecorations()anddecorateNode(). The approach I'm thinking is thatDecorationshould have a normalization method that looks like:And then we should normalize when we have access to a node. So in
getDecorations(), we normalize before they get used anywhere:We need to do it here, because
getDecorations()gets called when the document is being rendered already, and we can't wait for a top level normalization. This seems to work pretty well locally but I havne't done extensive testing. I'm most worried about performance.We also need to normalize at the top level in case someone passes decorations in directly through something like
setDecorations(). I think this needs to happen inEditor#normalize()along with the document normalization, but still wrapping my head around how this would work. Probably needs to happen after the document finishes normalizing, and just another map to normalization like the example above.@ianstormtaylor What do you think?