tiptap: [Bug]: Usage of ReactNodeViewRender drastically reduces performance for large Text

We have the performance issues in our pipeline and we’ll probably start working on all performance related issues with React after the holidays - just as a heads up.

Could we please get this re-opened since the issue has been noted to not have really been addressed?

We have the performance issues in our pipeline and we’ll probably start working on all performance related issues with React after the holidays - just as a heads up.

edit: There is a flaw with this patch that is addressed in a different post below.

I experienced a similar problem: text input in documents with many react node views was so slow as to make it unusable. There are probably a few performance optimizations to be had, but I found my issues to be related to #3580, which called out this method:

https://github.com/ueberdosis/tiptap/blob/42039c05f0894a2730a7b8f1b943ddb22d52a824/packages/react/src/EditorContent.tsx#L72-L82

~I fixed it by patching the function to just execute the function passed to it:~

// EDIT: see updated patch below
// import { PureEditorContent } from "@tiptap/react";
//
// PureEditorContent.prototype.maybeFlushSync = (fn: () => void) => {
//     fn();
//   };

~I’ll keep tabs on #3580 to see if there is a functional reason that it needs to call flushSync there, but I haven’t noticed any issues so far.~

Please can someone fix this at TipTap, because this makes TipTap with react components unusable… This should be fixed.

That can be achieved by wrapping the component in React.memo, since for this particular component it’s not using any of the props. For me, this gives comparable performance:

const ParagraphComponentPatched = React.memo(
  () => (
    <NodeViewWrapperPatched className="wrapper">
      <RenderCount />
      <NodeViewContentPatched as="p" />
    </NodeViewWrapperPatched>
  ),
  (prevProps, nextProps) => true
);

The fix incorporated one of the fixes, but it didn’t include my flushSync fix. I’ll open that in a pull request.

yes! Should be fixed by #4661. Let us know if the issue persists.

Sure. Here’s a codesandbox demonstrating the problem, with a toggle between tiptap-react, a vanilla js nodeview, and a version with my patch and @Zloka’s above. The number in the left margin is a count of the number of renders.

We had to revert the patch as well because of some weird cursor placement bugs that were introduced after applying it.

I am deploying to firebase and I cannot monkey patch because firebase rebuilds node_modules directory. Any way around this?

I confirm this is an issue for my team too!

Removing the flushSync(fn) as the comments above suggest improves performance massively.

With unpatched TipTap I see my custom nodes re-render on every keystroke and it gets very problematic when the document has tens of them.

With the patch, I see only the custom node I am editing re-render.

EDIT: Digging around with the React Profilers, I see many more React render commits (hundreds vs a handful) with flushSync enabled.

EDIT 2: digging through the blames in ReactNodeViewRenderer it seems as if the flushSync behavior was introduced to fix this bug and judging from the commit message it was intended to render node views immediately when created, but what seems to be happening here is that they are rendered on every keystroke, which was not the intended behavior.

I noticed the same. In my case, it seems to be related to when the selection changes. ReactNodeViewRenderer has the following code:

handleSelectionUpdate() {
    const { from, to } = this.editor.state.selection

    if (from <= this.getPos() && to >= this.getPos() + this.node.nodeSize) {
      this.selectNode()
    } else {
      this.deselectNode()
    }
}

The problem is that this.deselectNode() is called for all nodes, regardless of if they were selected or not, which causes a subsequent call to this.updateProps({ selected: false }), and that seems to cause a re-render of all nodes.

A better solution, assuming it is easy to track the currently selected node, would be to only deselect relevant nodes, i.e. something like:

handleSelectionUpdate() {
    const { from, to } = this.editor.state.selection

    if (from <= this.getPos() && to >= this.getPos() + this.node.nodeSize) {
      this.selectNode()
    } else if (this.node.attrs.selected) {
      this.deselectNode();
    }
}

From what I’ve tested, interestingly enough in my case, I could just remove the this.deselectNode(); call entirely to both see a drastic improvement in performance, and the behavior remained as one would expect 🤔