storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
84.21k stars 9.26k forks source link

CSF: Add typescript typings #7677

Closed shilman closed 4 years ago

shilman commented 5 years ago

Add typescript typings for CSF:

Update monorepo examples:

kroeder commented 5 years ago

I think we already have types for a story function. CC @ndelangen - Didn't we add them in our pair session a couple of weeks ago?

karthikbalajikb commented 5 years ago

Is this types fixed in v5.2 ?

ndelangen commented 5 years ago

@kroeder Yes there are.

There's a bit of a problem, and I do not know how to solve this:

A CSF file doesn't have have a reference to storybook. So even if you'd be able to link the file to the fact the CSF file is exporting StoryFn types, of what sub-type?

A StoryFn is a simple function, it's returntype is different for each viewlayer.

How do we link a CSF to a storybook viewlayer?

shilman commented 5 years ago

@ndelangen why can't the user import the right types from the appropriate package? just because CSF doesn't depend on storybook doesn't mean that "typed-CSF" can't depend on storybook.

ndelangen commented 5 years ago

The user can, they are already available then I guess?

lirbank commented 5 years ago

It would be nice if there was an exported interface for the CSF metadata (default export). Consider the following:

TS in strict mode will error if types for the decorators are not provided Parameter 'storyFn' implicitly has an 'any' type.

export default {
  component: Event,
  title: 'Event',
  decorators: [
    storyFn => <div style={{ backgroundColor: 'yellow' }}>{storyFn()}</div>,
  ],
};

Adding a StoryMetadata interface to handle the typings solves the issue. Being able to import a proper StoryMetadata interface directly from the library would be very nice IMHO. Also fully possible that I'm completely missing something here... :)

import { addDecorator } from '@storybook/react';

type DecoratorFunction = Parameters<typeof addDecorator>[0];

export interface StoryMetadata {
  component: React.ReactNode;
  title: string;
  decorators?: DecoratorFunction[];
}

// Alt 1:
const metadata: StoryMetadata = {
  component: Event,
  title: 'Event',
  decorators: [
    storyFn => <div style={{ backgroundColor: 'yellow' }}>{storyFn()}</div>,
  ],
};

export default metadata;

// Alt 2:
export default {
  component: Event,
  title: 'Event',
  decorators: [
    storyFn => <div style={{ backgroundColor: 'yellow' }}>{storyFn()}</div>,
  ],
} as StoryMetadata;
wKich commented 4 years ago

@lirbank, I think, for decorators you can use DecoratorFn type from @storybook/<framework> or import { DecoratorFunction } from '@storybook/addons'; for framework agnostic variant.

For stories it would be like this:

import { DecoratorFunction } from '@storybook/addons';

interface CSFStory<StoryFnReturnType = unknown> {
  (): StoryFnReturnType;
  story?: {
    name?: string;
    decorators?: DecoratorFunction<StoryFnReturnType>[];
    parameters?: { [name: string]: unknown };
  };
};

export const SimpleButton: CSFStory<JSX.Element> = () => <Button>Hello</Button>;
SimpleButton.story = {
  name: 'simple button',
  decorators: [storyFn => <div style={{ padding: '10px' }}>{storyFn()}</div>],
  parameters: { /* ... */ }
}
dsebastien commented 4 years ago

Is there already a TS type definition that we can use for the default export of a module containing stories? ATM I'm using any and it doesn't feel good :p Is there documentation somewhere around describing all the properties we can return?

shilman commented 4 years ago

Not yet. It's on the radar.

ndelangen commented 4 years ago

@dsebastien or @wKich or @lirbank would you be interested in contributing to https://github.com/storybookjs/csf?

wKich commented 4 years ago

Yes, I would like to try.

ndelangen commented 4 years ago

I think interfaces could be exported here: https://github.com/storybookjs/csf/blob/next/src/index.ts

WDYT @shilman ?

shilman commented 4 years ago

Yeah, that would be great. I'd like to revise what's in there currently and make it better reflect what we're doing with Storybook Args, among other improvements.

dgreene1 commented 4 years ago

Regarding https://github.com/storybookjs/storybook/issues/7677#issuecomment-535651986 I would recommend the first and not the second solution.

Here's why:

import { addDecorator } from '@storybook/react';

type DecoratorFunction = Parameters<typeof addDecorator>[0];

export interface StoryMetadata {
  component: React.ReactNode;
  title: string;
  decorators?: DecoratorFunction[];
}

// Alt 1:
// This approach is better since it forces the object to be constructed the way that the interface requires (unlike the "as" keyword that is used in Alt 2)
const metadata: StoryMetadata = {
  component: Event,
  title: 'Event',
  decorators: [
    storyFn => <div style={{ backgroundColor: 'yellow' }}>{storyFn()}</div>,
  ],
};

export default metadata;

// Alt 2:
export default {
  component: Event,
  title: 'Event',
  decorators: [
    storyFn => <div style={{ backgroundColor: 'yellow' }}>{storyFn()}</div>,
  ],
} as StoryMetadata; //<-- this doesn't force the obj to be a StoryMetadata, it coerces it. Don't do this.
// Use Alt 1 instead.
dgreene1 commented 4 years ago

Btw, I think that the typing for CSF should be carefully considered so that it plays nicely with react-docgen-typescript-loader since that's what everyone is using in TypeScript to generate the docs.

wKich commented 4 years ago

@dgreene1, could you please take a look at proposal of typings and it usage in this PR https://github.com/storybookjs/csf/pull/5?

dgreene1 commented 4 years ago

I think @strothj is a better person to review the typings proposal (PR https://github.com/storybookjs/csf/pull/5) since I am not a contributor to react-docgen-typescript-loader. I just wanted to make sure that the typings were created in such a way that @strothj's library could probably scan for them and therefore expose them to Storybook.

shilman commented 4 years ago

@dgreene1 @wKich currently react-docgen-typescript-loader is run on components, not stories, but I agree it would be good to make sure that the generated files are RDTL compatible (and react-docgen compatible, since we are recommending that in 6.0).

shilman commented 4 years ago

We've just released zero-config typescript support in 6.0-beta. Please upgrade and test it!

Thanks for your help and support getting this stable for release!

lancegliser commented 4 years ago

I don't find clear instructions in any of the documentation provided there about how to properly type my exports. Referencing back to #issuecomment-612927442 the 6x beta doesn't currently export a StoryMetadata yet. Reviewing some recent merge code for Kind and Story types, I don't see a match for my own Vue needs. Both lack a template property.

Is the short term solution to implement interface StoryMetadata and expect an export later?

wKich commented 4 years ago

@lancegliser, you are right. @storybook/csf not released yet. Also framework specific typings for exports would be live in @storybook/<framework> package, so you don't need to import StoryMetadata from csf directly.

ajkl2533 commented 4 years ago

@shilman Any update on StoryMetadata interface being exported somewhere?

shilman commented 4 years ago

I think this is going to end up as a 6.1 feature

zorfling commented 4 years ago

In case anyone else comes here looking, Meta is exported in @storybook/react/types-6-0

import { Meta } from '@storybook/react/types-6-0';

const meta: Meta = {
    title: 'Forms/Text Area',
    component: TextArea,
    argTypes: {
        onUpdateText: { action: 'text change' },
    },
    decorators: [
        Component => (
            <Surface
                style={{ background: '#272731', width: '300px', height: '100px', padding: '10px' }}
            >
                <Component />
            </Surface>
        ),
    ],
};

export default meta;
shilman commented 4 years ago

Thanks @zorfling ! I forgot about this issue, and introduced Story and Meta types for React and Angular types at the tail end of the 6.0 milestone. Closing this issue in favor of #11845 where we'll be bringing them to all frameworks

shilman commented 4 years ago

Also, regarding your example, Storybook source-loader doesn't like the exports from variables, so better might be:

import { Meta } from '@storybook/react/types-6-0';

export default {
    title: 'Forms/Text Area',
    ...
} as Meta;
bard commented 4 years ago

@shilman by doing that, however, you lose type checking. It would allow you to do:

export default {
    trustMe: "I am meta"
} as Meta;

~And the compiler will happily take it.~ Edit: the compiler is more helpful these days, in this particular case it does output the warning: Conversion of type '{ trustMe: string }' to type Meta may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.

However, it is still not equivalent, compare:

export default {} as Meta // all fine

With:

const meta: Meta = {} // Error: Property 'title' is missing in type '{}' but required in type 'BaseMeta<ReactComponent>'
export default meta
shilman commented 4 years ago

@bard Thanks for the clarification.

I saw an issue in the TypeScript project saying that {} as Meta didn't do any type checking (https://github.com/microsoft/TypeScript/issues/13626). Then somebody from the community told me it did, and I assumed that TS had improved since the issue was filed. Looks like it has improved but not enough.

So let me rephrase my comment from above: until we support variable default exports in CSF, my suggestion is a partial (no pun intended) workaround. 😄

fabb commented 4 years ago

Hm, as soon as I import @storybook/react/types-6-0, I get a lot of tsc compile errors all over my project regarding styled-components. probably it overwrites some global types from the styled components types in an incompatible way.

example error:

stories/Grid.stories.tsx:50:17 - error TS2322: Type 'FlattenSimpleInterpolation' is not assignable to type 'InterpolationWithTheme<any>'.
  Type 'readonly any[]' is not assignable to type 'ObjectInterpolation<undefined>'.

50                 css={css`
                   ~~~

  node_modules/@emotion/core/types/index.d.ts:96:7
    96       css?: InterpolationWithTheme<any>
             ~~~
    The expected type comes from property 'css' which is declared here on type 'IntrinsicAttributes & GridProps<"a" | "b" | "d" | "c">'

Interestingly, import '@storybook/addons' or import '@storybook/client-api' in the story file cause the same issue.

As a temporary workaround I avoid those imports, and rather declare my own Story type:

interface Story<Args> {
    (args: Args): ReactElement<unknown>
    args?: Partial<Args>
}
simontaisne commented 3 years ago

@fabb Same problem here. Did you find a way to fix it other than redeclaring a Story type?

fabb commented 3 years ago

Totally forgot about this workaround, seems like I'm still using it.

david-mart commented 2 years ago

import { Meta } from "@storybook/vue3"; works like a charm at the time of writing