csf icon indicating copy to clipboard operation
csf copied to clipboard

Type of args no longer inferred in render

Open IanVS opened this issue 2 years ago • 6 comments

Hi, I found that I am getting a lot of typescript errors in my app after updating to storybook 6.5.0-alpha.50, and I think it's due to https://github.com/ComponentDriven/csf/pull/33 now being included. I'd like to try to understand what I should do to resolve this. cc/ @tmeasday

First, here's how I've been typing my stories. I've taken an approach based on https://github.com/storybookjs/storybook/issues/13747#issuecomment-901117408.

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;

Then, in my story, I use it like so:

import type { CSF3, StoryObj } from '@dn-types/storybook';
import { Label, Input } from '../index';
import { HelpTip } from './HelpTip';

const meta = {
  title: 'Atoms/forms/<HelpTip>',
  component: HelpTip,
};
export default meta;

type Story = CSF3<typeof meta>;

export const WithLabelAndInput: Story = {
  render(args) {
    return (
      <div className="mt-32">
        <Label htmlFor="foo" className="mb-8">
          This is a label
          <HelpTip {...args} />
        </Label>
        <Input id="foo" />
      </div>
    );
  },
  args: {
    children: 'This is helpful text.',
  },
};

That used to work pretty well. The type of args in the story was passed to render, but now those args are just Args which is basically just "an object with anything in it". So <HelpTip {...args} /> fails with Property 'children' is missing in type '{}' but required in type 'Props'.

Is there some other way I should be typing my stories? I've never been completely clear how to get good type coverage in stories, and the approach above was the closest I could get to a sane method, which will error if I do not provide all the needed props either through the default meta object, or the story itself.

IanVS avatar Apr 06 '22 14:04 IanVS

Note, I tried manually typing the args argument in render (even though it's ugly and I'd prefer not to), but that doesn't work either:

render(args: React.ComponentProps<typeof HelpTip>) {
image

IanVS avatar Apr 06 '22 14:04 IanVS

Hmm, this is an interesting problem. The issue we were trying to solve there was a problem with decorators where often times a decorator wasn't going to be specific to the args of the story it's being applied to. I'm still not totally clear on why that was a problem, perhaps that can be fixed another way.

Perhaps we shouldn't have applied the same logic to the render function? I guess it's tricky if folks aren't using the same args type composition pattern, then it might be unhelpful to have such a specific type on the render function (as it won't include the component's args type).

Is a workaround @IanVS to simply define the type of the render function yourself in your CSF3 type constructor:

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> & { render: ... }
// etc

tmeasday avatar Apr 07 '22 01:04 tmeasday

if folks aren't using the same args type composition pattern, then it might be unhelpful to have such a specific type on the render function (as it won't include the component's args type

No matter what strategy they're using, now they won't be able to add any kind of information about the args on the render() function, right? Since it's always just Args, and trying to overwrite it with explicit types fails too.

Come to think of it, I believe my type-inferring strategy is a bit of a red-herring here, and that this will cause problems for anyone who needs good typing of their render() function.

I tried your suggestion, but I still get errors. Here's what I tried:

M extends { component: React.ComponentType<infer Props> }
  ? { args: Props } & Omit<StoryObj<Props>, 'render'> & { render?: ArgsStoryFn<ReactFramework, Props> }
  : never;

But, even then, the strict definition in the library types blocks me.

Type '{ args: Props; } & Omit<StoryObj<Props>, "render"> & { render?: ArgsStoryFn<ReactFramework, Props> | undefined; }' is not assignable to type 'BaseAnnotations<ReactFramework, Props>'.
    Types of property 'render' are incompatible.
      Type 'ArgsStoryFn<ReactFramework, Props> | undefined' is not assignable to type 'ArgsStoryFn<ReactFramework, Args> | undefined'.
        Type 'ArgsStoryFn<ReactFramework, Props>' is not assignable to type 'ArgsStoryFn<ReactFramework, Args>'.
          Types of parameters 'args' and 'args' are incompatible.
            Property 'role' is missing in type 'Args' but required in type 'Props'.

IanVS avatar Apr 07 '22 03:04 IanVS

@tmeasday did you hear any reports of problems with the typing of the render function previously? Perhaps it might be best to reset it back to the way it was, at least for now until maybe a better approach can be found?

as it won't include the component's args type

Could you explain this one a bit? I think that it's reasonable to expect that the args type defined for the story (TArgs) is passed to render, right? That's how it was before, but it's stopped working now.

IanVS avatar Apr 11 '22 02:04 IanVS

Perhaps it might be best to reset it back to the way it was, at least for now until maybe a better approach can be found?

Yeah, fair enough, let's do this.

Could you explain this one a bit?

I guess what I am thinking is if you have:

const meta = {
  args: { a: 'a' },
};
export default meta;

type Story = CSF3<typeof meta>;

export AStory : Story = {
  args: { b: 'b' },
  render: (args) => {}
};

Then the type of args in AStory will be { a: string, b: string }. But if you have:

const meta = {
  args: { a: 'a' },
};
export default meta;

export AStory : StoryObj = {
  args: { b: 'b' },
  render: (args) => {}
};

(The more "traditional" way to do it), then the type of args will be : { a: string }, which would be overly restrictive.

In the current way of doing it, the type of args is Args, which in theory is an object with any keys. I guess depending on your TS setup, that may or may not be an issue if you try and access args.b. It looks like in yours, Args is not OK. But for others, I am guessing it will be alright to do that for a generic type like Args?

tmeasday avatar Apr 12 '22 05:04 tmeasday

@IanVS : https://github.com/ComponentDriven/csf/pull/43

tmeasday avatar Apr 12 '22 07:04 tmeasday