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

Current and future state of react-docgen #115

Closed
fkling opened this issue Aug 11, 2016 · 9 comments
Closed

Current and future state of react-docgen #115

fkling opened this issue Aug 11, 2016 · 9 comments

Comments

@fkling
Copy link
Member

fkling commented Aug 11, 2016

As you may have noticed, I haven’t found the time to keep up with issues and pull requests in the recent months. In this issue I would like to talk about ideas how to handle reac-docgen’s development so that everybody gets the most value out of it.

Features, features, features

Since the initial release of react-docgen, quite a few features have been added:

  • Support for class syntax
  • Support for stateless functions
  • Extracting information about class methods (+ extracting information with jsdoc)
  • Support for flow types

and there are currently outstanding ideas such as

Most of these have been added by external contributors, which is amazing, and I can’t thank you enough for that. I especially wanted to thank @danez, @janicduplessis and @iamdustan for their work.

However, adding these big features also means that the complexity of the core app increases. And since I’m not that familiar with these parts, it’s also more difficult for me to keep with bug reports for them. @danez has created pull requests to improve the flow type logic, but this is not a sustainable model if this still depends on me processing these PRs.

I see two actions we can take to make this better

  • Add more maintainers
  • Keep the core small / simple

Add more maintainers

This is an easy one: The more people can review issues and PRs, the easier it is to keep things up to date. @wbinnssmith recently joined our team at Facebook and expressed interest in helping out. And of course it only seems reasonable to add @danez, since flow type support is a really big feature.

Keep the core small / simple

From the very beginning react-docgen was designed to be extensible: Handlers contain the actual information extraction logic and it’s possible to write and use your own handlers.

The obvious advantage of this is that it allows developers to customize reac-docgen to their needs. Similar to how coding conventions are subjective and differ from company to company, documentation conventions also differ.

So far I have added new features to the core, but as I said, I don’t think this is sustainable. My suggestion is to keep new features out of the core. If someone wants to make a new feature available to others, they can publish their own npm package (maybe prefixed with react-docgen-…). Of course we would still accept PRs to improve the utility methods to support new features. The other advantage of this is that I / we are not the bottleneck in maintaining these features.

That’s why I think the new suggestions, #93 and #98, should be published as plugins. Maybe we will also move some features that currently are in core outside of (like method documentation).

Overall this would be a similar approach like eslint, except we would still support the basics out of the box. However, I’m also happy to talk about pros and cons of having a monorepo instead, like Babel.

To make plugin creation and usage easier, I think we need to

  • Provide a template for handler unit tests.
  • Make it easy to add handlers from the command line.

That’s all for now, please let me know what you think!

@janicduplessis
Copy link
Contributor

janicduplessis commented Aug 12, 2016

@fkling Good writeup!

If you want to go the keep it small route and extract out component method docs into a separate package I can take care of it. I'd also like to help out but I don't have much time at the moment :(.

@ZauberNerd
Copy link
Contributor

I just published a custom handler which we could use as a testbed: react-docgen-imports-handler
A couple of things I noticed while publishing this:

  • The Documentation is not exported. It is possible to import it via import Documentation from 'react-docgen/dist/Documentation'; but that would mean that the dist/ part becomes part of the public API.
    This is being used in the test file for the handler
  • The babylon wrapper is not exported and also has to be accessed via dist/ or packages need to implement it themselves. For the initial version I went with the first approach but I'm not sure yet which approach is better.
    This is being used for the tests/utils.js (similar to the file in this repo).
  • flow-type definitions have to be manually created for now. But it seems to be possible to avoid this step if we would copy all source files into dist/ with their extensions renamed from .js to .js.flow. Other people seem to do it that way.
  • By renaming the flow/ folder to flow-types/ we don't need to reference it in the [libs] section of the .flowconfig anymore.
  • I kept the .babelrc, .eslintrc and .travis.yml, added some other files (editorconfig, eslintignore, npmrc, etc).
  • I switched from jest to ava to be able to write tests in ES6.

In regards to the flow-types: Maybe it will be enough to contribute type definitions of the public API of react-docgen to the flow-typed repository and use them in custom handlers (if they want to use flow).

Other than that it was pretty straight forward to create this module.

@ZauberNerd
Copy link
Contributor

Sorry for hijacking this thread. Back to the original topic:

I think it would be good to focus on both options (adding more maintainers and keeping the core small). Just to throw a crazy idea in here: http://openopensource.org/

Adding more maintainers could help:

  • reduce the bus factor
  • resolve issues and PRs in less time

I like the idea of keeping the core small or even going one step further (like babel) and having this project provide only the infrastructure (resolver and utilities) for other handlers (meaning: taking method, propTypes and displayName handler out of the core). These handlers could still be "core handlers" and be versioned in this repository, but are not automatically included. I think that would help to make the scope clearer on what goes into the core and what stays outside of it.

Cons of that approach could be the overhead of maintaining all these "core handlers" in isolation, maybe even chasing bugs that only occur in special combinations of handlers, confused or overwhelmed users because moving all the handlers out of the core could put more complexity in their projects.

But in any case, I agree that we first need to document and provide examples for how to create and publish custom handlers and figure out a way to make them accessible from the CLI.

@fkling
Copy link
Member Author

fkling commented Aug 18, 2016

Thanks for the feedback and input @ZauberNerd, that's really valuable! I also like the idea of moving each handler into the own package. Some of the handlers could still be loaded by default though, at least for the time being.

@levithomason
Copy link

levithomason commented Aug 21, 2016

This would be great. Would love to see a plugin oriented ecosystem around react-docgen. Would these plugins (handlers?) allow someone to write something to resolve imported props as in #33?

@fkling
Copy link
Member Author

fkling commented Aug 21, 2016

@levithomason: Definitely.

Currently most of the logic is contained in utility methods which is used by handlers, but of course handlers can contain arbitrarily complex logic. Handlers get passed a reference to the AST node that represents the component definition and they can do whatever they want with that.

Ideally, useful and reusable functions would be incorporated back into the core.

@khankuan
Copy link
Contributor

@fkling agreed. I just changed the strategy for #93 entirely and that really mean that these custom handlers can be unstable. Having them as plugins would probably be most ideal for development. However, I really would like to use the utils and tests setup that the repo already had. Those would reduce the overhead of splitting custom functionality into multiple repos.

Cheers! :)

@iamdustan
Copy link
Contributor

Huge 👍 to both proposal ideas, @fkling. I’m not certain what you would want or expect from me, but I’m on board for helping share the load a bit more on this. For getting the ball rolling I suspect we should work with #93 and #98 to move these out to individual handlers.

If we created a good workflow and documentation for plugin developers through that process I think the biggest bottleneck would be removed. Then a few of us will just need to watch the repo to help triage and point issues in the correct direction.

Many FB-led projects are driven by FB needs. How much will FBs internal approaches influence this that additional maintainers may not be aware of?

@fab1an
Copy link
Contributor

fab1an commented Sep 8, 2016

I would be great if one could just augment the default getPropType function and pass it to the handler. That way I could implement my custom props that I need.

@danez danez closed this as completed Mar 26, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants