Closed tmeasday closed 1 year ago
@kasperpeulen maybe I have this backwards and the type of args
that finally makes its way into the render
function should be TArgs
and the args that go over the channel (unmappedArgs
) is the one we don't know the type of?
It's sort of a problem though, because (as decorators can change args), the type of context.args
can change between each decorator + the final render function.
I'm not sure if this is a breaking change or if it is something that will mess people up @kasperpeulen?
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.
But people will loose autocompletion/type safety when using args in decorators, play function, loaders.
We can also type it as StrictArgs
, which has unknown
values, but then the user have to do runtime type checks, before they can use the values of the args in those places.
I kind of feel like keeping it TArgs
, and accepting some incorrectness here indeed, and keep investigating how we can make this more sound. For example, we already do some type mapping, between the user provided args and the context args over here, so that the user doesn't have to provide action args:
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 Jest.Mock
as that is one arg mapping we do right? Are action everywhere a Jest.Mock type or only in the play function?
But yes, we can not take into account user arg mappings and decorator modifications. I kind of envisioned that if you change the value in a decorator, the user might not to silence the compiler in some places:
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 TArgs
? Are the they inputs of the component (inferrable in many frameworks), or the (composed) set of "unmapped" (+ undecorated) args that are defined in CSF and go over the channel?
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 https://github.com/storybookjs/storybook/issues/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 https://github.com/storybookjs/storybook/pull/22135