Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Awareness Update before State Update #374

Closed
superBertBerg opened this issue Nov 17, 2022 · 5 comments
Closed

Awareness Update before State Update #374

superBertBerg opened this issue Nov 17, 2022 · 5 comments

Comments

@superBertBerg
Copy link

Hey @BitPhinix thanks for that decent work!

I'm hunting a bug since hour's. Correct me if i'm wrong there is no guarantee that an awareness update comes before it's state change, especially over the wire? So that

const overlayPosition = getOverlayPosition(editor, range, {
xOffset,
yOffset,
shouldGenerateOverlay,
});
can crash not finding a position?

@superBertBerg
Copy link
Author

Or even if that is guaranteed it's possible that slate is not already finished rendering that state?

@BitPhinix
Copy link
Owner

Hmm, I've been thinking about this one and it shouldn't be happening. Awareness updates definitely shouldn't arrive before their corresponding state update, but even if that happens it wouldn't cause an issue within slate-yjs itself because of the way relative positions work.

So if there is a race-condition it's the cursor change causing a render before slate-react re-renders which possibly could be happening. But to investigate that it would be immensely helpful to have something where I could reproduce the issue. Could you get it to happen on slate-yjs.dev?

@superBertBerg
Copy link
Author

i'll provide a minimal reproduction asap and provide a repo

@FindAPattern
Copy link

FindAPattern commented Nov 21, 2022

I've also been dealing with this exact issue for the past couple days. It only occurs if you start typing at the very end of the document, since the cursor range will be one character past the range in Slate.

Thus far I've figured out that the selection range is being calculated before the DOM updates, but after editor.children has been updated. It compares the offset to text nodes within the DOM, which causes it to throw an error. I suspect it's user error, since I'm using this exact code in the same application in a different component, but that one isn't running into this issue.

Edit:
I think this has to do with React 18's automatic batching, not out-of-order awareness / state updates.

Posted a bit more detail (and a potential workaround) in another issue: #372 (comment)

@superBertBerg
Copy link
Author

I can confirm that, switching back to the old render method solved it for me which should deactivate that kind of batching.

import { render } from 'react-dom';

@FindAPattern Thanks for that, that bug drove me mad especially because I searched in the wrong direction. That answer is relief!

@BitPhinix I did a bunch of tests trying to get a race condition but could not. My bug is pretty sure the bug mentioned in #372 so i'm closing this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants