-
Notifications
You must be signed in to change notification settings - Fork 106
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
Explore adding react-docgen #167
Conversation
"es-module-lexer": "^0.9.3", | ||
"glob": "^7.2.0", | ||
"glob-promise": "^4.2.0", | ||
"react-docgen": "6.0.0-alpha.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using 6.0.0-alpha.0
so it supports resolving of imports. My hope is that this means we don't have to implement react-docgen-typescript
although I have a working version of that too: https://github.com/joshwooding/storybook-builder-vite/tree/explore-react-docgen-typescript. Since my aim is for this to be temporary I'm trying to cover most of the functionality in one plugin.
@@ -0,0 +1,52 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copied from https://github.com/storybookjs/babel-plugin-react-docgen/blob/master/src/actualNameHandler.js but the imports were converted and dead code at the end of the function was removed.
da1e90a
to
480899f
Compare
// Escape windows paths | ||
const filePath = path.relative(process.cwd(), id).replace(/\\/g, '\\\\'); | ||
docGenExtra += ` | ||
${actualName}.__docgenInfo = ${JSON.stringify(docGenResult, null, 2)}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I basically reverse engineered this from the babel injection code (https://github.com/storybookjs/babel-plugin-react-docgen/blob/2df98996a51d7f537c29b63b93cc6b7687e2baaa/src/index.js#L102)
480899f
to
6cbc544
Compare
There are now two solutions, one which is relying on the babel plugin (6cbc544) and one which emulates it (480899f). Relying on the babel plugin is definitely cleaner but we can't make it easy to use Currently I've added the babel plugin as a dependency but it's always included in |
@joshwooding what's your sense for how important it is to use
Seems kind of important, right? I think it could be okay to use your re-implementation for the time being, perhaps with a note to swap it out once (if) there's ever a stable release of react-docgen@6 (tracking issue: reactjs/react-docgen#502). Alternatively, like you said we could make an alpha version of babel-plugin-react-docgen, it looks like a similar approach has been taken in the past: storybookjs/babel-plugin-react-docgen#72 |
I talked to @shilman and others about this, and they'd be willing to consider updating babel-plugin-react-docgen to use the alpha version of react-docgen if it solves the type import issue. There's even a good chance that it will end up being faster than the typescript version that's in use right now. We should explore it and see if it does indeed work for typescript projects. |
@IanVS Cool, I was looking at this today. I think releasing an alpha of babel-plugin-react-docgen is the best option. |
I published |
I know there's some work going on in storybook core (#2 (comment)) as @shilman mentioned, but I've managed to get react-docgen working (I haven't really tested it's performance) but it might be worth exploring adding this temporarily so the feature works?