wingedfox / dgeni-alive

Live docs on top of dgeni documentation generator
MIT License
26 stars 14 forks source link

Support for documenting React components. #16

Closed gbbr closed 7 years ago

gbbr commented 7 years ago

We are using dgeni-alive in our open-source project, Superdesk, and it's been working great so far. So thank you for writing this great pre-configuration to dgeni!

We had some things that we needed to modify in order for it to suit our project better, so we've done that in our fork (on my account). I was thinking that these changes could benefit others too, who are in the same situation. By Google-ing around, I've noticed that some people are.

In this PR:

What do you think?

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 57.505% when pulling 44b77df0c05eb03cdae2fadf9cd4794c8f09d012 on gbbr:react into d55bf1bdb078fe74b9eb39623c162006909210a7 on wingedfox:master.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 57.505% when pulling 6dcab90705928a8c6120c336d252762c6aa8250d on gbbr:react into d55bf1bdb078fe74b9eb39623c162006909210a7 on wingedfox:master.

wingedfox commented 7 years ago

Hi,

Thank you for the PR.

I've updated it a bit by moving react-related components to jsx package. You have to add jsx as an optional package in your documentation building task, but now you're free to implement your own extractJSDocCommentsProcessor, using estraverse-fb tool to extract comments.

gbbr commented 7 years ago

Yes, what you did looks great! Thanks!

wingedfox commented 7 years ago

Thanks.

Could you, please, add PR with fix for extractJSDocCommentsProcessor and update docs accordingly?

gbbr commented 7 years ago

I am quite caught up with work, but I will try to submit a PR when I get a chance. Not sure exactly what it is that you are asking for. These changes alone have helped me solve my problem and documentation is correctly extracted from JSX files in our app now...

Basically this was all I needed, besides including estraverse-fb in our project. Not very familiar with dgeni but if you can be more specific I could try and help (can't promise anything though! 😄 )

wingedfox commented 7 years ago

As long as you have the change that fixes your issue with broken eslint, I'd be glad to see it included in the project. If all you need is to have estraverse-fb together with es-lint, then it is an optional dependency and you have to check for its' existence in and display a corresponding warning.

gbbr commented 7 years ago

@wingedfox the change is already included. I have actually ended up spending hours to try and understand why having eslint as a peer dependency causes the issue, and I have not figured it out. After adding estraverse-fb to our project, it worked out. But the odd thing is that estraverse-fb was not needed when eslint was not there. So if we were to add a warning to dgeni-alive about this, it might be unneeded in some scenarios (afaik). This probably needs more thorough testing.

My time is limited but I'm glad to help if you have any ideas about directions we should go towards.

To summarise: dgeni-alive had no problem parsing JSX files in its current state, even without estraverse-fb using eslint 0.9.x, but not using the latest eslint.