-
Notifications
You must be signed in to change notification settings - Fork 266
Stubbed components slots aren't been rendered when 2 deep. #773
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
Comments
I did some digging around the code and think I may have found the issue. When creating a stub we are creating a render function and passing the ctx.$slots in directly. The issue here is I think that any child slots inside that stub are not getting passed through the vnodeTransform and therefore not being rendered correctly. I changed the render function to the follow: const render = (ctx: ComponentPublicInstance) => {
const slots = ctx.$slots
? Object.keys(ctx.$slots)
.map((k) => ctx.$slots[k])
.map((f) => {
if (f) return f()
})
: []
return h(tag, ctx.$props, renderStubDefaultSlot ? slots : undefined)
} and it started rendering out stubs inside of the first stub. I am sure that code could be far neater, I apparently suck at writing typescript, but I think that we need to call the slot functions before passing them into the |
I have taken a stab a reworking renderStubDefaultSlot, the issue being that it doesnt really provide full backwards compatibility with v1.
config.renderStubSlot = {
'component-with-slots-stub': {
scoped: { boolean: true, string: 'test' }
}
} The above config will pass in Thoughts? |
I didn't get a chance to look at the reproduction or look into this deeply yet, but
Is going to be a blocker. We have Issues like this come up a lot - ultimately, the purpose of |
@lmiller1990 with regards to the backwards compatibility. v1 would render all slots in a stub not just the default one. Now in v2 only the default gets rendered with the |
Please give me a day or two to go over this and really understand the implications. Like you said, this is quite hard to "explain", but the code sample you've provided will help a lot. Thanks! I try to take things to do with stubs pretty cautiously, since they've caused a few unexpected breaking changes in the past. |
@rikbrowning thank you for working on this! My 2¢ here since I'm working for a month on compatibility layer between v1 and v2: I really love current behaviour and I think it is worth to keep it. The reasoning is simple - IMO the library should not provide an easy way to shoot yourself in a leg by testing something which never appears in real world output and as you've mentioned in your commit - your approach allows user to build assertions vs non-existent slots when using Users who are migrating from v1 to v2 could provide an intermediate layer (by wrapping |
@xanf sorry for my delayed response. I was thinking about this more last night and do we not already allow users to put props on stub components which the stub component might not accept? In which case how is that different from rendering slots that might not get used. In fact I would make the case that rendering all the slots allows the user to ensure that what they are actually passing through their slots matches their expectations. Now whether the stubbed component uses those slots is really up to that component. In which case using mount would be the functionality that you want not shallowMount. If we really wanted to introduce safety we should allow configuration of slots that a stub has and render those out, thus matching reality a lot closer than only allowing rendering of the default slot. One point I would like to call out is the documentation on upgrading from v1 https://next.vue-test-utils.vuejs.org/migration/#shallowmount-and-renderstubdefaultslot This documentation doesn't explicitly highlight that any slot other than the default will not work even when enabling the new config property. Maybe perhaps suggest the recommendation of using mount to render deeper slots? |
^ Updating docs is always a good idea, if you want to make a PR improving them, that'd be great. |
I want to use this but can't find it anywhere in the docs. Does anyone have an example for how to use it? I tried setting config.global.renderStubDefaultSlot = true at the top of my test file and it didn't have any effect, the slot isn't rendered. I also tried it in beforeEach(). EDIT: Ugh, now I see my problem, the component doesn't use a default slot, it uses named slots. How can we achieve the same sort of automocking behaviour with named slots? |
I am not sure that is supported / possible rn @rocifier, can you rework your component (or avoid the need to stub/mock)? |
FWIW I've found a lot of success by avoiding shallowMount/stubs features all together and instead relying on vitest/jest mocking capabilities to mock component the same way I'd mock anything else.
|
This what we are experiencing too. Just to reconfirm:
The docs don't indicate this is the case and it's not listed as a breaking change. Is the suggestion really to just use |
Here's my solution for named slots content: I took my inspiration from the
I used vitest.setup.ts
stub.ts (I had to copy some functions from
We can then use
|
I put together a test case to show what I mean. It is very difficult to describe it in words. Essentially the produced snapshot of the test is incorrect. It should include all stubbed components and their slot contents as well.
rikbrowning@53a0a13
The text was updated successfully, but these errors were encountered: