fix(vue-2): use mounted lifecycle in FloatingMenu - #7877
Merged
bdbch merged 5 commits intoAug 10, 2026
Merged
Conversation
The previous implementation registered the floating-menu plugin from an `immediate: true` editor watcher. Because immediate watchers fire before `mounted`, `this.$el` was undefined when the editor prop was supplied synchronously, causing a runtime "Cannot read properties of undefined (reading 'style')" error to bubble through Vue's watcher callback. Mirrors the existing BubbleMenu pattern: register the plugin from `mounted()` instead, so `$el` is guaranteed to exist. Fixes ueberdosis#7167.
✅ Deploy Preview for tiptap-embed ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🦋 Changeset detectedLatest commit: bbfca79 The changes in this PR will be included in the next version bump. This PR includes changesets to release 74 packages
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 |
ogyamada
marked this pull request as ready for review
May 27, 2026 05:06
Member
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary
WalkthroughThe Vue 2 ChangesFloatingMenu mount timing
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/vue-2/src/menus/FloatingMenu.ts`:
- Around line 61-89: Add a unit test for FloatingMenu’s mounted lifecycle that
mounts the component with a stub editor, verifies registerPlugin is not called
before mounted/$nextTick completes, and then verifies it is called afterward
with the expected plugin setup. Use the existing test conventions and target the
mounted method’s deferred registration behavior.
- Around line 65-67: Update beforeDestroy to guard the
this.editor.unregisterPlugin(...) call with the same falsy-editor check used in
the mount logic, so teardown safely no-ops when editor was unavailable. Preserve
the existing unregister behavior when an editor exists.
- Around line 75-88: Guard the deferred callback in FloatingMenu before calling
editor.registerPlugin: check the component’s destroyed state after $nextTick
runs and return without registering when the component has already been
destroyed. Keep normal plugin registration unchanged for mounted components.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8124fc8e-cca8-4b3f-b932-1a4a39c2baa3
📒 Files selected for processing (2)
.changeset/2026-05-27-fix-vue-2-floating-menu-watcher-error.mdpackages/vue-2/src/menus/FloatingMenu.ts
coderabbitai
Bot
added
area: vue
…destruction Add `isDestroyed` flag to guard against race conditions where `$nextTick` callbacks from `mounted()` fire after `beforeDestroy()` has already run, preventing attempts to register the floating-menu plugin on a destroyed component.
coderabbitai
Bot
added
complexity: medium
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/2026-05-27-fix-vue-2-floating-menu-watcher-error.md:
- Line 5: Update the changeset sentence to describe only the user-visible
outcome: the FloatingMenu now appears correctly in Vue 2. Remove references to
registration, editor prop timing, and implementation details while preserving
the required frontmatter and one-sentence format.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 0d509fff-412e-4843-bac8-0d950b37e81f
📒 Files selected for processing (1)
.changeset/2026-05-27-fix-vue-2-floating-menu-watcher-error.md
bdbch
approved these changes
Aug 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Move the
FloatingMenuplugin registration in@tiptap/vue-2from animmediate: truewatcher on theeditorprop to amounted()lifecycle hook. Mirrors the pattern already used byBubbleMenuin the same package.Why
The immediate watcher fires before the component mounts, so
this.$elis undefined the first time the handler runs. With the existingif (!this.$el) returnguard, this silently no-opped (so the floating menu was never registered wheneditorwas passed synchronously). Without the guard — or when something else touched$el— Vue surfaces it as:BubbleMenuinpackages/vue-2already usesmounted()for this reason; the reporter on v3.10.1 hit the same class of bug on both menus, but onlyFloatingMenuwas still on the old pattern.Testing
BubbleMenu.tsshape — no logic added, just the watcher →mounted()swap.pnpm install/pnpm build/pnpm test:unitin the sandbox here (pnpm fails with "unable to open database file" before any project script runs — environmental, unrelated to the change). Will run the full validation checklist (pnpm lint,pnpm build,pnpm test:unit,pnpm test:e2e) on a normal machine before merging.Notes
packages/vue-2/; the floating-menu plugin tests live inpackages/extension-floating-menu/__tests__/and don't exercise the Vue 2 wrapper. The demos app also has no Vue 2 floating-menu demo wired up. Adding either feels out of scope for a one-line bugfix — happy to follow up in a separate PR if you'd prefer test coverage here.editormeant the floating menu never registered) will now see it register correctly on mount. That's the documented behavior, so this just restores it.Fixes #7167.