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

Expose more CSF types in all renderers #19833

Merged
merged 4 commits into from
Nov 16, 2022
Merged

Conversation

kasperpeulen
Copy link
Contributor

@kasperpeulen kasperpeulen commented Nov 14, 2022

Issue:

What I did

  • Expose Args, ArgTypes, Parameters and StrictArgs in all renderers
  • Expose StoryContext parametrized for the specific Renderer
  • Expose Decorator, Loader which are also parametrized for the specific Renderer
  • Deprecated DecoratorFn as I renamed it to Decorator for 7. This allows for making a breaking change to the DecoratorFn type. It now defaults to a more Strict version of Args. Making this strict was necessary so that any doesn't mess up type inference of Meta in the new strict StoryObj.

Lastly, I changed the export of @storybook/types to be the bare Parameters again of @storybook/csf and made a new StorybookParameters and StorybookInternalParameters in @storybook/types.

The reason is that going forward we want users to have autocompletion for Parameters, that is specific to their setup. The easiest way for users to do this, is using module augmentation:

declare module '@storybook/react' {
  interface Parameters
    extends StorybookParameters,
      ChromaticParameters,
      DocsParameters,
      BackgroundParameters {}
}

This only works, if the export of Parameters, is actually a direct reference to the interface defined in @storybook/csf.

I think we should also offer users a way to achieve autocompletion for Parameters without module augmentation. Especially, in the case of libraries, such as addons, using module augmentation should be discouraged, as it can have unintended side effects.

Next to module augmentation, we could also offer users to parametrize Parameters themself as well:

// storybook-types.ts (in user code)
import { StoryObj as StorybookStoryObj } from '@storybook/react';

interface Parameters
   extends StorybookParameters,
     ChromaticParameters,
     DocsParameters,
     BackgroundParameters {}

export type StoryObj<Args> = StorybookStoryObj<Args, Parameters>;
// then user can import this `StoryObj` instead of the one of storybook provides in the renderer

I didn't expose the StorybookParameters, as I want to first check the quality of it, and make sure that is complete.

Also I made an InternalStorybookParameters interface, as I had the feeling that we don't want to expose fileName and docsOnly to the user, as they should not be set by users I believe.

@JReinhold
Copy link
Contributor

... Especially, in the case of libraries, such as addons, using module augmentation should be discouraged, as it can have unintended side effects. - @kasperpeulen

Can you elaborate on this? I though addons would be a perfect usecase for augmenting types based on the user's setup, but maybe I'm misunderstanding you.

@kasperpeulen kasperpeulen marked this pull request as ready for review November 14, 2022 19:30
MIGRATION.md Outdated Show resolved Hide resolved
@kasperpeulen
Copy link
Contributor Author

@JReinhold

Can you elaborate on this? I though addons would be a perfect usecase for augmenting types based on the user's setup, but maybe I'm misunderstanding you.

Yes, I do think that addons are a perfect usecase for module augmentation, but I do think the "actual" module augmentation should happen in app code, and not in the library code of the addon. I think the addon should export types like DocsParameters or BackgroundsParameters which then can be imported into user code:

// for example in [app]/src/types.d.ts
import type { AngularParameters, StorybookParameters } from '@storybook/angular';
import type { ChromaticParameters } from 'chromatic';
import type { DocsParameters } from '@storybook/addon-docs';
import type { BackgroundsParameters } from '@storybook/addon-backgrounds';

declare module '@storybook/angular' {
  interface Parameters
    extends StorybookParameters,
      AngularParameters,
      ChromaticParameters,
      DocsParameters,
      BackgroundsParameters {}
}

MIGRATION.md Outdated Show resolved Hide resolved
MIGRATION.md Outdated Show resolved Hide resolved
MIGRATION.md Show resolved Hide resolved
Copy link
Member

@yannbf yannbf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really good stuff, @kasperpeulen !

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

Successfully merging this pull request may close these issues.

4 participants