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

fix: use Teleport with disabled=true instead of stub #2632

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

Conversation

victor141516
Copy link

@victor141516 victor141516 commented Mar 13, 2025

fixes #2628

cc @Alevagre7

As part of #1889 the child of Teleport is wrapped with an arrow function. This aligns with how Vue expects it (that's why we don't have the warning anymore). However the stub doesn't use the same implementation for the Teleport as Vue does: Vue doesn't render the children, but waits for the Teleport component to be mounted, then render the children in the teleport target element.

Instead of using a stub, we have a proposal to utilize the disabled prop from the Vue API for the Teleport component (https://vuejs.org/guide/built-ins/teleport#disabling-teleport) instead of creating a custom stub for it.

By using the official API, we can rely on native functionality rather than introducing a stub, which can lead to more friction and increase the likelihood of errors. The disabled prop allows us to effectively "disable" the teleport behavior, which aligns with the intent behind stubbing—removing specific behavior from the unit test.

In essence, whenever someone creates a stub for a component, the goal is typically to abstract away its behavior during testing. Using the disabled prop achieves the same outcome in a cleaner and more reliable manner.

The implementation may not be the best, so @cexbrayat if you have suggestions for it we can change it.

Thanks!

Copy link

netlify bot commented Mar 13, 2025

Deploy Preview for vue-test-utils-docs ready!

Name Link
🔨 Latest commit cda399f
🔍 Latest deploy log https://app.netlify.com/sites/vue-test-utils-docs/deploys/67d30d8b0d7a6c000871749a
😎 Deploy Preview https://deploy-preview-2632--vue-test-utils-docs.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.

Copy link
Member

@cexbrayat cexbrayat left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this @victor141516

The idea is interesting but I think we shouldn't lose the teleport-stub element, to be more aligned with all the other stubs. I guess it would be fine if the result was:

<teleport-stub to="body">
    <!--teleport start-->
    <div id="count">1</div>
    <!--teleport end-->
</teleport-stub>

Even if that might already be breaking for some people, I suppose we could live with that.

Or maybe we have a better alternative: we could provide a TeleportStub that does exactly what you suggest, which would be similar to what we do with RouterLinkStub (see https://github.com/vuejs/test-utils/blob/main/src/components/RouterLinkStub.ts). People would then use it like:

const wrapper = mount(Component, {
  global: {
    stubs: {
      Teleport: TeleportStub,
    },
  },
})

I think this could be a good option. Would that be enough for your use-case?

@victor141516
Copy link
Author

Hey @cexbrayat thanks for the feedback!

Okay let's go with the better alternative :D

So if I understood correctly, you are suggesting that:

  1. If stubs.Teleport === false then nothing is stubbed
  2. If stubs.Teleport === true then it's stubbed using the createStub function
  3. If stubs.Teleport === TeleportStub then it'll use the aforementioned TeleportStub

Is that right?

@cexbrayat
Copy link
Member

Yes, exactly.

VTU will just have to offer the TeleportStub, and developers can opt-in when needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants