tiptap icon indicating copy to clipboard operation
tiptap copied to clipboard

[Bug]: Race condition: the editor crashes if there is a frequent change in its dependency array

Open yyjlincoln opened this issue 7 months ago • 2 comments

Affected Packages

react, core

Version(s)

2.4.0

Bug Description

When an editor is recreated due to dependency changes in its dependency, for example:

const Comp = ({editorOptions}: {editorOptions: any}) => {
  const editor = useEditor(
    {
      extensions: createExtensions(editorOptions),
    },
    [editorOptions],
  );

  return (
    <EditorContent editor={editor} />
  )
}

And if the same editorOptions is used somewhere in its parents, the editor will crash due to a potential race condition with:

TypeError: Cannot read properties of null (reading 'updateOuterDeco')
    at EditorView.updateStateInner (index.js:5306:34)
    at EditorView.update (index.js:5235:14)
    at EditorView.setProps (index.js:5249:14)
    at Editor.createNodeViews (Editor.ts:313:15)
    at PureEditorContent.init (EditorContent.tsx:83:14)
    at PureEditorContent.componentDidMount (EditorContent.tsx:58:10)
    at commitClassLayoutLifecycles (react-dom-client.development.js:22140:18)
    at commitLayoutEffectOnFiber (react-dom-client.development.js:22312:11)
    at recursivelyTraverseLayoutEffects (react-dom-client.development.js:23856:7)
    at commitLayoutEffectOnFiber (react-dom-client.development.js:22298:9)
    at recursivelyTraverseLayoutEffects (react-dom-client.development.js:23856:7)
... (omitted)

I have diagnosed it and here are my assumptions:

useEditor creates a new instance of the editor in the initial render. When it is passed to EditorContent (which wraps around PureEditorContent), on the first mount, it calls:

// EditorContent.tsx
componentDidMount() {
   this.init()
}

init() {
  // ...
  editor.createNodeViews()
  // ...
}

In createNodeViews, we have:

// Editor.ts
this.view.setProps({
      nodeViews: this.extensionManager.nodeViews,
})

Following the call stack, this eventually lands on (prosemirror):

// EditorView from ProseMirror
this.docView.updateOuterDeco(outerDeco);

However, if the dependency prop has changed, then:

// useEffect cleanup within useEditor.ts (dependency array has extensionOptions)
return () => {
      isMounted = false
      editor.destroy()
}

// Editor.ts
public destroy(): void {
    this.emit('destroy')

    if (this.view) {
      this.view.destroy()
    }

    this.removeAllListeners()
  }

// EditorView from ProseMirror
destroy() {
  // ...
  this.docView.destroy();
  this.docView = null;
}

The editor instance would have been destroyed when the depencency array changes. This causes the null error (useEffect cleanup is triggered (triggered by deps change of useEditor) before editor initialisation (triggered by mounting).

The situation is particularly bad if multiple (>5-10) editors are running on the same page, which significantly increased the chances of it crashing.

Browser Used

Chrome

Code Example URL

No response

Expected Behavior

Editor to be recreated without crashing.

Additional Context (Optional)

Could be linked to #1451 and #2380.

Dependency Updates

  • [X] Yes, I've updated all my dependencies.

yyjlincoln avatar Jul 12 '24 04:07 yyjlincoln