Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft: (vue-3) render node only once #5041

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Rirax
Copy link

@Rirax Rirax commented Apr 8, 2024

Please describe your changes

In this PR we change the way nodes are rendered into Vue components and held in locale state during content loading.

For each node in EditorContent.ts, the current node and all its predecessors are rendered for each new node, which implies a performance issue.

To avoid all nodes being rendered each time, we now keep the rendered nodes as a reference

How did you accomplish your changes

Since we cannot mutate the Editor props in render method of EditorContent, we move the logic to render a node in VueRenderer.ts.
Instead of keeping a Map of VueRenderer instance for each node, we keep the rendered VNodes in Map.

For each new node, we render it and add it to the existing Map.
The render method of EditorContent now retrieves the VNodes and renders them.

How have you tested your changes

[add a detailed description of how you tested your changes here]

How can we verify your changes

[add a detailed description of how we can verify your changes here]

Remarks

[add any additional remarks here]

Checklist

  • The changes are not breaking the editor
  • Added tests where possible
  • Followed the guidelines
  • Fixed linting issues

Related issues

Link to the related issues here

Copy link

netlify bot commented Apr 8, 2024

Deploy Preview for tiptap-embed ready!

Name Link
🔨 Latest commit 5cdab90
🔍 Latest deploy log https://app.netlify.com/sites/tiptap-embed/deploys/66140c9f2d1f8200085b1d09
😎 Deploy Preview https://deploy-preview-5041--tiptap-embed.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@bdbch
Copy link
Contributor

bdbch commented May 10, 2024

@Rirax LGTM but it seems there is a test failing in a Vue demo (regarding a Vue demo for the Drawing example).
Could you verify and maybe take a look at that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ready to Merge
Development

Successfully merging this pull request may close these issues.

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