ueberdosis / tiptap

The headless rich text editor framework for web artisans.
https://tiptap.dev
MIT License
25.36k stars 2.17k forks source link

[Bug]: Performance issue with large documents migrating from vue 2 to vue 3 #5031

Open Rirax opened 3 months ago

Rirax commented 3 months ago

Which packages did you experience the bug in?

@tiptap/vue-3

What Tiptap version are you using?

2.2.4

What’s the bug you are facing?

Hello everyone,

In my company, we're fervent Tiptap users, and Tiptap pro subscribers. Today we're experiencing a blocking performance issue with tiptap/vue-3.

Since migrating from tiptap/vue-2 to tiptap/vue-3, we've noticed a slowdown on documents with a large number of nodes that need to be rendered in Vue.

A look at the thread on tiptap-vue3 implementation](https://github.com/ueberdosis/tiptap/issues/1166#issuecomment-777792156) mentions the breaking changes between vue-2 and vue-3, and the use of Teleport to add elements to the DOM, which may explain this drop in performance.

To demonstrate the bug, you need content with at least 1500 nodes.

The loading time increases quadratically with the number of nodes.

The repository below reproduces the bug 👇

What browser are you using?

Chrome

Code example

https://codesandbox.io/p/devbox/tiptap-vue-3-vuenodeviewrenderer-mg4dp8?file=%2Fsrc%2FApp.vue%3A16%2C1

What did you expect to happen?

We are a knowledge management software company, and our applications are particularly dependent on tiptap for the user experience.

We are in the process of migrating our stack from Vue 2 to Vue 3 and as a result are looking to adopt tiptap/vue-3. The current performance issue is blocking our migration to Vue3.

Anything to add? (optional)

Let us know if we can be of any help to reproduce this bug or contribute a fix to it.

Did you update your dependencies?

Are you sponsoring us?

bdbch commented 3 months ago

Thanks for reporting! I tried to get a quick grasp via performance debugging in Chrome Dev Tools but seems like it even crashes my chrome instance on that.

I just saw that the NodeViewRenderer takes a lot of time for each component - I don't have a Vue 2 setup now that I could easily test. Do you still have your code around that you could put into a sandbox real quick so I can do my tests on that to compare where and what increases the rendering time that much?

bdbch commented 3 months ago

Quick test (which I don't think is reliable) shows that each VueRenderer's constructor takes around 9 to 13ms which adds up real quick on 1500 elements.

Rirax commented 3 months ago

Hi @bdbch ,

Thank you for your reply !

For comparison, here is the same content with tiptap vue-2.

I've inspected the vue-3 package and I think the performance problem comes partly from the render method of EditorContent.ts.

`render() { const vueRenderers: any[] = []

if (this.editor) {
  this.editor.vueRenderers.forEach(vueRenderer => {
    const node = h(
      Teleport,
      {
        to: vueRenderer.teleportElement,
        key: vueRenderer.id,
      },
      h(
        vueRenderer.component as DefineComponent,
        {
          ref: vueRenderer.id,
          ...vueRenderer.props,
        },
      ),
    )

    vueRenderers.push(node)
  })
}

return h(
  'div',
  {
    ref: (el: any) => { this.rootEl = el },
  },
  ...vueRenderers,
)

}`

It appears that with each successive update to add a new node, all previously rendered nodes are rendered each time.

Here a PR Draft

I hope this will help you solve this bug.

Thank you very much for your help!

SevrainChea commented 3 months ago

We are also using tiptap (vue2) in my company and we are starting the process of migrating to vue3. Thank you @Rirax for highlighting this performance issue ! I'm posting to follow this thread, but this could potentially be a hard blocker for us because we currently have clients with several thousand lines documents....

@bdbch Any clues regarding the timeline of this fix ? 🙏

Thanks in advance for your work

bdbch commented 3 months ago

No timeline yet – @Rirax send in a PR which I'll check asap when I find time to see if it improves rendering performance and doesn't break anything – after that we'll see to release it in a new minor if everything is working fine.

But can't give a time estimate rn.

Bv-Lucas commented 2 months ago

Hello everyone, I'm facing the same issue at my company when moving from tiptap/vue2 to tiptap/vue3. Thanks for the investigation @Rirax and for the PR. I hope this will get fixed soon so we can complete our migration, is there still no timeline defined @bdbch ? Thanks everyone and have a good weekend

fushugaku commented 2 months ago

Hello, tried to build tiptap with pr from @Rirax and to add it to our project that uses a lot of VueNodeViewRenderer components, unfortunately didn't work for us, the tab just crashes, though it worked almost flawless with vue2 :/ (we use tiptap for big articles, about 2-6mb of json content) The project itself has almost no getters/watchers, even when I removed them all - the browser tab just freezes. Anyway, thanks everyone for their hard work, I hope this issue will be resolved sooner or later, since it's a major blocker for transitioning to vue 3.

timurerrant commented 2 months ago

Hello, tried to build tiptap with pr from @Rirax and to add it to our project that uses a lot of VueNodeViewRenderer components, unfortunately didn't work for us, the tab just crashes, though it worked almost flawless with vue2 :/ (we use tiptap for big articles, about 2-6mb of json content) The project itself has almost no getters/watchers, even when I removed them all - the browser tab just freezes. Anyway, thanks everyone for their hard work, I hope this issue will be resolved sooner or later, since it's a major blocker for transitioning to vue 3.

Yep, same here. Were using it on vue2 and got those issues with vue3. Tried to solve it by myself but no luck for now. Honestly still hope for an update. Thanks for the work again!

AndreewMelnik commented 2 months ago

Hi! Thanks @Rirax for writing about this problem. We also use tiptap in our company, and we started having the same problem since move from vue2 to vue3. Performance on large documents began to suffer greatly, some simply stopped opening because of this. I hope for a fix soon and thanks everyone for your work!

galibov commented 2 months ago

Hello, I also tried integrating Tiptap with the PR from @Rirax into our Vue 3 project and faced similar issues with the browser tab crashing, despite it working well with Vue 2

SevrainChea commented 2 months ago

Hi @bdbch, it's been a month since this thread has been opened and it seems to be a critical issue for several tiptap users for transitioning from vue2 to vue3

Any news on the timeline ? Thanks in advance for your work

Veseron commented 2 months ago

I see many problems similar to mine, but I decided to share anyway. A couple of days ago we completed the migration of the project to Vue 3 and started getting tab crashes when there was a large amount of content

Maybe there is some intermediate solution for us? Tried to make a build from pr above, but it did not fix the issue.

koalest commented 2 months ago

Hi! I've also stumbled upon this issue, unfortunately, vue2 performance was much better, it's a major blocker for us also. Hope there is some kind of solution, thank you for all your hard work!

Rirax commented 1 month ago

Hello everyone,

Thank you all for your feedback on this issue, which helps move things forward.

@bdbch thanks for the review. I took a closer look at the error test for the Drawing example and it turns out that the component needs to rerender at each line to be able to take update props into account.

I therefore revised my proposal, removing the use of Teleport and the need to rerender the app for each node. I took my inspiration from the vue-2 package to enable each component to be rendered as a unit and added to tiptap. https://github.com/ueberdosis/tiptap/pull/5206

According to my tests, I've noticed a clear improvement in performance for content with a lot of nodes to be rendered by Vue.

Capture d’écran 2024-06-03 à 22 03 27

I'm interested in your feedback to see if you're seeing the same thing as me or if there's still room for improvement.

fushugaku commented 1 month ago

@Rirax, thank you, we'll try your solution tomorrow in our project, I've also thought that the problem was with vue-teleport, but unfortunately had no time to investigate, thank you for you hard work.

fushugaku commented 1 month ago

@Rirax, we made a few pre-tests with json content of ~6mb, works perfectly for us, big thanks!

Rirax commented 1 month ago

Hi @fushugaku, Thanks for your feedback. I'm glad to hear it fixes your performance issues too !

fushugaku commented 1 month ago

Hi @Rirax, we've found another issue - when deleting vue nodes via deleteNode editor doesn't delete the node, we are investigating, but no luck yet. // upd found the problem, it was on our side, we were deleting component in onBeforeUnmount, but it's already handled now

nperez0111 commented 4 weeks ago

Hi everyone, I failed to mention that we have released @Rirax's fix to our beta v2.5.0-beta.4

Please try it out & let us know if you run into any issues.

Rirax commented 4 weeks ago

Hi @nperez0111, thank you for the release !