-
Notifications
You must be signed in to change notification settings - Fork 47
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
Add unmappedArgs
to story context
#68
Conversation
@kasperpeulen maybe I have this backwards and the type of It's sort of a problem though, because (as decorators can change args), the type of |
* The args that are passed on the context are mapped by arg mapping | ||
* and can be filtered by conditional arg filters. | ||
*/ | ||
args: Args; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we kind of want to override the args from TArgs to Args here right?
Because they are defined as TArgs in StoryContextUpdate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess they should be Args
in StoryContextUpdate
too, after all.
Will unmappedArgs be the one that the user defined in the story? Or the one that merges the args of preview/meta and the story? I think it is not breaking, because we weaken the type in some sense, as it becomes an object with values typed as any. I kind of feel like keeping it export type StoryObj<TMetaOrCmpOrArgs = Args> = TMetaOrCmpOrArgs extends {
render?: ArgsStoryFn<ReactRenderer, any>;
component?: infer Component;
args?: infer DefaultArgs;
}
? Simplify<
(Component extends ComponentType<any> ? ComponentProps<Component> : unknown) &
ArgsFromMeta<ReactRenderer, TMetaOrCmpOrArgs>
> extends infer TArgs
? StoryAnnotations<
ReactRenderer,
TArgs,
SetOptional<TArgs, keyof TArgs & keyof (DefaultArgs & ActionArgs<TArgs>)>
>
: never
: TMetaOrCmpOrArgs extends ComponentType<any>
? StoryAnnotations<ReactRenderer, ComponentProps<TMetaOrCmpOrArgs>>
: StoryAnnotations<ReactRenderer, TMetaOrCmpOrArgs>;
type ActionArgs<TArgs> = {
// This can be read as: filter TArgs on functions where we can assign a void function to that function.
// The docs addon argsEnhancers can only safely provide a default value for void functions.
// Other kind of required functions should be provided by the user.
[P in keyof TArgs as TArgs[P] extends (...args: any[]) => any
? ((...args: any[]) => void) extends TArgs[P]
? P
: never
: never]: TArgs[P];
}; We could maybe go further and make sure that context action args are But yes, we can not take into account user arg mappings and decorator modifications. const Component = (props: { label: string; setInDecorator: string }) => <></>;
const withDecorator: Decorator = (Story, { args }) => (
<Story args={{ ...args, setInDecorator: 'adsf' }} />
);
const meta = { component: Component } satisfies Meta<Props>;
const Basic: StoryObj<typeof meta> = {
args: { label: 'label', setInDecorator: null! /* trust me, we will set in decorator */ },
decorators: [withDecorator],
}; |
Let's have a chat about it. I think we should just get our story (ha!) straight--what are Either way is there a principled way the user can tell us what the relation is between the two? Maybe something like: type Mapper<T> = T & { setInDecorator: boolean };
const meta = Meta<Component, Mapper>; |
Closing this for now, will revisit in storybookjs/storybook#22228 |
NOTE: this changes the type of
context.args
to reflect that by this point args have been mapped and actually we no longer know what type they have in detail.I'm not sure if this is a breaking change or if it is something that will mess people up @kasperpeulen?
It is strictly more correct because:
I'd be happy to change it back to
args: TArgs
if we are worried about that. Keeping in mind that that type is not actually correct :).For storybookjs/storybook#22135