react-docgen icon indicating copy to clipboard operation
react-docgen copied to clipboard

Add support for imported propTypes

Open echenley opened this issue 10 years ago • 50 comments

Importing prop types is a fairly common use case which doesn't appear to be possible at this stage. Any plans to support this pattern?

Example:

// defaultPropTypes.js

import { PropTypes } from 'react';

export default {
    prop1: PropTypes.string
};
// otherPropTypes.js

import { PropTypes } from 'react';

export default {
    prop2: PropTypes.object
};
// MyComponent.jsx

import { Component } from 'react';
import defaultPropTypes from './defaultPropTypes.js';
import otherPropTypes './otherPropTypes.js';

export default class MyComponent extends Component {
    static propTypes = {
        ...defaultPropTypes,
        ...otherPropTypes
    }
    // etc...
}

Results in:

{
  "MyComponent.jsx": {
    "description": ""
  }
}

echenley avatar Oct 19 '15 23:10 echenley

I guess we could do this for simple cases, where a variable is spread into propTypes (i.e. ...foo, but not ..foo.bar) and that variable resolves to a module import.

I didn't want to make react-docgen make any assumptions about how module identifiers are resolved, but I guess it's fair to assume that it will be CommonJS compatible (i.e. module identifiers refer to npm modules or local files).

Then we could inspect the other file and if it returns an object literal, inspect and merge it as well.

fkling avatar Oct 20 '15 16:10 fkling

+1 :)

frontendphil avatar Dec 02 '15 15:12 frontendphil

That would be a great feature

oliviertassinari avatar Dec 07 '15 22:12 oliviertassinari

I've got a similar usecase, but not using spread when injecting the props.

// ILink.js
import {PropTypes} from 'react';

export default PropTypes.shape({
  url: PropTypes.string.isRequired,
  selected: PropTypes.bool,
  target: PropTypes.string,
  title: PropTypes.string.isRequired
});
// MyComponent.jsx

import { Component } from 'react';
import ILink from './ILink.js';
import otherPropTypes './otherPropTypes.js';

export default class MyComponent extends Component {
    static propTypes = {
        link: ILink
    }
    // etc...
}

thebuilder avatar Mar 02 '16 21:03 thebuilder

The same idea would also apply to flow types, as they can be also imported. So maybe this "import something" could be abstracted in a way so it makes imported propTypes and flowTypes possible.

danez avatar Mar 02 '16 21:03 danez

Even we stuck with similar use case, any plans to add support for external imports of propTypes, below is our use case.

constants.js
const BUTTON_STYLES = ['A', 'B', 'C', 'D'];
export default BUTTON_STYLES;
component.js
import BUTTON_STYLES from 'constants';
propTypes = {
kind: PropTypes.oneOf(constants),
}

class SomeComp extends React.Component{
static propTypes = propTypes;
}

export default SomeComp;
Generated AST:
'kind': {
            'type': {
                'name': 'enum',
                'computed': true,
                'value': 'BUTTON_STYLES'
            },
            'required': false,
            'description': 'some description',
            'defaultValue': {
                'value': 'A',
                'computed': false
            }
        },
Expected AST:

value of AST should be as below(which is working fine when BUTTON_STYLES are in same file, but not with import)

'kind': {
            'type': {
                'name': 'enum',
                'value': [
                    {
                        'value': 'A',
                        'computed': false
                    },
                    {
                        'value': 'B',
                        'computed': false
                    },
                    {
                        'value': 'C',
                        'computed': false
                    },
                    {
                        'value': 'D',
                        'computed': false
                    }
                ]
            },
            'required': false,
            'description': 'Button has a a different set of `kinds`',
            'defaultValue': {
                'value': 'A',
                'computed': false
            }
        }

pasupuletics avatar May 26 '17 09:05 pasupuletics

What do you think about a simpler approach? In our case, it would be enough to document our components with a generic message whenever a spread is found.

So, for a component like this one:

MyComponent.propTypes = {
  /**
    * Component Foo proptypes
    */
  ...Foo.propTypes,
  /**
    * Some other proptypes
    */
  ...propTypes,
  /**
    * Unhandled spread
    */
  ...{
    bar: Prop.bool,
  }
};

We would like to get the following descriptors:

{
  "...Foo.propTypes": {
    "description": "Component Foo proptypes"
  },
  "...propTypes": {
    "description": "Some other proptypes"
  }
}

Right now, these proptypes are just ignored, resulting in incomplete docs :(

We have got a proof of concept fork which adds support for Identifier or MemberExpression spreads, ignoring other spread expressions.

I can create a PR if you think this approach is worth it :)

asis avatar Jun 05 '17 12:06 asis

@asis: They are not ignored: https://github.com/reactjs/react-docgen/blob/00f2d2d62d93b9243157c4cea19cf241121a7721/src/handlers/tests/propTypeCompositionHandler-test.js, but they are stored separately from normal props.

fkling avatar Jun 05 '17 21:06 fkling

Oh, sorry! I was focused on extracting docs from the props declaration, so when I saw https://github.com/reactjs/react-docgen/blob/00f2d2d62d93b9243157c4cea19cf241121a7721/src/handlers/tests/propDocblockHandler-test.js#L127 I thought spread props were just being ignored altogether 😊

Thanks for the quick response!

asis avatar Jun 06 '17 07:06 asis

+1 Have same use case as @asis We have our wrappers around semantic-ui components very often, and I try to spread their propTypes to have their props documented as well as ours but got nothing. Just wrote some makes-no-sense component for test and parse it and got:

{
  "description": "Our wrapper component around semantic-ui button\r\nFor more props visit\r\n[semantic-ui-button](https://react.semantic-ui.com/elements/button#button-example-button)",
  "methods": [],
  "props": {
    "children": {
      "type": {
        "name": "any"
      },
      "required": false,
      "description": "Button label"
    }
  },
  "composes": [
    "semantic-ui-react"
  ]
}

Code:

import React from "react";
import PT from "prop-types";
import { Button as SButton } from "semantic-ui-react";

/**
 * Our wrapper component around semantic-ui button
 * For more props visit
 * [semantic-ui-button](https://react.semantic-ui.com/elements/button#button-example-button)
 */
const Button = ({ children, ...rest }) =>
  <SButton {...rest}>{children}</SButton>;

Button.propTypes = {
  /** Button label */
  children: PT.any,
  ...SButton.propTypes
};

export default Button;

Will be awesome to have this with comments that propTypes of the imported component have. Since all semantic and our components are react-docgen documented.

RIP21 avatar Jun 06 '17 22:06 RIP21

@RIP21 The recommended approach (which is also what we do at Facebook), is to merge the docs yourself. I.e. since you know that this component composes semantic-ui-react, you can look up the documentation for the imported component.

I guess in your specific case we would also have to record which export you are importing (Button).

fkling avatar Jun 06 '17 22:06 fkling

I'm running into the same thing while using either propTypes = CustomPropTypes; or trying to destructure as propTypes = {...CustomPropTypes};. Is there any work around currently or is this still being discussed? I apologize but reading through the comments here it was not clear to me what the proposed solution is.

reintroducing avatar Jul 24 '17 14:07 reintroducing

We just used the list of "composed props" to infer the source files where they were defined. To solve the problem in styleguidist/react-styleguidist#548, we use a custom propsParser which turns each composed props reference into a "custom" prop, with a description containg a link to the relevant section of our docs. It is brittle... but it works.

asis avatar Jul 26 '17 06:07 asis

I have over come this limitation by writing my own custom handler, And would like to thank @benjamn for providing such a wonderful library called recast, which makes my life easy while dealing with AST. And big thanks to react-docgen too.

pasupuletics avatar Jul 28 '17 20:07 pasupuletics

Would you mind sharing your handler?

reintroducing avatar Jul 28 '17 22:07 reintroducing

@reintroducing , Unfortunately I can't share it now, If possible will publish here for sure.

pasupuletics avatar Jul 29 '17 06:07 pasupuletics

@pasupuletics, how about now? Care to share your custom handler?

workjalexanderfox avatar Oct 27 '17 20:10 workjalexanderfox

So this is still not a thing?

selrond avatar Feb 23 '18 08:02 selrond

@workjalexanderfox , Here you go https://github.com/pasupuletics/learning/blob/master/react-docgen-external-proptypes-handler.js

Just go through react-docgen custom handler documentation to configure.

pasupuletics avatar Mar 06 '18 08:03 pasupuletics

This feature is really vital, especially for styleguides etc. Can somebody integrate such finally :(

RIP21 avatar Mar 06 '18 08:03 RIP21

@pasupuletics That's awesome!!! Works a treat, thanks!

SimeonC avatar Mar 06 '18 11:03 SimeonC

@RIP21: Have you tried using @pasupuletics' custom handler?

Can somebody integrate such finally :(

Supporting dependency resolution in such a way that it works reliably in various environments is not trivial. That's why I'm suggesting that you are using a custom handler that works specifically for your environment.

Of course someone could write a reusable handler that works with plain CommonJS, someone else could provide one that understands with webpack, etc.

I don't have the bandwidth to build and support all this.

fkling avatar Mar 06 '18 15:03 fkling

@pasupuletics can you please add some license to your repo?

If you find software that doesn’t have a license, that generally means you have no permission from the creators of the software to use, modify, or share the software. Although a code host such as GitHub may allow you to view and fork the code, this does not imply that you are permitted to use, modify, or share the software for any purpose.

https://choosealicense.com/no-permission/

You've added UNLICENSED to your package.json, but that's probably not what you actually want:

Finally, if you do not wish to grant others the right to use a private or unpublished package under any terms:

{ "license": "UNLICENSED" }

https://docs.npmjs.com/files/package.json#license

Hypnosphi avatar Apr 27 '18 15:04 Hypnosphi

@pasupuletics I hope you don't mind, I published the handler on npm and documented it: https://github.com/siddharthkp/react-docgen-external-proptypes-handler

siddharthkp avatar May 11 '18 07:05 siddharthkp

@siddharthkp , Thats really awesome. No issues. Glad to see my name in repo.

pasupuletics avatar Jul 13 '18 11:07 pasupuletics

This issue has been opened in 2015 and still open. Any progress on fixing this?

JuhQ avatar Aug 16 '18 11:08 JuhQ

I'm here as I'm using docz which uses docgen in its pipeline. Each of my components have their own directory with several files at the root which abstracted from the main component, one of those is a Flow file which annotates the component and imports its types. Because docgen can't read this file, none of my props generate for the prop table.

@danez you mentioned this earlier and looks still to be an issue. Any ideas where could get started on this to create a PR?

danm avatar Aug 19 '18 14:08 danm

@pasupuletics @siddharthkp If I understand correctly (I have downloaded and tried this handler in my project), that handler only works for imported constants but not for actual imported propTypes? I have entire propTypes declared in external files which I would like to use.

benwiley4000 avatar Oct 01 '18 06:10 benwiley4000

@benwiley4000 , It should work for both. If you never mind, can you please share your project config.

pasupuletics avatar Oct 01 '18 06:10 pasupuletics

@pasupuletics thanks for following up! here's my styleguidist config which specifies the docgen handlers. https://github.com/benwiley4000/cassette/blob/48ab0a5ea187d4458df6933927b6cc1522cc1469/styleguide.config.js#L180

In particular I'm wanting this propType (repeatStrategy) to show up as an enum which should be one of "none", "playlist" or "track" as specified here.

benwiley4000 avatar Oct 01 '18 07:10 benwiley4000