generate named slots content for stubbed components via shallowMount #1309

chriswa · 2019-09-13T19:13:35Z

resolves #1307

chriswa · 2019-09-13T22:23:07Z

Hi! This is my first ever OSS contribution. Please let me know any feedback you may have! :)

chriswa · 2019-09-17T20:54:44Z

I did create an Issue for this PR, but I guess I failed to link it:
#1307

ndchelsea · 2019-09-25T21:57:18Z

This is a super helpful fix... What's the chances of this being merged?

GiacomoVoss · 2019-11-26T12:35:54Z

Any updates on this? It's really helpful and would make snapshot testing with Vuetify components a lot better.

lmiller1990 · 2020-01-03T07:40:56Z

Hey @chriswa , awesome first OSS contribution. I will review this now and give you feedback so w can get this merged.

lmiller1990

This is an awesome first OSS contribution. Left a few comments, only one really requires any action (first one about ignored elements).

Might also be good to rebase on the last develop. Great job! I can't wait to get this one merged in.

lmiller1990 · 2020-01-03T07:58:03Z

packages/create-instance/create-component-stubs.js

+      const slots = context ? context.slots() : this._renderProxy.$slots
+
+      // ensure consistent ordering of slots (default first, then alphabetical)
+      const sortedSlotEntries = Object.entries(slots)


Nice usage of Object.entries

lmiller1990 · 2020-01-03T07:59:55Z

packages/create-instance/create-component-stubs.js

+          ? -1
+          : slotNameB === 'default'
+          ? 1
+          : slotNameA.localeCompare(slotNameB)


I would have used a simple > and < for the string compare. Not sure it makes a big difference - it is common to use characters like á in code? (my language does not have those characters, so I have never seen this before).

If we are doing to support using localeCompare, maybe we should have a test containing a á character vs an a character. Although probably not strictly necessary.

Just a thought, this kind of ternary is super confusing. I would deff advise for some sort of reordering or other.

lmiller1990 · 2020-01-03T08:01:25Z

test/specs/shallow-mount.spec.js

+
+  it('renders named slots for functional components', () => {
+    const localVue = createLocalVue()
+    localVue.component('child', {


nice job! it's easy to forget the case of functional components.

lmiller1990 · 2020-01-03T08:01:43Z

packages/create-instance/create-component-stubs.js

@@ -86,31 +86,48 @@ export function createStubFromComponent(
  // ignoreElements does not exist in Vue 2.0.x
  if (Vue.config.ignoredElements) {
    Vue.config.ignoredElements.push(tagName)
+    if (Vue.config.ignoredElements.indexOf('template-stub') === -1) {


Why is this necessary?

So the template-stub stays as <template-stub /> in the finished render, correct?

lmiller1990 · 2020-01-03T08:02:54Z

@chriswa left some comments, let me know what you think. Excited to get this one in.

dobromir-hristov · 2020-01-03T08:07:44Z

packages/create-instance/create-component-stubs.js

+          ? -1
+          : slotNameB === 'default'
+          ? 1
+          : slotNameA.localeCompare(slotNameB)


Just a thought, this kind of ternary is super confusing. I would deff advise for some sort of reordering or other.

dobromir-hristov · 2020-01-03T08:10:23Z

packages/create-instance/create-component-stubs.js

@@ -86,31 +86,48 @@ export function createStubFromComponent(
  // ignoreElements does not exist in Vue 2.0.x
  if (Vue.config.ignoredElements) {
    Vue.config.ignoredElements.push(tagName)
+    if (Vue.config.ignoredElements.indexOf('template-stub') === -1) {


So the template-stub stays as <template-stub /> in the finished render, correct?

dobromir-hristov · 2020-01-03T08:11:33Z

packages/create-instance/create-component-stubs.js

+          : slotNameA.localeCompare(slotNameB)
+      )
+
+      const children = sortedSlotEntries.map(([slotName, slotChildren]) =>


So this should render the default slot, and all others as children of the template-stub? Wont this be a super breaking change for snapshots?

True, this will break all snapshots using the original behavior. We probably should have some snapshot tests in VTU as well to capture these regressions. But then we will need to move to Jest, I think? Or we can do a manual comparison between strings.

Probably not the biggest deal, we just need to make sure that's clear in release notes. Something like:

NOTE: You will need to update all snapshots when updating.

JessicaSachs · 2020-01-21T11:11:28Z

packages/create-instance/create-component-stubs.js

+//
+//   return []
+// }
+


So @lmiller1990 I'm mostly concerned about why no tests break when I comment this out

JessicaSachs · 2020-01-21T11:11:56Z

packages/create-instance/create-component-stubs.js

-  hyphenate,
-  keys
+  hyphenate
+  // keys


Commented out for formatting until we can figure out how to merge everything in.

JessicaSachs · 2020-01-21T11:13:04Z

packages/create-instance/create-component-stubs.js

+      //     this.$options.parent._vnode.data.scopedSlots[x]()
+      //   )
+      // }
+


This was initially how I expanded out and merged dev and this PR, but was failing so I commented it out and reverted to mainly this PR's version of rendering slots

JessicaSachs · 2020-01-21T11:18:21Z

test/specs/shallow-mount.spec.js

@@ -130,7 +136,11 @@ describeRunIf(process.env.TEST_ENV !== 'node', 'shallowMount', () => {
    expect(wrapper.find({ name: 'Foo' }).exists()).to.equal(true)
    expect(wrapper.find('.new-example').exists()).to.equal(true)
    expect(wrapper.html()).to.equal(
-      '<foo-stub>\n' + '  <p class="new-example">text</p>\n' + '</foo-stub>'
+      '<foo-stub>\n' +
+        '  <template-stub slot="newSyntax">\n' +


The need to update all the assertions with template-stub worry me. This will be a breaking change :/

afontcu · 2020-04-05T08:33:08Z

Hi! As mentioned here, we're not merging this before v1 is released.

Just in case someone stumbles upon this PR – there are 2 workarounds:

use mount
write your own stub that implements the named slots

thanks for the effort put on this one, though! 🤗

lmiller1990 · 2020-04-05T12:59:49Z

I thought about this a bit more, I'm not convinced this behavior is actually correct. I think children of stubbed components, including slots, should not be rendered. I am in favor of closing this as a wontfix.

We need to figure out what shallowMount should/shouldn't do.

lmiller1990 · 2020-04-08T05:19:04Z

This looks to be a significant a breaking change at this point. I also am not sure this actually makes sense - I think stubbed children should not render their slots.

I'm going to close this one. Thanks for the work - sorry the team was unresponsive and this didn't get the attention it should have when the PR was opened.

We are looking to a v1 release soon, and we will revisit shallowMount and what it should be rendering for the Vue 3 integration.

chenxeed · 2020-05-10T12:51:54Z

Hi @lmiller1990 , just sharing my opinion here regarding your thought:

I think children of stubbed components, including slots, should not be rendered.

I'm afraid if there's multiple interpretation of defining this use-case.

I agree that generally, shallowMount shouldn't render the child component and simply stub them.

However, slot is different. Slot is a way for the parent to "inject" the content they expect in the child component. The child is responsible for the placement of the slot, but it's the parent responsibility to ensure the content injected are working as expected.

Thus, testing the slot content is the parent responsibility.

For example, when we have a component that needs to load a modal component like b-modal with specific content for the named slot:

// parent component
<template>
  <b-modal v-model="visible">
    <template #modal-header>
      {{ title }}
      <button @click="onClickModalHeader">
    </template>
</b-modal>

On testing the parent component, I don't care with how the <b-modal> renders the slot since I believe b-modal will do its job properly. I just want to make sure the content of the header are rendering the title that I expect and the button inside are working.

note: example with jest

const parentComponentWrapper = shallowMount(parentComponent);

parentComponent.setData({ title: 'The Parent Title' });
const modal = parentComponent.findComponent({ name: 'BModal' });
expect(modal.html()).toContain('The Parent Title'); // better if can assert which slot it's expecting
modal.find('button').trigger('click')
// assert the side effect from `onClickModalHeader`

The idea is, slots are used by parent for the parent's logic, and we need to test them too. Using mount to test them will defeat the purpose of shallowMount to only render components from the parent layer.

I beg you to reconsider 🙇 and let me know if my example above doesn't makes sense or not applicable. Thank you 🙇

dobromir-hristov · 2020-05-10T13:08:55Z

This is taken care of in Vue 2. Default slots are rendered behind a flag, named slots only via a special helper. :)

chenxeed · 2020-05-10T13:42:33Z

Thanks to respond @dobromir-hristov

named slots only via a special helper

Can you clarify what's the special helper you mentioned about?

Is it the manual stub that @danieltian suggested:

const wrapper = shallowMount(RootComponent, {
  stubs: { ChildComponent }
}

expect(wrapper.find(ChildComponent).text()).toContain('some text');

dobromir-hristov · 2020-05-10T13:45:11Z

Sort of. In shallowMount default slots can be auto rendered, but not named and scoped slots. We cant know that data, as it is deeply buried in the render function tree of the components you are stubbing. Does it make sense?

I tried to make this for a while, bit there is just no sensible way to do render all slots, while still stubbing.

chenxeed · 2020-05-10T13:48:47Z

@lmiller1990 @dobromir-hristov I've seen the discussion has been going on heavily in #1307 which has better explanation than mine. My appreciation to you and the core team for the stable release, and taking in the concern of this issue ❤️

Does it make sense?

Yes, seems like the issue here is more towards the implementation, not the goal behind.

I agree with @lmiller1990 suggestion that this behavior can be made as optional.
I'll followup the discussion from #1307 and look forward in the next vue-test-utils.

Again, thank you! 🥇

lmiller1990 self-requested a review January 3, 2020 07:41

lmiller1990 reviewed Jan 3, 2020

View reviewed changes

lmiller1990 self-assigned this Jan 3, 2020

dobromir-hristov reviewed Jan 3, 2020

View reviewed changes

Chris Waddell and others added 2 commits January 21, 2020 10:28

enforce consistent sort order of named slots in shallowMount output

a7fdcbd

chore: rebasing create-component-stubs

97228ce

JessicaSachs force-pushed the shallowMount-render-named-slots branch from f437d08 to 97228ce Compare January 21, 2020 11:10

JessicaSachs reviewed Jan 21, 2020

View reviewed changes

vetruvet mentioned this pull request Jan 22, 2020

named slots not rendering in children of other components with shallowMount #1413

Closed

afontcu mentioned this pull request Apr 3, 2020

To do before release #246

Closed

13 tasks

lmiller1990 closed this Apr 8, 2020

danieltian mentioned this pull request Apr 22, 2020

Named slots' content is not generated for stubbed components via shallowMount #1307

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

generate named slots content for stubbed components via shallowMount #1309

generate named slots content for stubbed components via shallowMount #1309

chriswa commented Sep 13, 2019 •

edited

chriswa commented Sep 13, 2019

chriswa commented Sep 17, 2019

ndchelsea commented Sep 25, 2019

GiacomoVoss commented Nov 26, 2019

lmiller1990 commented Jan 3, 2020

lmiller1990 left a comment

lmiller1990 Jan 3, 2020

lmiller1990 Jan 3, 2020

dobromir-hristov Jan 3, 2020

lmiller1990 Jan 3, 2020

lmiller1990 Jan 3, 2020

dobromir-hristov Jan 3, 2020

lmiller1990 commented Jan 3, 2020

dobromir-hristov Jan 3, 2020

dobromir-hristov Jan 3, 2020

dobromir-hristov Jan 3, 2020

lmiller1990 Jan 3, 2020 •

edited

JessicaSachs Jan 21, 2020

JessicaSachs Jan 21, 2020

JessicaSachs Jan 21, 2020

JessicaSachs Jan 21, 2020

afontcu commented Apr 5, 2020

lmiller1990 commented Apr 5, 2020 •

edited

lmiller1990 commented Apr 8, 2020 •

edited

chenxeed commented May 10, 2020

dobromir-hristov commented May 10, 2020

chenxeed commented May 10, 2020

dobromir-hristov commented May 10, 2020

chenxeed commented May 10, 2020

generate named slots content for stubbed components via shallowMount #1309

generate named slots content for stubbed components via shallowMount #1309

Conversation

chriswa commented Sep 13, 2019 • edited

chriswa commented Sep 13, 2019

chriswa commented Sep 17, 2019

ndchelsea commented Sep 25, 2019

GiacomoVoss commented Nov 26, 2019

lmiller1990 commented Jan 3, 2020

lmiller1990 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmiller1990 commented Jan 3, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmiller1990 Jan 3, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afontcu commented Apr 5, 2020

lmiller1990 commented Apr 5, 2020 • edited

lmiller1990 commented Apr 8, 2020 • edited

chenxeed commented May 10, 2020

dobromir-hristov commented May 10, 2020

chenxeed commented May 10, 2020

dobromir-hristov commented May 10, 2020

chenxeed commented May 10, 2020

chriswa commented Sep 13, 2019 •

edited

lmiller1990 Jan 3, 2020 •

edited

lmiller1990 commented Apr 5, 2020 •

edited

lmiller1990 commented Apr 8, 2020 •

edited