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

Infinite cursor awareness update loop when two prosemirror docs edit same underlying y.XmlFragment #85

Open
milesingrams opened this issue Dec 10, 2021 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@milesingrams
Copy link

milesingrams commented Dec 10, 2021

Describe the bug
I am using TipTap (https://tiptap.dev/) which uses y-prosemirror under the hood to handle collaboration cursors. I have a semi unique situation where I have a page with two editors both which edit the same Y.XmlFragment. When I click on either of the editors to begin editing the cursor starts flickering and runs and infinite cycle of updating the awareness state. I have narrowed down the cycle to this block of code:

const updateCursorInfo = () => {
const ystate = ySyncPluginKey.getState(view.state)
// @note We make implicit checks when checking for the cursor property
const current = awareness.getLocalState() || {}
if (view.hasFocus() && ystate.binding !== null) {
const selection = getSelection(view.state)
/**
* @type {Y.RelativePosition}
*/
const anchor = absolutePositionToRelativePosition(selection.anchor, ystate.type, ystate.binding.mapping)
/**
* @type {Y.RelativePosition}
*/
const head = absolutePositionToRelativePosition(selection.head, ystate.type, ystate.binding.mapping)
if (current.cursor == null || !Y.compareRelativePositions(Y.createRelativePositionFromJSON(current.cursor.anchor), anchor) || !Y.compareRelativePositions(Y.createRelativePositionFromJSON(current.cursor.head), head)) {
awareness.setLocalStateField(cursorStateField, {
anchor, head
})
}
} else if (current.cursor != null && relativePositionToAbsolutePosition(ystate.doc, ystate.type, Y.createRelativePositionFromJSON(current.cursor.anchor), ystate.binding.mapping) !== null) {
// delete cursor information if current cursor information is owned by this editor binding
awareness.setLocalStateField(cursorStateField, null)
}
}

The infinite loop is cycling between:

awareness.setLocalStateField(cursorStateField, { 
  anchor, head 
}) 

and

awareness.setLocalStateField(cursorStateField, null)

My guess is that while the cursor plugin beautifully handles multiple prosemirror documents on a page, it has issues when two prosemirror documents edit the same underlying Y.XmlFragment.

Perhaps more logic around the editor gaining and losing focus could resolve this. i.e. only update awareness when editor is actively focused and set awareness to null when editor loses focus.

Expected behavior
I expect multiple editors to be able to seamlessly edit the same Y.XmlFragment without causing the awareness updates to enter an infinite loop.

Environment Information

  • Browser: Chrome
  • Yjs: "^13.5.22"
  • y-webrtc: "^10.2.2"
  • y-prosemirror: "^1.0.13"

Anyways I'm sure this isn't the most common use case but I would greatly appreciate the help. Thank you for such an awesome tool!

@milesingrams milesingrams added the bug Something isn't working label Dec 10, 2021
@milesingrams
Copy link
Author

I managed to get this to work by explicitly unsetting the cursor state only on focusout. Not sure if this code will inevitably cause issues but seems to be working pretty smoothly so far.

Here is the modified code I am using:

const updateCursorInfo = () => {
	const ystate = ySyncPluginKey.getState(view.state)
	const current = awareness.getLocalState() || {}
	if (view.hasFocus() && ystate.binding) {
		const selection = getSelection(view.state)
		const anchor = absolutePositionToRelativePosition(selection.anchor, ystate.type, ystate.binding.mapping)
		const head = absolutePositionToRelativePosition(selection.head, ystate.type, ystate.binding.mapping)
		if (
			!current[cursorStateField] ||
			!Y.compareRelativePositions(Y.createRelativePositionFromJSON(current[cursorStateField].anchor), anchor) ||
			!Y.compareRelativePositions(Y.createRelativePositionFromJSON(current[cursorStateField].head), head)
		) {
			awareness.setLocalStateField(cursorStateField, {
				anchor,
				head,
			})
		}
	}
}

const unsetCursorInfo = () => {
	const current = awareness.getLocalState() || {}
	if (current[cursorStateField]) {
		awareness.setLocalStateField(cursorStateField, null)
	}
}

awareness.on('change', awarenessListener)
view.dom.addEventListener('focusin', updateCursorInfo)
view.dom.addEventListener('focusout', unsetCursorInfo) // Dont use update function for blur event and instead unset
return {
	update: updateCursorInfo,
	destroy: () => {
		view.dom.removeEventListener('focusin', updateCursorInfo)
		view.dom.removeEventListener('focusout', unsetCursorInfo)
		awareness.off('change', awarenessListener)
		unsetCursorInfo()
	},
}

Let me know if you think this would break in certain situations. Happy to submit a PR if you think it's worth adding!

@bdbch
Copy link

bdbch commented Oct 11, 2023

I think we also ran into this issue while working with React in strict mode. (but it would also happen if you have two Prosemirror plugin instances listening on the same provider causing the editors to throw updates up to react).

@furkan3ayraktar
Copy link

@bdbch We've been using the solution from @milesingrams for a long while without any issues. I've created a PR with that solution here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants