Skip to content
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

Type of args no longer inferred in render #41

Open
IanVS opened this issue Apr 6, 2022 · 6 comments
Open

Type of args no longer inferred in render #41

IanVS opened this issue Apr 6, 2022 · 6 comments

Comments

@IanVS
Copy link

IanVS commented Apr 6, 2022

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 #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 storybookjs/storybook#13747 (comment).

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 IanVS changed the title Type of args no longer inferred Type of args no longer inferred in render Apr 6, 2022
@IanVS
Copy link
Author

IanVS commented Apr 6, 2022

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

@tmeasday
Copy link
Contributor

tmeasday commented Apr 7, 2022

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

@IanVS
Copy link
Author

IanVS commented Apr 7, 2022

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
Copy link
Author

IanVS commented Apr 11, 2022

@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.

@tmeasday
Copy link
Contributor

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
Copy link
Contributor

@IanVS : #43

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants