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

Typescript: Update react-docgen-typescript-plugin #11140

Merged
merged 1 commit into from
Jun 12, 2020
Merged

Typescript: Update react-docgen-typescript-plugin #11140

merged 1 commit into from
Jun 12, 2020

Conversation

hipstersmoothie
Copy link
Contributor

What I did

react-docgen-typescript-plugin loads an dummy plugin when typescript is not installed

How to test

  • Is this testable with Jest or Chromatic screenshots? yes
  • Does this need a new example in the kitchen sink apps? no
  • Does this need an update to the documentation? no

If your answer is yes to any of these, please make sure to include it in your PR.

@hipstersmoothie
Copy link
Contributor Author

hipstersmoothie commented Jun 12, 2020

@mrmckeb This should fix the regression

https://github.com/hipstersmoothie/react-docgen-typescript-plugin/blob/v0.4.0/src/index.ts

@mrmckeb
Copy link
Member

mrmckeb commented Jun 12, 2020

Thanks! I'll get another reviewer to take a look.

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Thanks for the fast turnaround on this @hipstersmoothie!

When you have time can you take a quick look at the Chromatic tests? I think there was maybe a regression here: https://www.chromatic.com/snapshot?appId=5a375b97f4b14f0020b0cda3&id=5ee338c1bc150a0022389b99 The other two changes look like improvements.

I'm going to approve and merge this, but I think fixing that one is probably a follow-up task.

@shilman shilman changed the title update react-docgen-typescript-plugin Typescript: Update react-docgen-typescript-plugin Jun 12, 2020
@shilman shilman merged commit 5b79fd7 into storybookjs:next Jun 12, 2020
@hipstersmoothie
Copy link
Contributor Author

I’ll look at those tomorrow @shilman

@hipstersmoothie
Copy link
Contributor Author

The first one is actually an improvement. That story seems to be demonstrating that you can filter out props from React (html attrs).

For other the two where you think it's better it seems like the plugin is now respecting the shouldExtractLiteralValuesFromEnum option correctly.

@shilman
Copy link
Member

shilman commented Jun 13, 2020

Nice @hipstersmoothie! I didn't actually dig into the example -- assumed missing props = bug, but you're right. Sorry for the false alarm!

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.

3 participants