fix(react): resolves React NodeView performance issues #5273

nperez0111 · 2024-06-26T17:35:24Z

Changes Overview

This PR significantly improves the performance of React NodeViews in a couple of ways:

It uses useSyncExternalStore to synchronize changes between React & the editor instance
It dramatically reduces the number of re-renders by re-using instances of React portals that have already been initialized and unaffected by the change made in the editor

We were seeing performance problems with React NodeViews because a change to one of them would cause a re-render to all instances of node views, for an application that heavily relies on node views in React, this is expensive. This should dramatically cut down on the number of instances that have to re-render as well as making each of those re-renders much less costly.

Implementation Approach

Similar to the performance updates for Tiptap 2.5, React has it's own lifecycle for components and it really does not like when you try to force it to be synchronous (since it would prefer to batch renders together). Now we can have the best of both worlds by forcing a synchronous render on initialization, but updates afterward can be done on React's normal lifecycle (i.e. without a flushSync)

Testing Done

Verification Steps

Lots of manual testing with tons of custom react node views. Much faster than before, but hard to write tests for.

Additional Notes

Checklist

I have renamed my PR according to the naming conventions. (e.g. feat: Implement new feature or chore(deps): Update dependencies)
My changes do not break the library.
I have added tests where applicable.
I have followed the project guidelines.
I have fixed any lint issues.

Related Issues

#3976
#3580
#4695

netlify · 2024-06-26T17:35:41Z

✅ Deploy Preview for tiptap-embed ready!

Name	Link
🔨 Latest commit	`b379aa2`
🔍 Latest deploy log	https://app.netlify.com/sites/tiptap-embed/deploys/66b5af56c96cfd00082e10ff
😎 Deploy Preview	https://deploy-preview-5273--tiptap-embed.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

changeset-bot · 2024-07-06T20:33:35Z

🦋 Changeset detected

Latest commit: b379aa2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 54 packages

Name	Type
@tiptap/react	Major
@tiptap/core	Major
@tiptap/extension-blockquote	Major
@tiptap/extension-bold	Major
@tiptap/extension-bubble-menu	Major
@tiptap/extension-bullet-list	Major
@tiptap/extension-character-count	Major
@tiptap/extension-code-block-lowlight	Major
@tiptap/extension-code-block	Major
@tiptap/extension-code	Major
@tiptap/extension-collaboration-cursor	Major
@tiptap/extension-collaboration	Major
@tiptap/extension-color	Major
@tiptap/extension-document	Major
@tiptap/extension-dropcursor	Major
@tiptap/extension-floating-menu	Major
@tiptap/extension-focus	Major
@tiptap/extension-font-family	Major
@tiptap/extension-gapcursor	Major
@tiptap/extension-hard-break	Major
@tiptap/extension-heading	Major
@tiptap/extension-highlight	Major
@tiptap/extension-history	Major
@tiptap/extension-horizontal-rule	Major
@tiptap/extension-image	Major
@tiptap/extension-italic	Major
@tiptap/extension-link	Major
@tiptap/extension-list-item	Major
@tiptap/extension-list-keymap	Major
@tiptap/extension-mention	Major
@tiptap/extension-ordered-list	Major
@tiptap/extension-paragraph	Major
@tiptap/extension-placeholder	Major
@tiptap/extension-strike	Major
@tiptap/extension-subscript	Major
@tiptap/extension-superscript	Major
@tiptap/extension-table-cell	Major
@tiptap/extension-table-header	Major
@tiptap/extension-table-row	Major
@tiptap/extension-table	Major
@tiptap/extension-task-item	Major
@tiptap/extension-task-list	Major
@tiptap/extension-text-align	Major
@tiptap/extension-text-style	Major
@tiptap/extension-text	Major
@tiptap/extension-typography	Major
@tiptap/extension-underline	Major
@tiptap/extension-youtube	Major
@tiptap/html	Major
@tiptap/suggestion	Major
@tiptap/vue-2	Major
@tiptap/vue-3	Major
@tiptap/starter-kit	Major
@tiptap/pm	Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

bdbch

LGTM - lets see how well it performs! :)

bdbch · 2024-07-08T09:52:20Z

demos/src/Examples/CustomParagraph/React/Paragraph.jsx

@@ -6,6 +6,7 @@ import {
 } from '@tiptap/react'

 const ParagraphComponent = ({ node }) => {
+  console.count('render')


Stray console log here

fdintino · 2024-07-13T14:23:20Z

packages/react/src/ReactRenderer.tsx

@@ -121,7 +122,15 @@ export class ReactRenderer<R = unknown, P = unknown> {
      })
    }

-    this.render()
+    if (this.editor.isInitialized) {


Wouldn't this call flushSync every time setRenderer was called after the editor has initialized?

This is in the constructor function, for the initial render.

Before this change, we were calling flushSync on every setRenderer call (with maybeFlushSync).

With this change, it will only flush sync on the first render and then every update after will be handled with a useSyncExternalStore (i.e. letting React "pick up" the changes whenever it feels like it)

bartlomiej-korpus · 2024-07-29T16:50:39Z

Isn't removing flushSync going to break the cursor in transactions that insert a new node resulting in rendering a new React NodeView with NodeViewContent inside and try to set the selection inside of editable content in the new node? From my limited understanding of ProseMirror, in this case prosemirror-view is going to try to set the cursor to proper position synchronously - expecting the DOM to be there, but React will render sometime in the future so it's not yet available at this point in time.

I'm not sure, but looking at the commit history, it might've been added to account for this scenario in the first place.

nperez0111 · 2024-07-29T16:52:43Z

Isn't removing flushSync going to break the cursor in transactions that insert a new node resulting in rendering a new React NodeView with NodeViewContent inside and try to set the selection inside of editable content in the new node? From my limited understanding of ProseMirror, in this case prosemirror-view is going to try to set the cursor to proper position synchronously - expecting the DOM to be there, but React will render sometime in the future so it's not yet available at this point in time.

I'm not sure, but looking at the commit history, it might've been added to account for this scenario in the first place.

That is what this code & comment is regarding: https://github.com/ueberdosis/tiptap/pull/5273/files#r1676838961

bartlomiej-korpus · 2024-07-29T16:53:57Z

Isn't removing flushSync going to break the cursor in transactions that insert a new node resulting in rendering a new React NodeView with NodeViewContent inside and try to set the selection inside of editable content in the new node? From my limited understanding of ProseMirror, in this case prosemirror-view is going to try to set the cursor to proper position synchronously - expecting the DOM to be there, but React will render sometime in the future so it's not yet available at this point in time.
I'm not sure, but looking at the commit history, it might've been added to account for this scenario in the first place.

That is what this code & comment is regarding: https://github.com/ueberdosis/tiptap/pull/5273/files#r1676838961

I might be mistaken, but from what I understand this is regarding editor initialization, but a transaction like that might occur at any point in time after the editor is already initialized, isn't that the case?

nperez0111 · 2024-08-06T09:35:43Z

bartlomiej-korpus

So this PR only skips flushSync if the editor has not yet initialized (because react can't render new nodes when the editor is just being initialized). So, in the case of an already initialized editor, it will use flushSync to force the node view to be created synchronously so that the selection can be moved into the element synchronously. Once the element is created though, this no longer is a problem because the DOM element is present (since it already is initialized and in the dom) so prosemirror can move the selection into that element.

I will definitely double check to verify this though

fdintino · 2024-08-06T15:12:08Z

It is possible to reproduce the cursor issue with our in-house tiptap editor (it's why I never opened a PR for my "fix" from #4492). I can see whether I can reproduce it, and if the problem remains I'll try to share a reduced test case.

nperez0111 · 2024-08-06T15:46:40Z

It is possible to reproduce the cursor issue with our in-house tiptap editor (it's why I never opened a PR for my "fix" from #4492). I can see whether I can reproduce it, and if the problem remains I'll try to share a reduced test case.

Alright, I think that this PR resolves that cursor issue (by flushSyncing when the node view is first constructed) but happy for you to test it!

fdintino · 2024-08-06T16:38:19Z

I can confirm that we don't see the cursor placement issue with this patch.

nperez0111 · 2024-08-06T17:19:01Z

I can confirm that we don't see the cursor placement issue with this patch.

That's great to hear, I just rebased it off the current develop (which has the latest useEditor impl) so this should be more accurate to how it actually will behave

bdbch · 2024-08-07T10:06:45Z

Looks good from my side then and can be merged. Feel free @nperez0111

bartlomiej-korpus · 2024-08-08T15:58:11Z

bartlomiej-korpus

So this PR only skips flushSync if the editor has not yet initialized (because react can't render new nodes when the editor is just being initialized). So, in the case of an already initialized editor, it will use flushSync to force the node view to be created synchronously so that the selection can be moved into the element synchronously. Once the element is created though, this no longer is a problem because the DOM element is present (since it already is initialized and in the dom) so prosemirror can move the selection into that element.

I will definitely double check to verify this though

that's right, now I see! thanks for explaining and sorry for confusion!

leoyli · 2024-09-13T01:08:43Z

Hey @nperez0111, I have experienced some weird issue after this upgrade (after @tiptap/react:2.5.9). Component menu rendered by ReactRenderer did not get the component rendered at all when I destroy one and quickly recreate another instance of ReactRenderer. For example, the Mention extension that uses with Suggestion extension depends on ReactRenderer will not show the menu at all.

https://github.com/ueberdosis/tiptap/blob/main/demos/src/Nodes/Mention/React/suggestion.js#L43-L63

I have a chat app that uses what similars to Mention to suggest user a text snippets. But when the user navigate to the next contact that has the same UI, the suggestion did not triggered. What I found after the inspection, the menu function component did not get called, and the ReactRenderer just attach an empty node there. Even I force the editor to be a new instance, or force React to remount the component by key prop did not resolve the issue. See this video:

Screen.Recording.2024-09-13.at.9.01.27.AM.mov

I'm using React 19 RC, but it was fine previously... Do you have any clue why this is happening? Any help is much appreciated.

nperez0111 · 2024-09-13T08:13:30Z

Hey @nperez0111, I have experienced some weird issue after this upgrade (after @tiptap/react:2.5.9). Component menu rendered by ReactRenderer did not get the component rendered at all when I destroy one and quickly recreate another instance of ReactRenderer. For example, the Mention extension that uses with Suggestion extension depends on ReactRenderer will not show the menu at all.

https://github.com/ueberdosis/tiptap/blob/main/demos/src/Nodes/Mention/React/suggestion.js#L43-L63

I have a chat app that uses what similars to Mention to suggest user a text snippets. But when the user navigate to the next contact that has the same UI, the suggestion did not triggered. What I found after the inspection, the menu function component did not get called, and the ReactRenderer just attach an empty node there. Even I force the editor to be a new instance, or force React to remount the component by key prop did not resolve the issue. See this video:

Screen.Recording.2024-09-13.at.9.01.27.AM.mov

I'm using React 19 RC, but it was fine previously... Do you have any clue why this is happening? Any help is much appreciated.

Their were changes to how suggestions are triggered in this version too so it is more likely to do with that then an issue with React. Please look for an associated GitHub issue. Tracking a bug in a PR will be difficult

nperez0111 force-pushed the fix/react-rerenders branch from 10b4a50 to 575a467 Compare June 26, 2024 18:38

nperez0111 force-pushed the fix/react-rerenders-node-views branch 2 times, most recently from 72fecbe to ad0bdfc Compare June 26, 2024 18:48

nperez0111 force-pushed the fix/react-rerenders branch from 575a467 to a26ed74 Compare June 26, 2024 18:50

nperez0111 force-pushed the fix/react-rerenders-node-views branch from ad0bdfc to 3a066ba Compare June 26, 2024 18:50

nperez0111 force-pushed the fix/react-rerenders branch from a26ed74 to b65ac57 Compare June 26, 2024 20:50

nperez0111 force-pushed the fix/react-rerenders-node-views branch from 3a066ba to 62a37d7 Compare June 26, 2024 20:50

nperez0111 mentioned this pull request Jul 4, 2024

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

Closed

2 tasks

nperez0111 force-pushed the fix/react-rerenders branch from b65ac57 to 607e939 Compare July 6, 2024 20:26

nperez0111 force-pushed the fix/react-rerenders-node-views branch from 62a37d7 to 2b0e7e0 Compare July 6, 2024 20:33

nperez0111 force-pushed the fix/react-rerenders branch from 204e7c5 to f157449 Compare July 7, 2024 13:50

nperez0111 force-pushed the fix/react-rerenders-node-views branch from 2b0e7e0 to e0b53a4 Compare July 7, 2024 13:51

bdbch approved these changes Jul 10, 2024

View reviewed changes

Base automatically changed from fix/react-rerenders to develop July 10, 2024 09:43

nperez0111 force-pushed the fix/react-rerenders-node-views branch 3 times, most recently from 4f15693 to 572842e Compare July 13, 2024 09:52

nperez0111 marked this pull request as ready for review July 13, 2024 10:19

nperez0111 requested a review from svenadlung as a code owner July 13, 2024 10:19

fdintino reviewed Jul 13, 2024

View reviewed changes

nperez0111 force-pushed the fix/react-rerenders-node-views branch 2 times, most recently from a662e8f to adb6d7b Compare July 15, 2024 14:30

nperez0111 mentioned this pull request Jul 17, 2024

[Bug]: React Node Views are not rendered on the initial render, causing layout shift #5166

Closed

1 task

nperez0111 force-pushed the fix/react-rerenders-node-views branch 2 times, most recently from 55e6662 to 32720c8 Compare July 23, 2024 09:18

bdbch approved these changes Jul 23, 2024

View reviewed changes

nperez0111 added 7 commits August 6, 2024 19:16

feat: tried to do it with useSyncExternalStore, is too slow

5be986d

feat: finally something that works

fbeaae1

fix: much better method for knowing if the editor is initialized already

191911f

chore: console

873532a

perf(react): even faster node view rendering

8033f70

chore: use react api

369072e

chore: minor naming

6719f43

nperez0111 force-pushed the fix/react-rerenders-node-views branch from a3bc347 to 6719f43 Compare August 6, 2024 17:16

nperez0111 mentioned this pull request Aug 7, 2024

Error: flushSync was called from inside a lifecycle method in <PureEditorContent> #3580

Closed

2 tasks

chore: add changeset

b379aa2

nperez0111 merged commit e31673d into develop Aug 9, 2024
12 of 13 checks passed

nperez0111 deleted the fix/react-rerenders-node-views branch August 9, 2024 05:56

nperez0111 mentioned this pull request Aug 11, 2024

[Bug]: React renderer is causing update for each nodeview when unique-id extension is used #4695

Closed

2 tasks

deepakjosp mentioned this pull request Sep 2, 2024

Update TipTap to v2.6.0 or latest to include the improvements in React NodeView. bigbinary/neeto-editor#1214

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(react): resolves React NodeView performance issues #5273

fix(react): resolves React NodeView performance issues #5273

nperez0111 commented Jun 26, 2024 •

edited

Loading

netlify bot commented Jun 26, 2024 •

edited

Loading

changeset-bot bot commented Jul 6, 2024 •

edited

Loading

bdbch left a comment

bdbch Jul 8, 2024

fdintino Jul 13, 2024

nperez0111 Jul 13, 2024

bartlomiej-korpus commented Jul 29, 2024 •

edited

Loading

nperez0111 commented Jul 29, 2024

bartlomiej-korpus commented Jul 29, 2024

nperez0111 commented Aug 6, 2024

fdintino commented Aug 6, 2024

nperez0111 commented Aug 6, 2024

fdintino commented Aug 6, 2024

nperez0111 commented Aug 6, 2024

bdbch commented Aug 7, 2024

bartlomiej-korpus commented Aug 8, 2024 •

edited

Loading

leoyli commented Sep 13, 2024 •

edited

Loading

nperez0111 commented Sep 13, 2024

fix(react): resolves React NodeView performance issues #5273

fix(react): resolves React NodeView performance issues #5273

Conversation

nperez0111 commented Jun 26, 2024 • edited Loading

Changes Overview

Implementation Approach

Testing Done

Verification Steps

Additional Notes

Checklist

Related Issues

netlify bot commented Jun 26, 2024 • edited Loading

✅ Deploy Preview for tiptap-embed ready!

changeset-bot bot commented Jul 6, 2024 • edited Loading

🦋 Changeset detected

bdbch left a comment

Choose a reason for hiding this comment

bdbch Jul 8, 2024

Choose a reason for hiding this comment

fdintino Jul 13, 2024

Choose a reason for hiding this comment

nperez0111 Jul 13, 2024

Choose a reason for hiding this comment

bartlomiej-korpus commented Jul 29, 2024 • edited Loading

nperez0111 commented Jul 29, 2024

bartlomiej-korpus commented Jul 29, 2024

nperez0111 commented Aug 6, 2024

fdintino commented Aug 6, 2024

nperez0111 commented Aug 6, 2024

fdintino commented Aug 6, 2024

nperez0111 commented Aug 6, 2024

bdbch commented Aug 7, 2024

bartlomiej-korpus commented Aug 8, 2024 • edited Loading

leoyli commented Sep 13, 2024 • edited Loading

nperez0111 commented Sep 13, 2024

nperez0111 commented Jun 26, 2024 •

edited

Loading

netlify bot commented Jun 26, 2024 •

edited

Loading

changeset-bot bot commented Jul 6, 2024 •

edited

Loading

bartlomiej-korpus commented Jul 29, 2024 •

edited

Loading

bartlomiej-korpus commented Aug 8, 2024 •

edited

Loading

leoyli commented Sep 13, 2024 •

edited

Loading