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

Experiment with react-docgen #190

Closed
wants to merge 4 commits into from
Closed

Experiment with react-docgen #190

wants to merge 4 commits into from

Conversation

IanVS
Copy link
Member

@IanVS IanVS commented Dec 29, 2021

This is a similar experiment to #167, but using @ianvs/babel-plugin-react-docgen@5.0.0-alpha.0 so we can try out the 6.0.0-alpha.0 version of react-docgen.

A few things I notice, which @ewlsh also pointed out in #2 (comment):

  • Union types are not handled correctly, and are shown as union
  • Use of React.FC is not handled correctly
  • Imported types now work, but they are shown with their typename, rather than expanding out to the actual value. I'm not sure what react-codegen-typescript does, but having the actual type seems nicer than the type alias name.
  • Function signatures are shown as signature, which doesn't really help much.

image

Despite these limitations, it might be worth thinking about moving forward in this direction to at least get some kind of support, though. Ideally, we'd give an option to also use something like react-codegen-typescript, but this seems like maybe a good start.

@ewlsh
Copy link

ewlsh commented Dec 29, 2021

I added some context on our Vite plugin work to #2, I'd be curious if custom handlers receive a TypeScript AST in this implementation.

@joshwooding
Copy link
Member

I think my thoughts were to provide at least some support for now.
We might want to add some custom handlers, that we can push upstream so everyone benefits but I don't think we should necessarily worry about making sure everything is fully supported before we release.

It's definitely possible to get react-docgen-typescript working and we might want to support it after this basic support. I know that I'll need it for my storybooks

@IanVS
Copy link
Member Author

IanVS commented Feb 17, 2022

We now have support for react-docgen-typescript thanks to @joshwooding. I think it's maybe worth also supporting this option, for those not using typescript. If there's interest, I'll clean this up and get it ready for review/merge.

@reintroducing
Copy link

@IanVS non-TS user here weighing in from the peanut gallery. I definitely think this should be supported for those that choose not to use TS, I think that's a pretty common use case and all these tools support both TS and non-TS projects in them.

@IanVS
Copy link
Member Author

IanVS commented Mar 30, 2022

Thanks @reintroducing. I'll work on this a bit. We may find a better way in the future, but it would be good to support folks for now, and this is pretty close to being ready I think.

@sebastienbarre
Copy link

sebastienbarre commented Mar 31, 2022

Thanks @IanVS for your work -- definitely a feature I'm missing as well after switching from Webpack to the Vite plugin. This would be worth mentioning in the "Known Issues" section in the README.

@IanVS
Copy link
Member Author

IanVS commented Apr 1, 2022

@reintroducing and @sebastienbarre thanks for the nudges. I've opened #299 which I think is a better approach to add support for react-docgen, if you'd like to keep an eye on that one.

@reintroducing
Copy link

@IanVS thank you! i've gone ahead and subscribed to that PR, appreciate your work on this.

@IanVS IanVS closed this in #299 Apr 3, 2022
IanVS added a commit that referenced this pull request Apr 3, 2022
Fixes #103
Closes #190
Fixes #2

This takes a slightly different approach from #190.  Instead of using a babel transform, this creates a vite plugin, similar to the vue-docgen plugin, but based on the babel-plugin-react-docgen.  This gives us the chance to rely directly on react-docgen, without needing another package in between, and it avoids being accidentally overwritten if users replace our @vitejs/plugin-react with their own config.

This new vite plugin is not configurable like the babel plugin is, but I'm not certain those configurations are necessary in the first place.  If we hear from users that it's something they need, we can find a way to make them customizable, but I prefer to avoid extra options if possible.

This version does not create a global list of all the docs, like the babel plugin does (`DOC_GEN_COLLECTION_NAME`).  I looked through the storybook source, and it doesn't seem like anything still relies on it, so I think it's just a legacy detail for now that we can probably avoid supporting.

I've updated the react example to demonstrate the use of the plugin, so feel free to check this out there.
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.

5 participants