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

Extract StorybookArgs type to separate file and use it in other stories too #3761

Merged
merged 1 commit into from
Sep 17, 2023

Conversation

PavelVanecek
Copy link
Collaborator

followup from #3759 (comment)

Adding the type helps to find out what the properties are and what shape is expected.

Storybook website has it documented, and provides typescript types, but for some reason that type is not exported in code.

Description

Add type to storybooks

Related Issue

#3759

Motivation and Context

Helps to write storybooks.

How Has This Been Tested?

lint, ts

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@PavelVanecek PavelVanecek mentioned this pull request Sep 16, 2023
8 tasks
Comment on lines +8 to +10
* This has been reported before: https://github.com/storybookjs/storybook/issues/11916
* If the official types get updated then we can remove this whole file
* and import directly Args from '@storybook/react'
Copy link
Contributor

Choose a reason for hiding this comment

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

Just wondering: Would you like to do a PR in storybook to export those types?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I'll look into that. I am afraid there's some roadblock - why else would the storybook people add the type in the docs but not in the code? But it's worth a try at least.

Comment on lines +3 to +6
/*
* This is the type as defined here: https://storybook.js.org/docs/react/api/arg-types#argtypes
* The actual library exports only `interface Args { [name: string]: any; }`.
* The "any" does not allow your editor to show autocomplete or documentation.
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️ Love this. Thank you.

Copy link
Contributor

@nikolasrieble nikolasrieble left a comment

Choose a reason for hiding this comment

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

Thank you a lot, this is great! Certainly makes all our life easier.

@PavelVanecek
Copy link
Collaborator Author

Thanks for the review @nikolasrieble. I wonder what's the usual workflow? I don't think I have merge permissions myself. Are we waiting for two approvers?

@ckifer
Copy link
Member

ckifer commented Sep 17, 2023

Generally @nikolasrieble or myself will review and merge, sometimes waiting for more reviews depending on the size/implications of a change. Eventually I hope to have more maintainers will merge permissions

Copy link
Member

@ckifer ckifer left a comment

Choose a reason for hiding this comment

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

Very nice, thanks for doing this

@ckifer ckifer merged commit ae675fc into recharts:master Sep 17, 2023
5 checks passed
GMer78 pushed a commit to GMer78/recharts-1 that referenced this pull request Nov 24, 2023
@PavelVanecek PavelVanecek deleted the storybook-args-type branch March 19, 2024 11:35
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

Successfully merging this pull request may close these issues.

None yet

3 participants