Downgraded from React 18 back to 17 due to stability issues and problems with .updateScene calls.

[zsviczian]

Coinciding with the upgrade of Obsidian-Excalidraw to React 18 and migrating to ReactDOM.createRoot for creating the Excalidraw component I've been experiencing various random issues. Two in particular

1. Excalidraw and/or Obsidian screen going blank on certain operations. One example is this bug: #738.
2. Executing multiple updateScene() calls within a function results in updateScene calls to be dropped. I can trace back the point in time when it stopped working to the commit when I moved to .createRoot. #747

I have found that if I space .updateScene() calls by placing them in separate window.requestAnimationFrame() calls the updateScene() issue has gone away, however, the stability issue remained... or rather it could be that the stability issue is caused by requestAnimationFrame(), however without that I wasn't able to overcome the issue mentioned in bullet 2.

For now, it seems the downgrade has resolved immediate problems, however, I'd like to understand if I am misusing the package in some way that is leading to the above issues, or if something needs to be changed in the Excalidraw package.

[zsviczian]

on face value #5602 is very similar to #738.

[dwelle]

• Executing multiple updateScene() calls within a function results in updateScene calls to be dropped. I can trace back the point in time when it stopped working to the commit when I moved to .createRoot. #747

This reproduces only on React 18? Which update is being dropped, the first one or the second one?

[zsviczian]

Only (and consistently) with React 18. I will need to revert to an earlier version to do some more testing, but as far as I remember it was the second one that was missed.

[zsviczian]

I was only able to reproduce the error from code calling updateScene() from external to the Excalidraw package (i.e. from the Obsidian plugin). When debugging the code with the developer console I could not reproduce the issue... I guess somehow frames are issued while I press F8 to step through the process (?). Anyway, requestAnimationFrame seems to have solved the problem, but I am afraid that it had side effects somehow leading to a white screen of death in certain scenarios.

[zsviczian]

#5606 seems to have solved point 1. Point 2. is still relevant.

[zsviczian]

@dwelle, I just bumped into #5508 Could this issue have the same root cause? What would happen if we move updateScene() to use flushSync?

[dwelle]

What would happen if we move updateScene() to use flushSync?

can you try that in your fork first, if it fixes it, since you seem to be the only one to reproduce?

[zsviczian]

I moved the Obsidian plugin to REACT 18 again. So far I am unable to reproduce the problem from August. Will give it a few more days of testing before declaring victory (and closing this issue)

[zsviczian]

@dwelle, unfortunately, the issue has surfaced again. I added flushSync to updateScene like this, but that did not resolve the problem. Is this what you had in mind for flushSync?

  public updateScene = withBatchedUpdates(
    <K extends keyof AppState>(sceneData: {
      elements?: SceneData["elements"];
      appState?: Pick<AppState, K> | null;
      collaborators?: SceneData["collaborators"];
      commitToHistory?: SceneData["commitToHistory"];
    }) => {
      if (sceneData.commitToHistory) {
        this.history.resumeRecording();
      }

      flushSync(() => {
        if (sceneData.appState) {
          this.setState(sceneData.appState);
        }
      });

      if (sceneData.elements) {
        this.scene.replaceAllElements(sceneData.elements);
      }

      if (sceneData.collaborators) {
        this.setState({ collaborators: sceneData.collaborators });
      }
    },
  );

[dwelle]

I added flushSync to updateScene like this, but that did not resolve the problem. Is this what you had in mind for flushSync?

I'm not sure where the root cause is here, but since I couldn't repro I wanted you to try what you suggested, first.

Honestly, we're having issues with async state updates everywhere, since updating to React 18. Either we start wrapping setState() calls into flushSync everywhere, or we rearchitect — long term we want to remove react from the editor, so we may as well start that process now.

/cc @ad1992

[zsviczian]

Is there a way to overload React.Component setState with flushSync included?

[dwelle]

Is there a way to overload React.Component setState with flushSync included?

Possibly, for testing. In prod we'd want to create a wrapper method though.

The first step to decoupling react from the editor would be to consolidate updates to a single/couple of dispatches, anyway. Similar to what we do already with syncActionResult.

[zsviczian]

It turns out that the issue I encountered in the afternoon was due to an error in the plugin code, not Excalidraw. I fixed the error, since then I've not experienced issues with React 18. I'll continue to use this version of the plugin for few days. If still nothing, then I will close this issue.

[zsviczian]

@dwelle, just to let you know, in the end, I did modify updateScene as proposed above. The issue happens when I trigger multiple updateScene calls from script using the Obsidian Excalidraw Script API that I developed. Otherwise, all the other issues that I experienced with React18 in August were my coding issues in the plugin, involving the topRightUI JSX component... so nothing to do with the core Excalidraw package.

[dwelle]

@zsviczian so do I understand it that this issue is related to how Obsidian makes specific updates, and thus no action is necessary on our part? If so, let's close.

[ad1992]

closing this as from the discussion it concludes that issue was related to Obsidian script and not Excalidraw specific. Feel free to reopen if it persists.