fix(vitepress-twoslash): fix popper positions being recomputed too early within vitepress code groups

[Dschungelabenteuer]

Note

This PR only relates to the vitepress-twoslash package

The issue I'm trying to address

I'm using Twoslash within VitePress code-groups. A few months ago, @antfu added this to improve tabs experience. However, despite that change, I often run into the same issue where switching to a "code-group" that either includes a completion or a query popper would actually not show that popper.

Here's a very simple repro, on the /issues page.

Here's an overview of the issue

When switching on the second group, completion popup doesn't show up until the whole block gets re-rendered.
In this video I click on a tab in an unrelated code-group to trigger a re-render and actually show my second group's completion popper.

Here's an ever more annoying consequence of this issue

You can see how playing around with the active tab messes up what actual popper should be displayed. I'd rather have the popper not showing up (like in the previous example) than displaying wrong data!

The cause

It turns out it is just a timing-related issue.

Keep in mind that:

• VitePress conditionally displays a code-group based on an active class.
• VitePress handles such active classes with a "click" listener.
• vitepress-twoslash provides its own tab "click" listener to re-compute poppers positions.

This happens in my scenario:

• vitepress-twoslash's click handler is fired first and calls floating-vue's recomputeAllPopppers.
• At this point, VitePress still did not add the active class to the popper reference.
• Popper position is therefore recomputed against a reference which is still not displayed, which means zeroed positions.
• This causes the popper to behave exactly as if tab was not active.

See original suggested fix

The suggested fix

On vitepress-twoslash side, when handling a "tab click", the recomputeAllPoppers is called whenever target element's classList contains either vp-group-code or tabs class. While vp-group-code obviously targets VitePress code groups, I wasn't sure about tabs so I preferred to leave that logic untouched,

Essentially, my suggested fix consists in waiting, if needed, for the "active" class to be added to the target code-group (and therefore for the relevant code-block to be displayed) before recomputing its poppers positions.

This is done by spawning a simple MutationObserver that waits for this "active" class to be included then self-destructs. That's the best approach I came up with because vitepress-twoslash and VitePress do not have a direct communication channel and I wouldn't expect this to have much performance impacts (unless maybe end-users start unreasonably spam-switching between code groups?).

About escape hatches…

1. All conditions that do not match VitePress's own conditions of handling a code-group tab change should apply Anthony's previous logic, which is to directly call recomputeAllPoppers if the click matches .tabs (or VitePress code blocks).
2. Otherwise it re-checks [at each critical step] whether the MutationObserver is actually still needed (classList may have been updated since and now contain "active" for several reasons).
3. There could be edge-cases where "active" never actually makes it to the observed element's classList This supposes a "downstream" issue and figured out it wasn't worth implementing a timeout mechanism on waitForActiveClass.

[netlify]

✅ Deploy Preview for shiki-next ready!

Name	Link
🔨 Latest commit	7735945
🔍 Latest deploy log	https://app.netlify.com/projects/shiki-next/deploys/693be2db3642a20008698813
😎 Deploy Preview	https://deploy-preview-1116--shiki-next.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 project configuration.

[netlify]

✅ Deploy Preview for shiki-matsu ready!

Name	Link
🔨 Latest commit	7735945
🔍 Latest deploy log	https://app.netlify.com/projects/shiki-matsu/deploys/693be2db6b1c590009d4adad
😎 Deploy Preview	https://deploy-preview-1116--shiki-matsu.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 project configuration.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.21%. Comparing base (c78b1d7) to head (7735945).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1116   +/-   ##
=======================================
  Coverage   95.21%   95.21%           
=======================================
  Files          92       92           
  Lines        7936     7936           
  Branches     1695     1694    -1     
=======================================
  Hits         7556     7556           
  Misses        374      374           
  Partials        6        6           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Dschungelabenteuer]

My current solution feels kind of hacky and should probably be fixed upstream. We should ideally be able to rely on VitePress notifying "hey, the user switched to that tab of the code group!", but that's not something VitePress does at the moment. I've created this issue on their side to get some feedback

Also, I've fixed the link to repro which was outdated

EDIT: VitePress should expose a vitepress:codeGroupTabActivate we can leverage as of the yet-to-be-released 2.0.0-alpha.14

[Dschungelabenteuer]

As stated in my previous comment, I've created an upstream issue on VitePress side that was addressed. As of 2.0.0-alpha.14, they emit a vitepress:codeGroupTabActivate CustomEvent that we can now use to trigger floating-vue's recomputeAllPoppers in a safer and consistent way.

It would be even better to be able to only recompute poppers within the relevant code group. VitePress passes it along with the CustomEvent, but floating-vue doesn't expose nor support such method. I'm still not sure on how to suggest such change on floating-vue's side.

[Dschungelabenteuer]

vitepress:codeGroupTabActivate obviously is a CustomEvent. This assertion is only here to satisfy TypeScript through control flow, but in the end it's a pretty pointless runtime condition. Should I prefer a type-only approach?

[antfu]

I think a type only approach would do

[antfu]

I wonder if we should keep both approaches?

[Dschungelabenteuer]

I wonder if we should keep both approaches?

For backward compatibility reasons? Or is there any other reason I'm not aware of?

In VitePress >=2.0.0-alpha.14, that would mean calling recomputeAllPopppers twice every time we switch between tabs, isn't that a bit drastic? (given that it even recomputes poppers that are unrelated to the code group the user interacts with)

I'd love to get your insight on my own review as well if you feel like it :)

[antfu]

Yes, for compatibility reasons. I just think 'vitepress:codeGroupTabActivate' seems a bit too specific to the version (also giving it's still in alpha). If we worry about the duplication, we could do a slightly delayed debounce.

[antfu]

Oh, thank you, but is this too much to introduce VueUse for only the throttle function. We can implement a simple one in-house instead.

[Dschungelabenteuer]

Hmm just realized throttling those recomputeAllPoppers calls actually reintroduces (inconsistently) my original issue since your click listener might fire right before vitepress' new event (i.e. during the specified delay).

I wonder if trying to obsessively avoid these duplicate calls is in fact relevant and even realistic? The only approach I see is to only start throttling whenever vitepress:codeGroupTabActivate is received since it guarantees it's supported by the used VitePress version and that we won't need the older click listener's callback, but is it worth it?

I'll simply remove the throttle for now, I can go for the above approach if you feel like it's worth it or keep it that way if you feel like it's not!