-
-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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
Bug: Type support for Vue component slots in stories #21221
Comments
When this is done, we should also improve the documentation for how to use slots in stories.
|
Just a small detail, the boilerplate for Vue docs can also be simplified if using TypeScript 4.9 by writing: export default {
// ...
} satisfies Meta<typeof Button> ...instead of first assigning to another variable. |
I'm looking at that example, and it's not clear to me how that Vue3 / TS example would compile, based on current typings and the regression in slot support. Are the examples type-checked? |
@matthew-dean the reason we don't use that syntax is that we're promoting the following pattern: const meta = { } satisfies Meta<...>;
export default meta;
type Story = StoryObj<typeof meta>;
export const Story1: Story = { ... } This allows you to split required args across both the default export & the story exports, at the expense of an extra line of code. |
@kasperpeulen calling this a bug, but if it's actually a feature request feel free to relabel and we can address in 7.1 |
The link was not leading to the correct example (has been updated now). Just scroll down to the heading "Args can modify any aspect of your component" and look at the TS-3 example (with Vue and SB7 selected). |
@CasperSocio @shilman Actually in the current version Vue3/TS SB7, you should not be able to pass default as arg cause it is not a prop of the component, however, you can pass it in Slot control. The only downside here is the control type which is JSON object by default, we can easily support string as well, but for a proper type like HTML may be relevant here, definitely, we can have it in 7.1 |
args.footer would still cause a type error, unless this has been fixed. The typings don't add the slot names to the args property keys, so I feel like the Vue 3 / TS example is not real.
In what control? How? Do you have an example? |
if you component has slots it will be shown in the control in SLOTS section , see here i have 2 default and icon and i can passe as JSON format |
In my example at the top, you'll find the
|
@chakAs3 Maybe we can pair up on this? @CasperSocio As a workaround you can do: type Story = StoryObj<typeof meta> & { default: string; sectionAside: string } I'm not 100% sure how slots in Vue work, and what makes sense here. Those slots args are only used if there is a custom render function right? Does it make sense to show autocompletion for the slots, even when they are not referenced in the render function? And what if a user wants to do something differently, then use the named slots: render: (args) => ({
components: {
Select,
Option,
},
setup() {
return { args }
},
template: `
<Select v-bind="args">
<Option>{{ args.option1 }}</Option>
<Option>{{ args.option2 }}</Option>
</Select>
`,
}), |
If you look at how Cypress is currently implementing component testing, you have: cy.mount(Button, {
props: {
icon: 'add',
},
slots: {
default: () => 'Add new',
},
}) This would translate to: const meta: Meta<typeof Button> = {
title: 'Components/Button',
component: Button,
render: (args, slots) => ({
components: {
Button,
},
setup() {
return { args, slots }
},
template: `
<Button v-bind="args">{{ slots.default }}</Button>
`,
}),
argTypes: {
icon: {
control: 'select',
options: ['add', 'remove', 'save'],
},
},
args: {
icon: 'add',
},
slots: {
default: 'Add new',
},
} This would separate slots from That said, hasn't this been available for React a while now? I worked with TypeScript and React when I first started using Storybook and can't recall having these issues. There's probably an existing implementation for React that can pave the way for how to implement this. |
The Vue TS-3 example (Args can modify any aspect of your component) uses I'll set up a React project to verify that |
I can confirm that SB7 has full support for Button.tsximport './button.css'
interface ButtonProps {
children: string
icon?: 'add' | 'arrow-left' | 'arrow-right' | 'delete' | 'remove' | 'save'
primary?: boolean
size?: 'small' | 'medium' | 'large'
onClick?: () => void
}
export const Button = ({
children,
icon,
primary = false,
size = 'medium',
...props
}: ButtonProps) => {
const mode = primary ? 'storybook-button--primary' : 'storybook-button--secondary'
return (
<button
type="button"
className={['storybook-button', `storybook-button--${size}`, mode].join(' ')}
{...props}
>
{
icon && <img src={`../assets/icons/${icon}.svg`} alt={`${icon} icon`} />
}
{children}
</button>
)
} Button.stories.tsximport { Meta, StoryObj } from '@storybook/react'
import { Button } from './Button'
const meta: Meta<typeof Button> = {
title: 'Example/Button',
component: Button,
args: {
children: 'Click me',
primary: false,
size: 'medium',
},
}
export default meta
type Story = StoryObj<typeof meta>
export const Primary: Story = {
args: {
primary: true,
}
}
export const Secondary: Story = {}
export const Large: Story = {
args: {
size: 'large',
}
}
export const Small: Story = {
args: {
size: 'small',
}
} There's not even a need for Vue is such a fast growing framework, we should be able to have the same amount of SB features as for React. |
Hi @CasperSocio I even think that Vue has more features when it comes to slots, it is really incredible how named and scoped slots can give you encapsulation of Logic or UI, also Dynamics slots, which is something that svelte can't do because svelte is based heavily on the build. |
Yes @kasperpeulen sure, I was thinking a bit about what we can do, this can be a big feature. |
I was thinking about using volar ( 'vue-component-meta' package) to replace the current docgen we have for Vue. |
Sounds like a good idea @chakAs3 Probably after the 7 release. Trying to implement the types now, the type definition should also support scoped slots right? |
it should definitely work and it is really similar, I haven't tested it specifically but I guess it works like normal named slots. |
Okay, I got something working on typelevel, will finalize tomorrow, and check if it also works on runtime: <script setup lang="ts">
defineProps<{ otherProp: boolean; }>();
</script>
<template>
<div class="container">
<header>
<slot name="header" title="Some title"></slot>
</header>
<main>
<slot></slot>
</main>
<footer>
<slot name="footer"></slot>
</footer>
</div>
</template> test('Infer type of slots', () => {
const meta = {
component: BaseLayout,
} satisfies Meta<typeof BaseLayout>;
const Basic: StoryObj<typeof meta> = {
args: {
otherProp: true,
default: () => 'Default',
header: ({ title }) => `<h1>Some header with a ${title}</h1>`,
footer: () => 'Footer',
},
};
type Props = {
readonly otherProp: boolean;
header?: (_: { title: string }) => any;
default?: (_: {}) => any;
footer?: (_: {}) => any;
};
type Expected = StoryAnnotations<VueRenderer, Props, Props>;
expectTypeOf(Basic).toEqualTypeOf<Expected>();
}); |
It is quite neat that default h function (that we use as default render function) also accept non-string slots, like so: export const Primary: Story = {
args: {
otherProp: true,
header: ({ title }) =>
h({
components: { Button },
template: `<Button :primary='true' label='${title}'></Button>`,
}),
default: 'default slot',
footer: h(Button, { primary: true, label: 'footer' }),
},
}; |
Egads!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.0.0-beta.61 containing PR #21359 that references this issue. Upgrade today to the
|
@shilman I just upgraded and am still getting: Edit: Okay, I got it working. I wiped node_modules and re-installed, but I still had an error because I had this: export default {
title: 'Stack',
component: Stack,
render: <T,>(args: T) => ({
components: { Stack, Box },
setup() {
return { args }
},
template: `<Stack v-bind="args">${args.default}</Stack>`
})
} satisfies Meta<typeof Stack> I had to remove the generic type to get it to work: export default {
title: 'Stack',
component: Stack,
render: args => ({
components: { Stack, Box },
setup() {
return { args }
},
template: `<Stack v-bind="args">${args.default}</Stack>`
})
} satisfies Meta<typeof Stack> Thanks so much! |
What would really make this rock is to not have to write a render function in order to get this to work (similar to the auto-render functions provided for React). 😁 |
@matthew-dean same feeling here, that will have more rocking features. already in mind, if you have any you can share them with us |
I don't understand, it works without render functions right? @matthew-dean @chakAs3 ? |
yes @kasperpeulen i can't see the issue here ? |
So then we already have this implemented right? Or do we need to do something more to make it more in line with what we do in React? |
no it is there that is why we have export const render: ArgsStoryFn<VueRenderer> = (props, context) => {
const { id, component: Component } = context;
if (!Component) {
throw new Error(
`Unable to render story ${id} as the component annotation is missing from the default export`
);
}
return h(Component, props, getSlots(props, context));
}; in renderer render function we pull the info from docgen and generate the slots content |
@kasperpeulen i guess there is some bugs but no worries i'm already reworking the renderer render in reactive way i'm will be checking this and fix the bug i will add you to review the PR if you don't mind |
@kasperpeulen Yes, I was mistaken. However, I think adding arg types for slots opens up another bug, or at least something in conflict with documentation. That is, if I do a standard pattern, according to the docs of: template: `
<Dialog v-bind="args">
{{ args.default }}
</Dialog>
` The problem is that all args are bound to the outer element, including the default slot. So I get Vue warnings in the console, and the contents of |
I really apriciate the work put into this bug guys! TS is happy again in
Using vite v4.1.4 and vite-plugin-dts 2.1.0, which is throwing the errors, but it doesn't prevent the build from completing. Nor does it impact the SB instance I'm hosting om Firebase. Just thought you should know that there's probably missing type exports 😅 Again: Thanks guys, stories are going to be a lot easier to write now |
Son of a gun!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.1.0-alpha.7 containing PR #21954 that references this issue. Upgrade today to the
|
Sorry for missing this issue, I noticed that PR now uses vue-tsc to generate component types, volar v1.3 adds a new vue-component-type-helpers package, which is used behind vue-component-meta and can be used to extract The component type generated by vue-tsc, this may also apply to storybook. Please note that v1.3 is a pre-release version of v1.4, there are still some unresolved edge cases, you can also wait for v1.4. |
Given we're dealing with slots designed to accept HTML, this should probably be defaulting to binding with For those looking for a working solution without the non-javascript code mixed in.
|
@mryellow You can also do: export const Example = {
args: {
// Props
bar: 'baz',
// Slots
default: h({ template: '<img src="/images/testpattern.jpg"/>' }),
},
} |
How would one go about passing slot properties? For example if we wish to fire a method ( Component:
Usage:
Attempting the slot pattern from https://vuejs.org/guide/extras/render-function#rendering-slots the ... I can wrap the entire component in a demo version which initialises the components, defines slots and attaches their events. Then exposes new properties for things within that template (i.e. However I'd like to give the end-user the ability to simply edit the template as a string and have the correct context so that properties from the displayed component are passed to templates as required. |
Struggled to get anything out of attempting to improve This pattern looks workable. By injecting the string directly into that slot it binds the slot properties correctly.
|
@mryellow Do you mean something like this? <!-- <MyComponent> template -->
<script setup lang="ts">
const props = defineProps({
label: String,
year: Number,
});
</script>
<template>
<div data-testid="scoped-slot">
<slot :text="`Hello ${props.label} from the slot`" :year="props.year"></slot>
</div>
</template> const meta = {
component: MySlotComponent,
} satisfies Meta<typeof MySlotComponent>;
export default meta;
type Story = StoryObj<typeof meta>;
export const Basic: Story = {
args: {
label: 'Storybook Day',
year: 2022,
default: ({ text, year }) => `${text}, ${year}`,
},
} This is supported. |
There is no slot component, only native HTML placed into an The contents of the HTML may be
There is no preset expectations of what this content might look like, only that it must fire a method provided by slot property.
Only when given a |
Is your feature request related to a problem? Please describe
Storybook 7 introduces many improvements for type support when using Vue. Still, we're seeing errors when trying to pass arguments to component slots.
Below, I'm making stories for my button component. Instead of passing text (content) to a label prop, I want my button component to behave like a native HTML element. The button component has a single, unnamed slot (default) where the content goes. The correct way to set up the default slot is with args/argTypes:
The code runs correctly and the story controls allows me to change the button text, but TypeScript is not happy.
Describe the solution you'd like
Add type support for Vue component slots, both default slot and named.
The
BaseAnnotations
type, which is extended by theComponentAnnotations
interface, holds the type definitions forargs
(Partial<TArgs>
) andargTypes
(Partial<ArgTypes<TArgs>>
). From what I've been able to find, this seems like the right place to do the implementation.TArgs
is a mapped type by default, but is likely assigned only the component props. Then the implementation might be deeper than I've been able to look at this point.Describe alternatives you've considered
The alternative is using
@ts-ignore
, but that is never a permanent solution.Are you able to assist to bring the feature to reality?
yes, I can
Additional context
I've tried to solve this myself, but I'm not familiar enough to see where the actual changes needs to be made. I am however a TypeScript developer so if anyone could point me to the right file, I'd be happy to work out a solution.
The text was updated successfully, but these errors were encountered: