Slate: Provide a `getInsertionPoint` query

[justinweiss]

Do you want to request a _feature_ or report a _bug_?

Request a feature. (See https://github.com/ianstormtaylor/slate/issues/2865)

What's the expected behavior?

At the boundary between two text nodes, or a text and an inline node, we have points that are ambiguous. Should the point be at the end of the preceding node? Or at the beginning of the next node?

Different browsers have different behaviors for their native selections. To avoid edge cases and make things easier for plugin devs and slate-react itself, that point should be normalized consistently. I think the default today is always "end of the preceding node."

Plugins like slate-sticky-inlines would like more control over that normalization, so you can edit the edges of nodes. So this should also be customizable.

To solve both these problems, Slate should use and provide a getInsertionPoint query. This query would take a point and return a normalized point. Slate would use it internally when normalizing points, so plugins could customize it with their own behavior.

[justinweiss]

I've been exploring this, and I think it will be tricky to get it consistent everywhere. There are many places inside Slate where normalize / resolveRange / resolveSelection / resolvePoint are being called, and in most of those places, we don't have an editor to run queries on. It seems like we have a couple of options:

1. Thread editor through all the methods that eventually resolve ranges
2. Have the internal range normalizing only partially normalize, and leave ambiguity-resolving to the editor query

Option 2 is a lot cleaner, and much less annoying for anyone working directly with the Slate data model. But it's weird (and probably buggy) to have different resolving logic depending on where you are.

That said, it might be OK. If we use getInsertionPoint when updating the DOM selection and when inserting text or nodes, I don't think anyone would be able to tell the difference.

We make pretty heavy use of sticky inlines, and so I think my overall preference would be for Slate not to care about normalizing end-of-last-node / beginning-of-next-node. It _seems_ like those two positions should refer to the same thing, but depending on how you're rendering inlines and marks, they could be entirely different. Leaving it to the user to handle seems more flexible, but I don't see a clean way to do that with how often points are normalized today.

(This is kind of how things work right now, because of a bug: https://github.com/ianstormtaylor/slate/pull/2933, but that bug also breaks other things in Safari).

[ianstormtaylor]

@justinweiss I think going with (1) makes sense. All resolution being editor-based makes sense to me since the schema might affect some kinds of resolution behaviors.

[justinweiss]

That makes sense -- in slate-react, for example, selection normalization depends on isVoid. For things like https://github.com/ianstormtaylor/slate/blob/master/packages/slate-hyperscript/src/creators.js#L353 where we don't naturally have an editor, would you create a dummy editor to use? Or maybe provide a way for a hyperscript user to provide an editor?

[ianstormtaylor]

That’s a good question. I’d lean towards making it not required to normalize the selection in hyperscript I think. Such that it can actually be created with unnormalized selections. Potentially throwing errors if so, but maybe not even. Unsure what the effect of removing that line would be.

But for all of the resolve* queries I think it would work alright?

[justinweiss]

Yep, it works for the most part. I'm still running into some places where it gets more complicated -- for example, element.createIterable calls resolveRange internally, so it would need to have an editor parameter passed in, too. Value.setSelection would now need an Editor, as would Value.fromJSON. Same with isInRange, the create* methods, getOffsetAtRange, etc.

The way I'm thinking of approaching this is separating out the two things that point.normalize() is trying to do:

1. Resolve the point to the nearest text node
2. Resolve ambiguities at the edges of nodes

(1) would stay within Point, although I would name it something else so I can add a deprecation warning to normalize. getInsertionPoint would call (1), and then would perform (2). This would mean someone could easily override getInsertionPoint to only change how those ambiguities are resolved, without having to reimplement the "find text node" logic.

For things like Value.fromJSON, Operation.applyOperation, etc., I wonder if it would make sense to only use (1), so it wouldn't need to have an editor passed in. They could be immediately normalized by the caller, if the caller has an editor.

I will probably also base this off my WIP fix in #2933, because that fix is _really_ similar to how getInsertionPoint will be implemented.

[ianstormtaylor]

@justinweiss that list of places is awesome, thank you! Helps a lot to think about it.

Something it brings up is... I think we're using resolveRange/Point in some places to pave over "invalid" entries (eg. in element.createIterable, value.setSelection, etc.). Instead, we might want to introduce dumber element.hasPoint and element.hasRange methods, or similar, which can be used to check whether an argument is valid, and throw errors if it's not. Instead of silently fixing them.

In the past this was more problematic since the points/ranges could more easily be invalid. But I think going forward we'll be able to more nicely guarantee that methods are always returning valid values for these, so it will be harder for people to pass in invalid ones accidentally.

Just generally throw a few more errors when things don't line up with expectations.

[justinweiss]

Yep, we absolutely are -- besides filling in key / path if one or the other is missing, that's all it's doing today. Until we remove keys, though, I think it's still useful to do the silent fixes. We expect to do some amount of fixup for selections, because not everything will fill in both key and path. Once we expect selections to stay mostly unchanged, I think it makes more sense to raise.

I can separate out the "fixup" / "fixup and resolve ambiguities" into two different methods -- only the second one would take the editor. Then, when it makes sense to raise errors, it would be an easy replacement.

[ianstormtaylor]

@justinweiss ah, yup. Keys vs. paths was the main reason for them, I totally forgot. Well hopefully we'll land the key-less code soon! Working on it again now.

[ianstormtaylor]

Closing in favor of https://github.com/ianstormtaylor/slate/issues/3151