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

Better support for custom types #24

Open
chrisui opened this issue Sep 18, 2015 · 10 comments
Open

Better support for custom types #24

chrisui opened this issue Sep 18, 2015 · 10 comments

Comments

@chrisui
Copy link

chrisui commented Sep 18, 2015

This is more of a "would you be willing to accept a pull request which does this?" question than an issue of sorts. :)

I would really like to extend the functionality of "custom" types so that:

  1. We support destructured imports (Eg. PropTypes.array - note the lack of React.)
  2. We allow common react-esque prop type function patterns to be picked up with custom props in the same way that arrayOf is currently handled. (Eg. PropTypes.iterableOf(PropTypes.record(User)))

Basically I feel the library should be able to support custom user prop types in such a way we don't have to reparse the raw value which would mean anyone can easily implement simple custom prop types and create documentation from that.

Custom prop types are super useful in building a real-world app with React and so I would like to see how we can help developers document these too.

We, at Lystable, want to build an open living component docs solution and are hoping react-docgen can feature at the core!

@chrisui
Copy link
Author

chrisui commented Sep 19, 2015

To clarify here are a few examples:

Ex 1. Firstly the destructuring pattern should be supported.

static propTypes = {
  prop: PropTypes.string
}

// yields {type: {name: 'string'}}

Ex 2. With the above the [x].isRequired would be a common pattern we can extract.

static propTypes = {
  prop: PropTypes.string.isRequired
}

// yields {type: {name: 'string'}, required: true}

Ex 3. Custom function prop-types should be supported.

static propTypes = {
  prop: PropTypes.record(User)
}

// yields {type: {name: 'record', value: 'User'}}

Ex 4. Again, .isRequired pattern should be honoured.

static propTypes = {
  prop: PropTypes.record(User).isRequired
}

// yields {type: {name: 'record', value: 'User'}, isRequired: true}

Ex 5. Function pattern will support nested types.

static propTypes = {
  prop: PropTypes.iterableOf(PropTypes.record(User))
}

// yields {type: {name: 'iterableOf', value: {name: 'record', value: 'User'}}}

Ex 6. To confirm, these custom props can be used deeply.

static propTypes = {
  prop: PropTypes.iterableOf(PropTypes.oneOf([PropTypes.record(User), PropTypes.bool])
}

// yields {type: {name: 'iterableOf', value: {name: 'enum', values: [{name: 'record', value: 'User'}, ...]}}}

Notes/thoughts:

  • Custom enum syntax is not needed as standard oneOf covers any possible usage of this.
  • .raw should still be present on the type object
  • This output implements a breaking change now that name: 'custom' has gone.
    • Can't see this as being an issue as .raw still exists and consumers can still do the same stuff
    • If this is an issue we could just have a new .customName property.

@fkling
Copy link
Member

fkling commented Sep 22, 2015

Sorry for the late reply. Just to clarify, all your examples would work on objects that are not React.PropTypes and have arbitrary methods. E.g.

static propTypes = {
  prop: Types.validationFunction
}

// yields {type: {name: 'validationFunction'}}

correct?

If so, yes, that would be great! Thank for you for the detailed list, that makes a lot of sense to me.

@chrisui
Copy link
Author

chrisui commented Sep 22, 2015

That is correct.

If you agree also:

We would also accept a single variable/method reference. Ie.

static propTypes = {
  prop: validationFunction
  // or
  prop: validationFunction(props, propName, ...) {}
}

// yields {type: {name: 'validationFunction'}}

isRequired is the only special case property name where we stop looking further I think. Which results in the following behaviour:

static propTypes = {
  prop: Types.isRequired
}

// yields {type: {name: 'Types'}}

@chrisui
Copy link
Author

chrisui commented Sep 22, 2015

My last comment may be a second iteration though looking at the time I have available ;)

@fkling
Copy link
Member

fkling commented Sep 22, 2015

How would you resolve a.very.long.member.access.chain? Would you just look at the last property? Overall this looks very reasonable to me and I will gladly accept a PR for this :)

I believe it wouldn't even be too much effort to implement it with the utility functions that react-docgen has.

@chrisui
Copy link
Author

chrisui commented Sep 22, 2015

This was exactly what I was just thinking of as well. Not unreasonable to expect users may want to have more granular namespacing

I think we take the last property of the chain for now and we can see what use cases come up. .raw will always be there for manual inspection anyway.

I'll get to work!

@chrisui
Copy link
Author

chrisui commented Sep 22, 2015

Any quick pointers to get me going quicker will be appreciated :P

@fkling
Copy link
Member

fkling commented Sep 25, 2015

Looking at the current implementation would be a good start: https://github.com/reactjs/react-docgen/blob/master/src/utils/getPropType.js

@mik01aj
Copy link

mik01aj commented Jun 7, 2016

@chrisui this is a great spec, thank you! Do you have any progress on this?

@levithomason
Copy link

Ditto. I'm also wondering if this may actually become a plugin in light of #115.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants