reactjs / react-docgen

A CLI and library to extract information from React component files for documentation generation purposes.
https://react-docgen.dev
MIT License
3.67k stars 297 forks source link

Descriptions not included in output #11

Closed mfunkie closed 9 years ago

mfunkie commented 9 years ago

I'm running docgen in the 'docs' script in package.json listed here https://github.com/BoomTownROI/boomstrap-react/blob/master/package.json

All of the properties and their types are output to https://github.com/BoomTownROI/boomstrap-react/blob/master/docs/docs.json

However, you may notice that none of the descriptions from https://github.com/BoomTownROI/boomstrap-react/blob/master/dist/Components/Fauxbox.jsx are not making it to the output documentation json file despite following conventions.

fkling commented 9 years ago

Mmh, I can't repro. I ran react-docgen with just the source of Fauxbox.jsx and the output includes the description.

I also cloned your repo and ran npm run docs and I can also see the description.

Did you try to delete node_modules and npm install again?

mfunkie commented 9 years ago

npm install after deleting node_modules/ didn't work, and I even checked to see if my react-docgen global was out of date just in case the npm script was not using the local node_modules/.bin to no dice. After that I tried deleting the repo and re-cloning which still did not work.

I suppose if you cannot reproduce, it's fine to close this bug, but I just cannot get the docs task to fill out the description.

fkling commented 9 years ago

Are there any errors, other than

Error with path "dist/Components/App.js": Error: No suitable component definition found.

?

mfunkie commented 9 years ago

That's the only one I'm getting when running the task. I know it's at least successfully completing because deleting docs/docs.json and re-running gets me all of the Components, just none of the descriptions.

fkling commented 9 years ago

The only thing I could think of is that the comments are stripped somehow. But in that case it shouldn't work for me either.

fkling commented 9 years ago

Could you provide some infos about your system? Which Node version, npm version, OS, shell, etc.

Just in case.

mfunkie commented 9 years ago

Node version: v0.10.29 (Note: Just upgraded to 0.12.2 and tried to no luck) npm version: 2.7.0 (Note: Just upgraded to 2.7.4 to no luck) OS: OS X Yosemite.3 Shell: fish (Note: I switched to bash and tried to no luck)

mfunkie commented 9 years ago

Found the issue that updating caused a fix. My line endings were set to Windows for some reason, and changing them to Unix style started letting Descriptions funnel through.

fkling commented 9 years ago

Glad you found the issue/solution! I think this line is responsible: https://github.com/reactjs/react-docgen/blob/master/src/utils/docblock.js#L37.

I guess we could support Windows line endings? Feel free to open a new issue for that if you feel strongly about it.

mfunkie commented 9 years ago

Yeah that looks like the one. When you're checking for *\n, does that first asterisk come from the second * in /** ?

fkling commented 9 years ago

Yes. The /*, */ are already stripped by the parser, since they are delimiters of the comment. So, my way of determining whether a comment is a docblock comment, is to look for *<line break> at the beginning.

I think we should actually consider any line terminator that the ECMAScript spec considers as valid.

mfunkie commented 9 years ago

So maybe something like

comment.value.match(/^\*(\r|\n)/).length

instead?

I'm sure there's a way to match unicode in regex, I just haven't done it in a long while.

Edit: from what I understand, it might look like

comment.value.match(/^\*(\u000A|\u000D|\u2028|\u2029)/).length

fkling commented 9 years ago

Yep, that looks like it should suffice, although I would write as

/^\*(\u000A|\u000D|\u2028|\u2029)/.test(comment.value)
mfunkie commented 9 years ago

Yup, that sounds better. I'm running into node issues when testing, so it may be a while before I can get in a pull request on that running.

fkling commented 9 years ago

No worries, I appreciate the help!

gonzochic commented 9 years ago

Hi, im using react-docgen on Windows and I am not able to get the prop type description in. It seems that the changes mentioned by @fkling and @mfunkie have not been implemented in the code. Have there been any issues with the implementation?

mfunkie commented 9 years ago

It worked pretty well when I ran the code change locally, I just couldn't get testing to run properly on my machine so I put it on the back burner and made sure all my line-endings were Unix. Would love to see it make it in though.

fkling commented 9 years ago

@gonzochic: Thanks for brining this up again. I haven't implemented the changes yet but I will take care of it now!

fkling commented 9 years ago

It turns out that this is not a problem with react-docgen but with recast. I will create a PR there.

To expand a little: It seems that recast (or something) normalizes comments to use \n as line terminator. That's why all we need to do in order to fix this is to make sure recast understands the different line terminators.

fkling commented 9 years ago

This was fixed in recast in v0.10.31. I added a (now passing) test that that I believe suffices to verify that this issue is fixed.

I'm not publishing a new version for this. If you reinstall react-docgen you should get the latest version of recast. Please give it a try and let me know!

gonzochic commented 9 years ago

Ok i will try! Thanks a lot.

gonzochic commented 9 years ago

Hi, for me it is still not working (i updated it to recast 0.10.32). Maybe it is because of the ES2015 Syntax? Here is my example code:

import React from 'react';

/**
 * General component description.
 */

export default class DocTest extends React.Component {
  constructor(props) {
    super(props);
  }
  render() {
    return null;
  }
}

DocTest.propTypes = {
  /**
     * Description of prop "prop1".
 */
  prop1: React.PropTypes.string.isRequired
};
DocTest.defaultProps = {
  prop1: 'default'
};

and as a result of the react-docgen script i get:

{
  "description": "",
  "props": {
    "prop1": {
      "type": {
        "name": "string"
      },
      "required": true,
      "description": "",
      "defaultValue": {
        "value": "'default'",
        "computed": false
      }
    }
  }
}

As you can see the description is not taken from the React Component. Do you have any hints on this issue?

fkling commented 9 years ago

@gonzochic: Mmh, your example works for me, but I guess the line terminators you are using might get lost somehow along the way. Not really sure what to do :-/

fkling commented 9 years ago

Closing this because I cannot repro. If someone still experiences this issue, please reopen.