Text.equals on ...rest of node and editor's marks

[idosilverwater]

Hi,

There is a part of the code, that I'm not sure I understand the reasoning behind. I'd be grateful if someone can tell me where what I'm missing.

In editable.tsx:770 and editable.tsx:796 there Text.equals is used to compare the rest from {text, ...rest} of a Text node and editor.marks.
from my understanding marks is not a text object and could optionally be null.

in both these Text.equals invocations I've referenced above, Text.equals is called with loose=true which means both the rest and marks will be destructed (see here), since marks can be null in that case an exception will be throw (destructing a "text" out of null is not allowed)

This is what happens to me, the marks is null and the destructing fails

index.js:4305 Uncaught TypeError: Cannot read property 'text' of null
    at omitText (index.js:4305)
    at Object.equals (index.js:4311)
    at eval (index.js:4116)

My question: How does this compare makes sense? marks is not text (as I understand it)

[BitPhinix]

Valid point indeed. Just a shortcut I took and then overlooked during cleanup. Will put in a PR tomorrow 🙏

[idosilverwater]

Do you know of which of the existing versions this bug does not exist?
thanks for your help!

[dylans]

Likely 0.82.0+ is where it's an issue... as soon as there's a PR I'll review/land/release.

[BitPhinix]

Ok, looking back into this I know why I kept the Text.equals. It's totally fine to use in this case since we check for loose equality which omits the text anyway. If we don't use Text.equals exported by slate we would either:

1. export a new function/util/interface to compare marks
2. copy and paste (and keep in sync) isDeepEqual into slate-react

The only thing here that is causing an issue is a missing check to see if marks are set. Here is the cleanup pr: #5127. I'm curious why we never encountered this error at slite, we've been using this in prod for a while now 🤔

[idosilverwater]

I think that I might know the reason. Im using slate through clojurescript. Maybe this has to do with how clojurescript compiles js.

Just saying Im pretty sure I copied the plaintext example one to-one.

I'm sorry if it seems like I failed to mention something important for the understanding of the problem.. But it looked like an unrelated logical problem.

[ALai57]

It took me a while, but I'm so happy I just found this thread.

I have the same use case - I'm using Slate through CLJS and was encountering the error described above (Console log pollution by logging "Uncaught Type Error: Cannot read property 'text' of null") on every cursor change, or character change when using Slate. Upgrading to the new release (Slate > 0.83.0) fixed the issue for me.

Just out of curiosity, did you ever find a root cause to the issues, or is the compilation our best guess right now?