slate: Slate throws exceptions too liberally in relation to selection failures

So far the only error that have been really problematic for me is this one (Cannot resolve a DOM point from Slate point):

Tracked in own issue here: https://github.com/ianstormtaylor/slate/issues/3575 (my comment here https://github.com/ianstormtaylor/slate/issues/3575#issuecomment-620568898)

It makes collaborative editing near impossible, because as far as I can tell, there is no way we are able to adjust the selection when a new value is coming in through props before slate-react tries to naively restore the current user selection to a potential invalid DOM-range.

Hi @CameronAckermanSEL, I use componentDidCatch in production as it is necessary whether using Slate or not. However, it feels like Slate is the source of too many errors and that’s the key point. There is also the development experience when first getting started to consider, too - here people might not have proper error handling in place and have to constantly restart/refresh/whatever due to errors for things they probably don’t even care about.

Slate seems to think it has to do something based on every interaction with the editor otherwise there is a bug but I don’t think this is right. I guess raising errors was just the easiest approach for the library devs when trying to get 0.50 ready but I don’t think this is the best longterm approach for the library consumers who should ideally decide what is critical and what is not.

Thanks for the response @dylans. I’ll we looking forward for it: in its current implementation we can not go to production with slate due to this issue.

I was developing an editor with Slate and React, but found several situations where I can not avoid the errors thrown by it: for example, when using Katex library to render math formulas.

Of course we can add an error boundary to the slate-react Editable component; but when a Cannot resolve a DOM point from Slate point error is thrown the Editable component will crash anyway, and the whole flow is lost.

Just wonder if there was some work done in the direction mentioned by @mpkelly and @cameracker

@karlludwigweise my fork wasn’t intended to fix #3575 per se but only stop Slate from raising errors for things my app does not care about, like clicking on some structural HTML elements. I still think Slate should be changed to return Node | null rather than just returning Node and raising an Error if it can’t do so. It should be the library consumers job to decide what is critical and what is not IMHO.

Should also point out that my fork is a quick and dirty fix done with a simple code search. It was not intended to be a community fix and I only mentioned it above to make it clear which parts of the Slate codebase I think should be changed.

Addressed in https://github.com/ianstormtaylor/slate/pull/5407

Discussed with @12joan on that one. We’ve ended up on a design that uses a separate “staging” editor that is not rendered:

• apply any incoming operation to the staging editor first.
• if it does not throw, it can be safely applied to the editor.

The goal is to have a system that prevents:

• normalizer errors
• any operation errors
• any transforms/queries (e.g. toDOMPoint) errors
• bonus with plate: log/disable the faulty plugin calling that operation

The alternative would have been to not use a staging editor and “rollback” faulty operations but I’m not comfortable with implementing that design.

As for the error throwing debate, there are many opinions:

1. keep as it is
2. early return null on “error”
3. console.warn the “errors”

For functions having editor as parameter, the common solution would be an editor method onError so we can pick one of the above behavior. For functions not having access to editor, we can’t please everyone. We’ll either need to pass a new option onError on all of these methods, choose one of the 3 above behaviors or reuse the Scrubber design https://github.com/ianstormtaylor/slate/pull/4999.

PR is WIP.

A tale from a developers first experience with slate:

We researched a buch of richtext editors and decided that slate would make the best fit for our prupose (react-admin and a custom markdown editor). We followed the installation guide and the first thing that hit us was the very error message @skogsmaskin erorted as #3575.

I’ve seen a fix in a fork from @mpkelly but that’s no long term fix.

My 2 cents are: When slate throws an error, it should not take the application down with it. And for the specific error #3575 a smart default would fix most cases.

Current categories of selection failures (from this thread):

1. The DOM selection is on a node that is not represented in the slate value

   • IMO, this is a user error and can be alleviated with making the node have a block in the slate value, or by making the node unselectable in the DOM (via contentEditable=false or user-select:none). We can mitigate this with more documentation or a FAQ.

2. An external event removes a node that is selected by slate

   • I’ve seen this mentioned when a user is dealing with OT, but it’s unclear whether this user was attempting to check whether the node about to be removed was selected prior to applying the operation. Slate could arguably handle this case, but the location where selection should move is likely very situational and hard to bake into core as a default.

3. An external event changes a node that is selected by slate

   • Mentioned by a user in slack who was mutating the value directly without the editor controller, and injecting the value back in via setValue. This probably happened because the node that was selected had an address change because immer, but unsure why the path based selection throws.
   • The same user said they solved this problem by checking whether the selected node has been mutated before applying the value. This seems like a good work around. Is this a viable solution?

Suggestions:

1. Provide support for a user defined callback for responding to selection failures, this will allow a user to move the selection.

   • Does the operation resume after the selection is restored? Where can this move cause bugs based on slate moving selection after validation but before making changes to state?

2. Have a default selection that slate moves to if it fails

   • Fallback selection is likely dependent on the users domain rules and may be difficult to enforce a reasonable default. We would likely also need a reasonable override for such a default selection feature, but over all defaults like this may go against some of slate’s “highly customizable, unopinionated” values. But is probably possible somehow

3. Have slate emit a warning and stop whatever it was doing when selection failed, or silently fail

   • this matches how a lot of users have been working around the issue, but I think this would violate a lot of usage expectations if this pattern were to be applied everywhere and it might lead to more complicated bugs that can’t be worked around easily.

@antondc I’ve been using katex for math rendering and haven’t run into the issues you mentioned. If you can post a reproducible example on codepen or something that would be useful.

I do think the error could be more detailed sometimes and provide a path of action for resolving it.

I was developing an editor with Slate and React, but found several situations where I can not avoid the errors thrown by it: for example, when using Katex library to render math formulas.

Of course we can add an error boundary to the slate-react Editable component; but when a Cannot resolve a DOM point from Slate point error is thrown the Editable component will crash anyway, and the whole flow is lost.

Just wonder if there was some work done in the direction mentioned by @mpkelly and @cameracker

I don’t believe much has progressed in this area, though the topic does come up pretty regularly, and along with things like #4769 . It’s an area I too would like to see improve this year.

@skogsmaskin I’ve been working around it by storing the selection, then doing

 const contenteditableElement: any = window
      .getSelection()
      ?.anchorNode?.parentElement?.closest("[contenteditable]").blur();

then changing the value, and afterwards restoring the selection

I am not suggesting this horrible hack as a viable workaround. I have been using this because I am super annoyed at this bug and wanted to move on to something else while still avoiding the crashes.