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

v-slot not rendering along side default slot. #1868

Open
rikbrowning opened this issue Jun 30, 2021 · 24 comments
Open

v-slot not rendering along side default slot. #1868

rikbrowning opened this issue Jun 30, 2021 · 24 comments

Comments

@rikbrowning
Copy link

Subject of the issue

I recently switched a project from the old slot syntax to the new v-slot syntax and my snapshot started failing. Some of the slots are not getting rendered at all.

Steps to reproduce

I have a simplified example that showcases the issue.

My component is as follows:

<template>
  <FakeComponent>
    <template v-slot:test>
      <TestComponent :msg="msg">
        <div>
          default slot of test component
        </div>
      </TestComponent>
    </template>
    <div>
      default slot of fake component
    </div>
  </FakeComponent>
</template>

<script>
export default {
  name: "HelloWorld",
  props: {
    msg: String,
  },
};
</script>

It is populating the test and default slot FakeComponent. According to the vue2 docs you can but do not have to specify the default slot in a template.
My test is as follows:

import { shallowMount } from "@vue/test-utils";
import HelloWorld from "@/components/HelloWorld.vue";

describe("HelloWorld.vue", () => {
  it("renders props.msg when passed", () => {
    const msg = "new message";
    const wrapper = shallowMount(HelloWorld, {
      propsData: { msg },
    });
    expect(wrapper).toMatchSnapshot();
  });
});

Expected behaviour

I would expect the test slot to be rendered in the snap shot as it is with the old syntax.

exports[`HelloWorld.vue renders props.msg when passed 1`] = `
<fakecomponent>
  <testcomponent slot="test" msg="new message">
    <div>
      default slot of test component
    </div>
  </testcomponent>
  <div>
    default slot of fake component
  </div>
</fakecomponent>
`;

NOTE: to get the result above I simply changed the component to this:

<template>
  <FakeComponent>
    <TestComponent :msg="msg" slot="test">
      <div>
        default slot of test component
      </div>
    </TestComponent>
    <div>
      default slot of fake component
    </div>
  </FakeComponent>
</template>

Actual behaviour

exports[`HelloWorld.vue renders props.msg when passed 1`] = `
<fakecomponent>
  <div>
    default slot of fake component
  </div>
</fakecomponent>
`;
@standbyoneself
Copy link
Contributor

standbyoneself commented Jul 6, 2021

It's little hard to understand. I really couldn't see <slot name="test" />. Please reproduce all the used components: HelloWorld, FakeComponent, TestComponent or it would be better to create a repository

rikbrowning pushed a commit to rikbrowning/test-utils-slot-bug that referenced this issue Jul 6, 2021
@rikbrowning
Copy link
Author

https://github.com/rikbrowning/test-utils-slot-bug

In this repository both snapshots should be the same. The only thing that is different is the slot syntax which is causing a break in snapshots.

@standbyoneself
Copy link
Contributor

standbyoneself commented Jul 6, 2021

@rikbrowning,

Check out my PR

What actually happened?

<FakeComponent /> has a default slot and in the <Working /> are passed this content into the slot:

    <TestComponent :msg="msg" slot="test">
      <div>
        default slot of test component
      </div>
    </TestComponent>
    <div>
      default slot of fake component
    </div>

In the <Broken /> you are close tag too early:

    <template v-slot:test>
      <TestComponent :msg="msg">
        <div>
          default slot of test component
        </div>
      </TestComponent>
    </template>
    <div>
      default slot of fake component
    </div>

Move it under the last <div> tag and snapshots will match

Also I'll recommend you to read the docs about slots again because the default content of the slot sets up directly inside the slot.

@rikbrowning
Copy link
Author

Hey I read the docs and found this example (taken from https://vuejs.org/v2/guide/components-slots.html)

<base-layout>
  <template v-slot:header>
    <h1>Here might be a page title</h1>
  </template>

  <p>A paragraph for the main content.</p>
  <p>And another one.</p>

  <template v-slot:footer>
    <p>Here's some contact info</p>
  </template>
</base-layout>

The code changes you have made would put my default content inside the test slot, which is not what I want. The issue being that semantically both Working and Broken are the same but yet result in different snapshots.

@standbyoneself
Copy link
Contributor

Broken.vue

<template>
  <FakeComponent>
    <template v-slot:test>
      <TestComponent :msg="msg">
        <div>
          default slot of test component
        </div>
      </TestComponent>
    </template>
    <template v-slot:default>
      <div>
        default slot of fake component
      </div>
    </template>
  </FakeComponent>
</template>

What about to manually set the template that falls down into the default slot of the <FakeComponent>?

@rikbrowning
Copy link
Author

I did some more investigating. The change you suggested did work for Broken.vue. However when I add a root level div the slots stop rendering again. I have updated the repo with the new Broken.vue, if you run the unit test you will see that the snapshot no longer renders out the slots.

@lmiller1990
Copy link
Member

I read through this and have some questions. Is this bug only snapshots? I'd say a shallow mount + snapshot bug is not that big a deal - realistically, these tests are very brittle and fairly useless when it comes to actually verifying your product is working correctly.

If there's a bug relating to actual slots and the v-slot syntax, we should fix it. Is this the case?

@rikbrowning
Copy link
Author

I am also not able to find any of the elements in the slots either so it is not simply snapshot related.

@rikbrowning
Copy link
Author

Did some further digging.

Issue seems to be in createStubFromComponent. When we add in the div.test-wrapper around the FakeComponent, the check for slots (ctx.$options.parent._vnode.data.scopedSlots) when rendering FakeComponent is actually checking the div.test-wrapper for scopedSlots. That means no slots are getting rendered.
Without the div.test-wrapper the check ctx.$options.parent._vnode.data.scopedSlots returns the correct data.

I noticed the following comment:

  // In Vue 2.6+ a new v-slot syntax was introduced
  // scopedSlots are now saved in parent._vnode.data.scopedSlots

I presume there is good reason for this change but I found that using ctx.$vnode.data.scopedSlots worked for both cases.

@lmiller1990
Copy link
Member

lmiller1990 commented Jul 12, 2021

Stubs are super messy - if you found a fix, make it and create a PR and see if CI fails :) I can't really remember what each and every line does there, but there should be tests around it all.

rikbrowning pushed a commit to rikbrowning/vue-test-utils that referenced this issue Jul 13, 2021
@rikbrowning
Copy link
Author

@lmiller1990 hey I have submitted a PR but seems to be failing on the CI even though the only thing added was tests. Any idea how to fix that?

@lmiller1990
Copy link
Member

Just took a look: https://app.circleci.com/pipelines/github/vuejs/vue-test-utils/570/workflows/29f090d2-6a16-4bcc-993d-63e09c4559d2/jobs/16342

image

Can you try running yarn format:check and fixing any problems? I think yarn format should auto fix most of the linting problems.

I am surprised your fix doesn't break things. Anyway, try fixing the linting and pushing again, see what happens. Thanks!

@rikbrowning
Copy link
Author

Hey was able to get the formatting sorted but can't seem to get flow check to pass:
image
Not quite sure what I need to do to get the flow to work.
Able to get test:unit to pass no issue, just failing on test.

@lmiller1990
Copy link
Member

lmiller1990 commented Jul 28, 2021

Can you push what you have? I can't see that error in CI. I can take a look, need more context to provide guidance. Where are you doing that import? It should work like that - we have that same line in many places. 🤔

@rikbrowning
Copy link
Author

Hey, pushed my latest changes to https://github.com/rikbrowning/vue-test-utils

@andrewbrennanfr
Copy link

Any news on this issue?

I've started removing all the old deprecated slot syntax from our app & a bunch of tests are failing.

Some tests kind of work, if you explicitly set a default slot. But this doesn't solve every case & in reality, I'd like to keep all our tests as they are.

The most simple example I have is:

<my-button>
  <template v-slot="prepend">
    <span>Hello</span>
  </template>

  <div>World!</div>
</my-button>

What I receive:

<my-button-stub>
  <div>World!</div>
</my-button-stub>

What I expect:

<my-button-stub>
  <span>Hello</span>
  <div>World!</div>
</my-button-stub>

@lmiller1990
Copy link
Member

lmiller1990 commented Sep 24, 2021

Could you try building https://github.com/vuejs/vue-test-utils/pull/1877/files locally and see if it solves your problem?

Sorry for the lack of activity here ... I have mostly been focused on VTU next, for Vue 3. I am also really hesitant to make major changes to the slots part of the code base, although we have decent test coverage, every time we change that part of the code base, it breaks a bunch of people's test suites.

If you can confirm this works, we could probably run the branch against some large OSS code bases and see if it breaks anything (namely GitLab, they have a huge suite using VTU).

@andrewbrennanfr
Copy link

andrewbrennanfr commented Sep 24, 2021

@lmiller1990

So I tried to quick route & just added the changes in the PR to our project's node_modules directly. 😅
It doesn't however solve my problem. 😓

The first issue is this.$options._renderChildren is populated here so I never even get inside getScopedSlotRenderFunctions.

And then secondly, even when removing this condition for testing, the proposed change doesn't work.

I checked out the proposed PR to see if I could make the tests fail & I can! 😄

By adding a default slot & some content, the named slot no longer renders.
So the test for .new-example existing in the DOM fails.

However, explicitly setting a default slot by name, correctly passes all slots (which is what I had seen above ☝️ ).
So the tests do fail, but they fail for the reason I'd expect.

You can see my fork here with the failing test when passing a default slot: https://github.com/andrewbrennanfr/vue-test-utils/commit/3c30a68e587a1abd0a101b77248917014fc0036b

And also with the default slot being explicitly named, the test above ☝️ no longer fails:
https://github.com/andrewbrennanfr/vue-test-utils/commit/f07f218c4ffe2defe07800e985fa6e09bcc7b5bb


Edit:
I've played around with the tests & it looks like we aren't well handling the default slot case when not explicitly named as #default. And for me it's the this.$options._renderChildren ||.

I think the proposed fix works to make named slots render correctly. But when paired with default slots, they are still omitted.

@andrewbrennanfr
Copy link

Something like this: https://github.com/andrewbrennanfr/vue-test-utils/commit/e03cb4c4786f73583609f6a87b126553fa53824f fixes the issue for me (also including the proposed changes from @rikbrowning )

Although I'm completely unsure of the impact of this change. 😅

But all tests are green, and the two broken use cases for me look to be fixed too.

@andrewbrennanfr
Copy link

Apparently this is the intended behavior in vue though 🤷‍♂️

https://vuejs.org/v2/guide/components-slots.html#Abbreviated-Syntax-for-Lone-Default-Slots

Note that the abbreviated syntax for default slot cannot be mixed with named slots, as it would lead to scope ambiguity

@snoozbuster
Copy link

@andrewbrennanfr those repo links you posted 404 for me now. I'm hitting this same issue and I'd love to see how you fixed it!

@ebisbe
Copy link
Collaborator

ebisbe commented Feb 15, 2023

I think this falls into #1564 (comment) @lmiller1990 , right?

@ebisbe ebisbe added the stubs label Feb 15, 2023
@lmiller1990
Copy link
Member

lmiller1990 commented Feb 16, 2023

Yeah, I'd say so - anything around shallowMount and stubs is pretty risky/messy to change at this point. If there's a quick/easy fix, that'd be fine. I won't be able to work on this right now, though.

@LeonardoRick
Copy link

I'm new on vue but I think I found a temporary solution that can help someone struggling with this issue. Its based on this reference:

  wrapper = mount(TestedComponent, {
    attachToDocument: true,
    stubs: {
      InnerComponent: mockSlots([['slotName', '']]),
    },
  });

And this the mockSlots function:

function mockSlots(scopedSlots) {
  return {
    render(h) {
      return h(
        'div',
        scopedSlots.map(([name, slotProps]) => this.$scopedSlots[name](slotProps))
      );
    },
  };
}

If someone could validate it or even improve that would be great :)

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

No branches or pull requests

7 participants