slate-yjs icon indicating copy to clipboard operation
slate-yjs copied to clipboard

useRemoteCursorOverlayPositions in react 18 does not work

Open ilya2204 opened this issue 3 years ago • 11 comments
trafficstars

Hello! useRemoteCursorOverlayPositions doesn't seem to work in react 18. In the simplest setup with the withCursors plugin, when a remote user types, an error occurs. Maybe the plugin does not support react 18, but I did not find any information about this. (When I switched to version 17 everything worked as it should) image

ilya2204 avatar Nov 11 '22 10:11 ilya2204

In @slate-yjs/[email protected] it seems react 18 worked

ilya2204 avatar Nov 11 '22 11:11 ilya2204

That only occurs for me if I'm editing at the end of a line my guess: RemoteCursorOverlay is updating before the document is in sync

superBertBerg avatar Nov 14 '22 11:11 superBertBerg

Could you share a reproducible example of it not working? Just tested bumping the example and it seems to be working there without any issues

BitPhinix avatar Nov 18 '22 20:11 BitPhinix

@BitPhinix Here not working example in my fork https://github.com/ilya2204/slate-yjs/tree/not-working-example

https://user-images.githubusercontent.com/52297646/202913795-c9c5603e-e369-4025-a881-452a7d67ca9b.mp4

ilya2204 avatar Nov 20 '22 16:11 ilya2204

Ahaha, I forgot to switch over to ReactDOM.createRoot, my bad 😅

BitPhinix avatar Nov 21 '22 11:11 BitPhinix

It seems like this issue may have to do with React 18's automatic batching. I've been looking into this for a couple of days, and noticed that the editor state seems to be behind the DOM state. getDOMPoint looks at the rendered DOM text nodes to compute valid positions. It appears that the automatic batching causes the DOM nodes to get out-of-sync with the editor state.

I've been able to work around this (hopefully temporarily) by disabling automatic batching during changes. This is really hacky, but it seems to solve the issue:

const { onChange } = editor;
editor.onChange = () => {
  flushSync(() => {
    onChange();
  });
};

Hopefully this helps someone!

FindAPattern avatar Nov 22 '22 00:11 FindAPattern

It seems like this issue may have to do with React 18's automatic batching. I've been looking into this for a couple of days, and noticed that the editor state seems to be behind the DOM state. getDOMPoint looks at the rendered DOM text nodes to compute valid positions. It appears that the automatic batching causes the DOM nodes to get out-of-sync with the editor state.

I've been able to work around this (hopefully temporarily) by disabling automatic batching during changes. This is really hacky, but it seems to solve the issue:

const { onChange } = editor;
editor.onChange = () => {
  flushSync(() => {
    onChange();
  });
};

Hopefully this helps someone!

Hi, thank you for this solution. However, I noticed that this causes a warning about flushSync getting called inside a lifecycle function, is this okay or will it cause problems going forward?

rashadaziz avatar Dec 20 '22 11:12 rashadaziz

It seems like this issue may have to do with React 18's automatic batching. I've been looking into this for a couple of days, and noticed that the editor state seems to be behind the DOM state. getDOMPoint looks at the rendered DOM text nodes to compute valid positions. It appears that the automatic batching causes the DOM nodes to get out-of-sync with the editor state. I've been able to work around this (hopefully temporarily) by disabling automatic batching during changes. This is really hacky, but it seems to solve the issue:

const { onChange } = editor;
editor.onChange = () => {
  flushSync(() => {
    onChange();
  });
};

Hopefully this helps someone!

Hi, thank you for this solution. However, I noticed that this causes a warning about flushSync getting called inside a lifecycle function, is this okay or will it cause problems going forward?

I haven't seen any problems on my end, but it's definitely not an ideal solution (the internal components should properly handle batching). I'm using it as a stopgap.

FindAPattern avatar Dec 21 '22 20:12 FindAPattern

Facing this same issue. Unfortunately in my case, I am mounting the Editor only after my HocusPocus Provider has synced so even before flushSync is used with onChange, my editor crashes. This is a work around to issue with Initial State conflict - #111 and #385

Crash Scenario:

User A already has an active selection. User B initializes Editor and as soon as editor mounts, I get the error message from getOverlayPosition.

I am temporarily running React in v17 by using ReactDOM.render as a workaround as suggested in #374

@BitPhinix Have you been able to probe deeper into this?

prabir-vora avatar Mar 13 '23 12:03 prabir-vora

same issue as well on my end using Plate to mount when the HocusPocus Provider synced. Tried as well the hack to add flushSync but it seems its still crashing on my end. Anyone has an idea how for to fix this rather than downgrading to react v17

     <PlateProvider<MyValue>
        editor={editor}
        initialValue={content}
        onChange={(newValue) => {
          flushSync(() => {
            setContent(newValue)
          })
        }}
      >

image

gregfunteratwoconnect avatar Mar 15 '23 00:03 gregfunteratwoconnect

Might help someone else but it seems I am able to fix this on my end without downgrading to v17 by using useLayoutEffect

  useLayoutEffect(() => {
    setShowCursors(true)
    return () => {
      if (!reportEditorRootRef?.current) {
        setShowCursors(false)
      }
    }
  }, [])
 <div
      ref={reportEditorRootRef}
    >
      <PlateProvider<MyValue>
        editor={editor}
        initialValue={content}
        onChange={handleOnChangeContent}
      >
          <div
            ref={reportPlateEditorRef}
            styles={{ backgroundColor: 'white', position: 'relative' }}
          >
            <Plate<MyValue>
              editableProps={{
                style: {
                  padding: '.4in',
                  width: '100%',
                  height: '100%',
                },
              }}
            >
              {showCursors && (
                <ReportRemoteCursors containerRef={reportPlateEditorRef} />
              )}
            </Plate>
          </div>
        </div>

gregfunteratwoconnect avatar Mar 16 '23 07:03 gregfunteratwoconnect