WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Conversation

@johntdowney
Copy link

@johntdowney johntdowney commented Dec 6, 2025

Changes Overview

Without checking if this EditorContent instance is currently assigned to the contentComponent property, it's possible for the EditorContent instance to destroy an Editor that is in use by another EditorContent instance. Before destroying the editor's view, it should make sure that it is the last EditorContent to use the editor.

Implementation Approach

It's just a single line change in a destruction function.

Testing Done

I've tested the change myself and it seems to be fine. In my case, I have a text editor that I want to quickly move from one component to another. Without this, the first component ends up destroying the text editor after it's already been put to use by the second component. With this, the second component is allowed to take the Editor from the first component without the second component destroying it on them.

Verification Steps

  • Do EditorContent component instances still destroy the views for Editor instances that have not been initialized by other EditorContent component instances?
  • Do EditorContent component instances not destroy the views of Editor instances that have been initialized by other EditorContent component instances?

Checklist

  • I have created a changeset for this PR if necessary.
  • 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.

…es currently in use by other EditorContent components.

Without checking if this EditorContent instance is currently assigned to the `contentComponent` property, it's possible for the EditorContent instance to destroy an Editor that is in use by another EditorContent instance.  Before destroying the editor's view, it should make sure that it is the last EditorContent to use the editor.

I've tested the change myself and it seems to be fine.  In my case, I have a text editor that I want to quickly move from one component to another.  Without this, the first component ends up destroying the text editor after it's already been put to use by the second component.  With this, the second component is allowed to take the Editor from the first component without the second component destroying it on them.
Copilot AI review requested due to automatic review settings December 6, 2025 16:14
@changeset-bot
Copy link

changeset-bot bot commented Dec 6, 2025

⚠️ No Changeset found

Latest commit: 4f951eb

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@netlify
Copy link

netlify bot commented Dec 6, 2025

Deploy Preview for tiptap-embed ready!

Name Link
🔨 Latest commit 4f951eb
🔍 Latest deploy log https://app.netlify.com/projects/tiptap-embed/deploys/6934567cb667820008b7d2a4
😎 Deploy Preview https://deploy-preview-7313--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 project configuration.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a race condition in the Vue 2 EditorContent component where a component instance could incorrectly destroy an Editor's view that has been transferred to and is actively being used by another EditorContent instance. The fix adds a simple ownership check (editor.contentComponent !== this) before performing cleanup operations in the beforeDestroy lifecycle hook.

Key Changes:

  • Added ownership verification to prevent premature editor destruction when an editor is transferred between EditorContent components
  • Ensures that only the current owner of an editor can perform cleanup operations on it

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

const { editor } = this

if (!editor) {
if (!editor || editor.contentComponent !== this) {
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A changeset is required for this PR as it fixes a user-facing bug where EditorContent components could incorrectly destroy editors that are in use by other components. Please run pnpm changeset to create one. The changeset should be a patch version bump and describe the fix: preventing Vue 2 EditorContent from destroying editors that have been transferred to other EditorContent instances.

Copilot generated this review using guidance from repository custom instructions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant