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

Slotting behaviour #482

Closed
nandi95 opened this issue Mar 27, 2021 · 13 comments · Fixed by #599
Closed

Slotting behaviour #482

nandi95 opened this issue Mar 27, 2021 · 13 comments · Fixed by #599

Comments

@nandi95
Copy link
Contributor

nandi95 commented Mar 27, 2021

Hi,

I would like to do the two following things:

  • Slot multiple components into the default or named slots.
  • Give the slotted components props (perhaps give the slot a mount(Comp))

This isn't necessarily a code smell that the testing scope is too wide. My use case is I have a component that has a default slot into which you pass multiple components of one type.

<AvatarGroup>
    <Avatar>
<AvatarGroup />

The second would also be very useful when the user trying to test some unexpected patterns such as accessing children or parent.

Are these already possible I just missed something and perhaps the documentation can use some improvement?
Without delving into the source is this possible to implement? Is it desired?

@lmiller1990
Copy link
Member

lmiller1990 commented Mar 30, 2021

Just to confirm what you'd like to do...

  1. multiple components

So something like:

Blah.vue:

<template>
  <slot />
</template>

Usage:

<template>
  <blah>
    <span /><foo /?>
  </blah>
</template>

And the problem is you cannot pass in <span /><foo /> to slots: { default: '...' } }?

This does seem like a problem. We might want to support passing an array to default? Eg:

mount(Blah, {
  slots: {
    default: [Foo, Bar] // or [h(Foo), h(Bar)]
  }
})

It looks like you should be able to accomplish this already using a template, see this test.

This might also solve your props problem?

@nandi95
Copy link
Contributor Author

nandi95 commented Mar 30, 2021

In the linked test, it only tests with a single html element
And if this html string includes a component you would get the warning

Failed to resolve component: Blah

This would work if to the mount the first argument is a wrapping component in which would have the template and the components both including the desired components. Something like

const wrapper = mount({
    template: `
        <ParentComp prop="something">
          <ChildComp prop="foo" />
          <ChildComp prop="bar" />
        </ParentComp>`,
    components: { ParentComp, ChildComp }
});

The h(Foo) is excellent, probably can be included in the docs somewhere? (more explicit docs or a cookbook section?)

The array to slots is probably a good idea.

@lmiller1990
Copy link
Member

We should include h(Foo) in the docs. Supporting an array should be pretty easy - if it's an array, just return a h function with an array.

Another alternative might be supporting h as slots (no idea if this works). Something like

mount(Foo {
  slots: {
    default: h(Bar, { someProp: 'val' })
  }
})

That solves the props problem too. 🤔

@cowills
Copy link

cowills commented Apr 22, 2021

Running into this same issue being unable to pass an array into a slot, would be happy to try to implement that in VTU if that work hasn't been started and you can point me in the right direction. Not sure how to work around it currently without wrapping the array contents in a div.

@lmiller1990
Copy link
Member

Sure @cowills. Looks like the slot stuff is here, I guess you can just check if it's an array an iterate over it.

I'd recommend figuring out the exact API first. How do you think it will look?

mount(Foo, {
  slots: {
    default: [Foo]
  }
})

Do you need to pass props to the slots? How would that look?

Crazy idea might be:

mount(Foo, {
  slots: {
    default: [
      {
        component: Foo, props: { msg: 'Hello' }
      }
    ]
  }
})

Not sure how useful this would be. Maybe there is a nicer API. We should figure this out before we write too much code.

@nandi95
Copy link
Contributor Author

nandi95 commented Apr 26, 2021

I think both of those proposed should be accepted.


Without looking at the code, would it be possible to give it a wrapper or instantiated vm? Like:

const childWraper = mount(Bar)

mount(Foo, {
  slots: {
    default: [
      childWrapper,
      // or
      childWrapper.vm
    ]
  }
})

That could be cool if I don't have to reach for getComponent() and already have it ready. But also not a pain if doesn't happen.

@cowills
Copy link

cowills commented Apr 26, 2021

couldn't it just support whatever component styles a non-array slot does? im not sure i understand the need for an additional API beyond supporting an array of components like VTU for vue2 currently supports

slots: {
  default: [
    '<div>plain html</div>',
    FooComponent, 
    h(FooComponent, {props: 'stuff'})
  ]
}

an example from vue2 VTU from the codebase i work on:
https://github.com/rei/rei-cedar/blob/next/src/components/chip/__tests__/CdrChipGroup.spec.js#L15-L18

@lmiller1990
Copy link
Member

I didn't realize we supported an array in VTU v1. We should support whatever we support there. Ideally these libraries will have the same API so people can easily migrate from Vue 2 to Vue 3.

I say we start with the above post (matching Vue 2/VTU v1 API). We can discuss additional improves separately. Would you like to work on this @cowills? I think this should not be too difficult, since it's basically the current behaviour, but you need to loop over the content if it's an array.

@cowills
Copy link

cowills commented Apr 27, 2021

Not sure I understand typescript or vue internals well enough to make the change sadly, here's what I came up with:

slots.spec.ts

    it('supports providing an array to a slot', () => {
      const defaultSlotTemplate = '<div><p class="defaultTemplate">Content</p></div>'

      const wrapper = mount(ComponentWithSlots, {
        slots: {
          default: [
            'plain string slot', 
            defaultSlotTemplate, 
            Hello, 
            h('span', {}, 'Default')
          ],
        }
      })

      expect(wrapper.text().includes('plain string slot')).toBe(true)
      expect(wrapper.find('.defaultTemplate').exists()).toBe(true)
      expect(wrapper.find('#msg').exists()).toBe(true)
      expect(wrapper.find('span').text()).toBe('Default')
    })

mount.ts

  function handleSlot(slot: Slot): Function {
    // case of an SFC getting passed
    if (typeof slot === 'object' && 'render' in slot && slot.render) {
      return slot.render
    }

    if (typeof slot === 'function') {
      return slot
    }

    if (typeof slot === 'object') {
      return () => slot
    }

    if (typeof slot === 'string') {
      // if it is HTML we process and render it using h
      if (isHTML(slot)) {
        return (props: VNodeProps) => h(processSlot(slot), props)
      }
      // otherwise it is just a string so we just return it as-is
      else {
        return () => slot
      }
    }
    return () => ''
  }

  // handle any slots passed via mounting options
  const slots =
    options?.slots &&
    Object.entries(options.slots).reduce(
      (
        acc: { [key: string]: any },
        [name, slot]: [string, Slot]
      ): { [key: string]: Function } => {
        if (Array.isArray(slot)) {
          acc[name] = slot.map(handleSlot) // should this be a function? an h() wrapping the array of slots?
        } else {
          acc[name] = handleSlot(slot)
        }
        return acc
      },
      {}
    )

@lmiller1990
Copy link
Member

Seems legit to me. Do you want to make a PR adding the test and code? It's fine if other tests break, I can help out.

Alternatively, if you don't have the time/bandwidth, I can pick this up using your code above. Either way, let me know. :)

@cowills
Copy link

cowills commented May 6, 2021

Seems legit to me. Do you want to make a PR adding the test and code? It's fine if other tests break, I can help out.

Alternatively, if you don't have the time/bandwidth, I can pick this up using your code above. Either way, let me know. :)

Sure! i just pushed up my branch and made a PR.

If anyone reading this needs a quick workaround, you can wrap the component you are testing in an h() and pass an array slot directly there like so:

   const wrapper = mount(h(ComponentYouAreTesting, { propsAndSuch: 'whatever' }, {default: () => [
      h(SlotComponentYouAreTesting, {propsAndSuch: 'first array element''}),
      h(SlotComponentYouAreTesting, {propsAndSuch: 'second array element''}),
    ]}));

@lmiller1990
Copy link
Member

Thanks, I'll continue on from your PR this week.

@lmiller1990
Copy link
Member

This feature has been implemented anad will be released soon.

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

Successfully merging a pull request may close this issue.

3 participants