-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
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
Why CSF Args Type is Partial type ? #13747
Comments
I have the same problem with export type TIdentify = { className: string; id: string };
export interface IButtonProps {
color: string;
label: string;
identity: TIdentify;
}
// Button.stories.tsx
import { Story, Meta } from "@storybook/react/types-6-0";
import Button, { IButtonProps, TIdentify } from "./Button";
export default {
title: "Test",
component: Button,
} as Meta;
const Template: Story<IButtonProps> = (args) => <Button {...args} />;
export const Primary = Template.bind({});
Primary.args = {
color: "blue",
label: "Primary",
identity: {
className: "first",
id: "first",
},
};
export const PrimaryLongName = Template.bind({});
PrimaryLongName.args = {
identity: {
...(Primary.args.identity as TIdentify),
id: "second",
},
};
# |
I would also love to see this addressed. It seems that casting the arg to component prop type is the only solution for now, although verbose. |
Also bumping into this issue when trying to use |
This is what I do, and yes, this is unnecessarily verbose, check basic example below. import React from 'react';
import { Story, Meta } from '@storybook/react';
import Component, { ComponentProps } from './Component';
export default {
title: 'atoms/Component',
component: Component,
} as Meta;
// Using an intermediate type is not required but sometimes useful.
type TemplateProps = Omit<ComponentProps, 'children'>;
const Template: Story<TemplateProps> = (props) => (
<Component {...props}>Hello World</Component>
);
export const Default = Template.bind({});
// Using an intermediate variable to strictly type args.
const defaultArgs: TemplateProps = {
propA: true,
propB: 'x',
};
Default.args = defaultArgs; |
Apologies for the lack of response here. This has been discussed extensively on another issue (or maybe a PR) but after 10 minutes of searching I wasn't able to find it. (cc @yannbf @tmeasday who both weighed in before). The reason that Args/ArgTypes are Partials by default is because of the way Storybook composes things hierarchically. Consider the following component: // MyComponent.tsx;
export interface MyProps {
a: string;
b: string;
}
export const MyComponent = (props: MyProps) => JSON.stringify(props); And stories: // MyComponent.stories.tsx
import { Meta, Story } from '@storybook/react';
import { MyProps, MyComponent } from './MyComponent';
export default {
args: { a: 'hello' };
} as Meta<MyProps>
export const MyStory: Story<MyProps> = (args) => <MyComponent {...args} />;
MyStory.args = {
b: 'goodbye';
}; Since Storybook composes args hierarchically, the story function's argument is a fully specified I appreciate that the current type system is deficient, so I'd love to come up with a resolution for this. There were a few proposals to address this last time around:
Alternatively perhaps there is some Typescript magic that can help the situation? Or we need to rethink Storybook's composition, which I don't see happening but am open to proposals on. What do you think? |
I understood. Thanks. Please give me a link if you have the appropriate documentation. |
One or the other would already work for me 😊 No preference which one from my side.
I wasn't really aware of that. I'd say I'm a vivid Storybook user, but still it's hard for me to keep track of things like that. Official Storybook ESLint rules or a Storybook language server might help here in the future. Depending on whether you use |
I have playing around with the following helper which makes sure that the relationship between meta and stories is understood by typescript: const createMeta = <P, D extends Partial<P>>(meta: { component: ComponentType<P>; args: D } & Meta<P>) =>
[meta, (story: { args: Omit<P, keyof D> } & Story<P>) => story] as const; So say we have this component: export const MyComponent = (props: { a: string; b: string }) => <>{JSON.stringify(props)}</>; We can create meta and a story creator using // Infers P (props) from MyComponent, and D (default props) from args
const [meta, createStory] = createMeta({ component: MyComponent, args: { a: "hello" } });
export default meta; If we wrap our story exports with createStory everything gets inferred now: export const MyStory = createStory({ args: { b: "goodbye" } }); // Fine as "a" has a default value
export const MyStory2 = createStory({ args: { a: "hello hello" } }); // Error: Property 'b' is missing in type '{ a: string; }'
export const MyStory3 = createStory({ args: { a: "hello hello", b: "goodbye" } }); // Fine you can overwrite defaults This is using the new CSF 3 format, using Note that, this is not doing any imperative magic. It is just exporting plain javascript objects, but the createStory makes sure it understand which args are required and which not. It will export the same javascript object as the following: export default { component: MyComponent, args: { a: "hello" } } as ComponentMeta<typeof MyComponent>;
export const MyStory: ComponentStory<typeof MyComponent> = { args: { b: "goodbye" } };
export const MyStory2: ComponentStory<typeof MyComponent> = { args: { a: "hello hello" } };
export const MyStory3: ComponentStory<typeof MyComponent> = { args: { a: "hello hello", b: "hello hello" } }; A lot more type annotations, but impossible for typescript to find the error in this way. |
Discussed another approach with @shilman and @yannbf that doesn't use factories so that we have predictable AST for tools to statically analyze: The helper: import type { Meta, Story } from "@storybook/react";
import type { ComponentType } from "react";
export type CSFType<M extends Meta> = M extends {
component: ComponentType<infer P>;
args: infer D;
}
? { args: Omit<P, keyof D> } & Story<P>
: never; The component: export const MyComponent = (props: { a: string; b: string }) => <>{JSON.stringify(props)}</>; The story: import type { CSFType } from "./helper";
import { MyComponent } from "./my-component";
const meta = { component: MyComponent, args: { a: "a" } };
type Story = CSFType<typeof meta>;
export default meta;
export const MyStory1: Story = { args: { b: "goodbye" } }; // Fine, a has default
export const MyStory2: Story = { args: { a: "hello hello" } }; // Error: Property 'b' is missing in type '{ a: string; }'
export const MyStory3: Story = { args: { a: "hello hello", b: "goodbye" } }; // Fine, can overwrite a |
Love this approach @kasperpeulen ❤️ Let's make it happen! Is this a breaking change or can it be added alongside the current types? I think this would be a fantastic addition to CSF3 🎁 |
I've just upgraded to 6.4 beta and using We use Chakra UI components and unfortunately there's none for the /* Component.tsx */
import { FC } from 'react';
import { chakra, SelectProps } from '@chakra-ui/react';
export interface CbSelectProps extends SelectProps {
options: SelectOption[];
label?: string;
}
const CbSelectBase: FC<CbSelectProps> = ({ options, label, id, ...props }) => ( ... )
export const CbSelect = chakra(CbSelectBase); /* Component.stories.tsx */
export const SelectDefault: ComponentStoryObj<typeof CbSelect> = {
args: {
// no autocompletion here
}, It was working in a former alpha version |
@pribilinskiy what does |
Thx @tmeasday for chiming in As I see it combines |
@pribilinskiy could you put a reproduction of the issue together? Which alpha was it working in? |
Not sure, solved by using the |
There is one problem with my solution: const meta = { component: MyComponent, args: { a: "a" } };
type Story = CSFType<typeof meta>;
export default meta;
export const MyStory1: Story = { args: { b: "goodbye" } }; // Fine, a has default You won't get autocompletion for meta. You will get autocompletion if you add a type annotation, but this will remove all the extra type safety that the CSFType provided. const meta: Meta = { component: MyComponent, args: { a: "a" } }; So basically, in typescript at this moment, if you don't use factories, you will often either loose auto completion, or the advanced type safety, type correctness that typescript can provide, and you can't have both. However, there is a PR that tries to improve this status quo, which provides a new "satisfies" operator: I reproduced my example with this new operator (inlined all external types, as I can not import in the typescript playground), and it does gives us both auto completion and type safety: type ComponentType<P> = (props: P) => any;
interface Meta<P = any> {
component: ComponentType<P>,
args: Partial<P>
}
type CSFType<M extends Meta<any>> = M extends {
component: ComponentType<infer P>;
args: infer D;
} ? { args: Omit<P, keyof D> } & {args: Partial<P>}
: never;
type Props = { a: string; b: string };
export const MyComponent = (props: Props) => props;
const meta = {
component: MyComponent,
args: {a: "default" /*autocompletion possible */ }
} satisfies Meta<Props>; // the new type operator
type MyStory = CSFType<typeof meta>;
export const MyStory1: MyStory = { args: { b: "goodbye" } }; // Fine, a has default
export const MyStory2: MyStory = { args: { a: "hello hello" } }; // Error: Property 'b' is missing in type '{ a: string; }'
export const MyStory3: MyStory = { args: { a: "hello hello", b: "goodbye" } }; // Fine, can overwrite a |
@kasperpeulen looks like |
Doh, looks like it's been pushed to 4.7 at the earliest. babel/babel#14211 (comment) 🤞 Edit: I see some confusion emoji. I mean that |
I found that with import type { Meta, StoryObj } from '@storybook/react';
export type CSF3<M extends Meta> = M extends {
component: React.ComponentType<infer Props>;
args: infer D;
}
? // Make the args which weren't in meta required
{ args: Omit<Props, keyof D> } & StoryObj<Props>
: // If there are no args in the meta, make sure all props are specified in story
M extends {
component: React.ComponentType<infer Props>;
}
? { args: Props } & StoryObj<Props>
: never; |
Thanks for the improvement :) I played around a bit more with it, I think we also need something for this variant, when all props are optional. declare const Component: (props: { a?: string; b?: string; }) => JSX.Element;
const meta = { component: Component1 };
// args should be optional now
export const Story: CSF3<typeof meta1> = {}; And it should also not require action props (event handlers) to be passed in the args, as a default value is passed by storybook itself, to be logged in the action panel. I made this new evolution on the idea: export type CSF3<M extends Meta, ActionKey extends string = `on${string}`> = M extends {
component: ComponentType<infer Props>;
args?: infer D;
}
? // If the default args D in meta implement Props, we can use the old non-strict StoryObj
D extends Omit<Props, ActionKey>
? StoryObj<Props>
: // If all props are optional (or action args), we can also use the old non-strict StoryObj
{} extends Omit<Props, ActionKey>
? StoryObj<Props>
: // Otherwise, use the strict args, but omit the defaults keys provided in Meta and action keys.
{ args: Omit<Props, keyof D | ActionKey> } & StoryObj<Props>
: never; And added some tests, because it is getting quite hairy 😅 : // If all props are optional (or actions), it should be okay to provide no args in both meta and the story.
declare const Component1: (props: { a?: string; b?: string; onLogin(): void }) => JSX.Element;
const meta1 = { component: Component1 };
export const Story1a: CSF3<typeof meta1> = {};
export const Story1b: CSF3<typeof meta1> = {};
// If there is a non-optional prop, it can be provided all in meta.
declare const Component2: (props: { a: string; b: string; onLogin(): void }) => JSX.Element;
const meta2 = { component: Component2, args: { a: '', b: '' } };
export const Story2a: CSF3<typeof meta2> = {};
export const Story2b: CSF3<typeof meta2> = { args: {} };
// If there is a non-optional prop, it can be provided partially in meta, and partially in the story.
const meta3 = { component: Component2, args: { a: 'default' } };
export const Story3a: CSF3<typeof meta3> = { args: { b: '' } };
// @ts-expect-error
export const Story3b: CSF3<typeof meta3> = {};
// @ts-expect-error
export const Story3c: CSF3<typeof meta3> = { args: {} };
// @ts-expect-error
export const Story3e: CSF3<typeof meta3> = { args: { b: 1 } };
// If there is a non-optional prop, it can be provided all in the story
const meta4 = { component: Component2 };
export const Story4a: CSF3<typeof meta4> = { args: { a: '', b: '' } };
// @ts-expect-error
export const Story4b: CSF3<typeof meta4> = { args: { a: '' } }; Sadly, I don't see much activity on the satisfies operator lately. I asked here for an update: |
Rewinding back to @shilman's great explanation:
In every one of our Storybook files, we define our export const Basic: Story = {};
export const WithoutImage: Story = {
args: {
image: null,
},
}; So in our case, a strict meta type + a partial story type is what we'd be looking for, since we always define everything in the meta, and then override it field by field in the stories. (Of course, then I can't help but notice that if Storybook only shipped "strict" types, all I'd need to do is spread What we've done for now is this: export interface ComponentMeta<
T extends
| keyof JSX.IntrinsicElements
| JSXElementConstructor<ComponentProps<T>>
> extends Meta<ComponentProps<T>> {
// Require complete component props
args: ComponentProps<T>;
} Which allows for this pattern, even if const args = {
title: "Item Title",
} as const;
const meta: ComponentMeta<typeof ItemCard> = {
component: ItemCard,
args,
};
export const WithLongTitle: Story = {
args: {
title: args.title.repeat(5),
},
}; And this spread pattern without needing a type assertion: const args = {
children: [...Array(20).keys()].map((n) => (
<ItemCard {...itemCardMeta.args} key={n} title={`Item ${n + 1}`} />
)),
} as const; |
It looks like |
I ended up writing a util function to help with this: /** Remove the Partial<> constraint from TArgs */
export type StoryRequiredArgs<TArgs> = Story<TArgs> & { args: TArgs };
export function templateBindRequireAllArgs<TArgs>(
template: Story<TArgs>,
): StoryRequiredArgs<TArgs> {
return template.bind({}) as StoryRequiredArgs<TArgs>;
}
...
const Template: Story<SomeProps> = (args) => <Some {...args}/>
// Use the util instead of Template.bind({})
export const MyStory = templateBindRequireAllArgs(Template)
MyStory.args = { ... } // all props should be required here now |
Yippee!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.0.0-alpha.37 containing PR #19238 that references this issue. Upgrade today to the
Closing this issue. Please re-open if you think there's still more to do. |
Not sure if it's still relevant, I suffered the same problem and solved this way:
|
@cesarriva In storybook 7 alpha you can write for a component like this: export const MyComponent = (props: { a: string; b: string }) => <>{JSON.stringify(props)}</>; import type { Meta, StoryObj } from '@storybook/react';
const meta = {
component: MyComponent,
args: { a: 'a' },
} satisfies Meta<typeof MyComponent>;
export default meta;
type Story = StoryObj<typeof meta>;
export const MyStory1: Story = { args: { b: 'b' } }; // Fine, a has default
export const MyStory2: Story = { args: { a: 'a' } }; // Error: Property 'b' is missing in type '{ a: string; }'
export const MyStory3: Story = { args: { a: 'a', b: 'b' } }; // Fine, can overwrite a |
Is there any chance these new types could be backported to |
import type { Meta, StoryObj } from '@storybook/react';
import type { ComponentMeta, ComponentStoryObj } from '@storybook/react';
import type { ComponentMeta, ComponentStoryObj } from '@storybook/react';
const meta = {
component: MyComponent,
args: { a: 'a' },
} satisfies ComponentMeta<typeof MyComponent>;
export default meta;
type Story = ComponentStoryObj<typeof meta>;
export const MyStory1: Story = { args: { b: 'b' } }; // Fine, a has default
export const MyStory2: Story = { args: { a: 'a' } }; // Error: Property 'b' is missing in type '{ a: string; }'
export const MyStory3: Story = { args: { a: 'a', b: 'b' } }; // Fine, can overwrite a This does not work. See #13747 (comment) |
@andykenward This doesn't work with the latest This appears to be because the (awesome!) new types from #19238 haven't been backported to types-7-0.ts on the v6.5.x release line. |
@kasperpeulen The |
@gynekolog We noticed that issue indeed as well. We are working on a fix! @aaronadamsCA You are right. We could backport it indeed, leaving that decision for @shilman. Note that the 7.0 types are backward compatible with 6.x, so you can migrate them to This is also valid in 7 (though you will get deprecations warnings in your editor): export default {
component: MyComponent,
args: { a: 'a' },
} as ComponentMeta<typeof MyComponent>; // or Meta<typeof MyComponent>;
type Story = ComponentStoryObj<typeof MyComponent>; // or StoryObj<typeof MyComponent>
export const MyStory1: Story = { args: { b: 'b' } }; // Fine, a has default
export const MyStory2: Story = { args: { a: 'a' } }; // Doesn't error with the 6.x syntax
export const MyStory3: Story = { args: { a: 'a', b: 'b' } }; // Fine, can overwrite a |
@aaronadamsCA unfortunately we don't patch things back unless it's a severe bug AND a non-disruptive change. I feel like this is neither. Fortunately, 7.0 is going to beta soon and hopefully will be stable enough to upgrade to before too long! |
@shilman Makes sense! Just note this means Storybook latest ships "v7 types" that aren't. I do think that's fine, though, since the new types are significantly more complex. Can't wait to try v7! We'll make the jump as soon as Babel configs are fixed up. |
Oh sorry I didn't read that far back in the thread (was on my phone) so I totally misunderstood the ask. We could potentially do something here. Will discuss with @kasperpeulen offline |
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
Describe the bug
I use storybook for react on TypeScript and create stories using CSF format.
like this:
Component
Story
I do not know why component arg type is not strict.
If I make a mistake in using it, please let me know.
To Reproduce
https://codesandbox.io/s/sharp-raman-2rbk6?file=/src/App.stories.tsx
Expected behavior
The props types error is showed like this:
System
Additional context
If here type
args?: Partial<Args>;
becomeargs?: Args;
, It works as expected.https://github.com/storybookjs/storybook/blob/next/lib/addons/src/types.ts#L171
The text was updated successfully, but these errors were encountered: