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

Fixing issue where stateless functional components created as regular function declarations are missing __docgenInfo #41

Merged
merged 2 commits into from
Jan 14, 2018

Conversation

jsanchez034
Copy link
Contributor

Currently stateless functional components defined as regular function declaration are being missed by the plugin. There is an issue opened for this already #33. This change fixes just the issues with "stateless functional components defined as regular function declaration".

Copy link
Member

@danielduan danielduan left a comment

Choose a reason for hiding this comment

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

everything else looks good, thanks so much for this PR!

src/index.js Outdated
return;
const nodeType = path.node.type;

let className;
Copy link
Member

Choose a reason for hiding this comment

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

can you assign className = path.parentPath.node.id.name; here and move the else statement from below up?

your statements are correct here, but I just want to prevent className being null in case this code gets refactored or changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure will do 👍

@jsanchez034
Copy link
Contributor Author

I actually fixed the conditional up a bit. className can't be set until the node.id check passes, so I just set a local node variable first depending on the node type early before className is set.

@danielduan
Copy link
Member

Thanks. Let me publish an alpha and do some testing in storybook and I'll publish a new version.

@danielduan danielduan merged commit f6b71c7 into storybookjs:master Jan 14, 2018
@danielduan
Copy link
Member

@jsanchez034 published! v1.8.2 :)

@jsanchez034
Copy link
Contributor Author

Awesome thanks @danielduan :)

@jsanchez034
Copy link
Contributor Author

@danielduan do you know the process of getting this into the next release of Storybook? I saw this PR that came from a bot that auto updates dependencies and created a PR in Storybook storybookjs/storybook#2712 but that was about a week ago, before this was published. I'm assuming the bot will pick up the version bump to babel-plugin-react-docgen soon right?

@danielduan
Copy link
Member

@jsanchez034 since we use the caret version, you should be able to delete your existing node modules and any lock files and reinstall to get the new one.

@jsanchez034
Copy link
Contributor Author

Ah I see right. Thanks!

@chemoish
Copy link

@danielduan minor, but should #33 be closed? Also, not sure how CHANGELOG is updated?

(Making some assumptions as it will take me a couple days to test)

@danielduan
Copy link
Member

@chemoish you're right, it should be closed. The changes have been published already.

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