Added post-processing step for documentation objects

[adityavohra7]

Definitely a WIP, but thought I'd get a PR going to have something to discuss on. This branch adds a post-processing step for documentation objects. Currently, this post-processing step only does what's needed to "fix" #221, but I can imagine we could use it for validation of documentation objects as well? I haven't added any tests yet. Let me know what you think!

[adityavohra7]

Looking into the test failures!

Edit: Fixed. Had to update a snapshot, and account for cases wherein props isn't defined.

[adityavohra7]

Friendly bump! Let me know if you think we should take some other approach here!

[rosskevin]

@adityavohra7 is this still a WIP or is it good to use/merge?

@danez #221 is a huge problem for material-ui docs - if you could take a look and let us know if this direction is good, we need to get our docs generating correctly again.

Thanks to you both!

[adityavohra7]

@rosskevin, the PR definitely needs a couple of tests. I was waiting for any maintainer to look it over though to confirm they agreed with the approach the PR takes. Majority of the doc generation is "independent" (handlers are unaware of each other), but this approach sort of "combines" the documentation that each handler creates.

[rosskevin]

I just noticed that this is for master and we are already dependent on the 3.0-dev releases. I'm going to try and work this code into that branch and see if I can get material-ui running again.

[adityavohra7]

Friendly bump! Any input by other contributors/maintainers would be appreciated!

[danez]

Sorry for the late reply, this looks overall good, to me. The only thing I'm not sure is that this right now would hardcode the behaviour. I don't see a reason why anyone would not want to have this, but who knows. So maybe it would be better to make it a prober handler and add it as the last one. I guess that should work too?
Some tests would be necessary.

And targetting the master branch is totally fine, I can pick stuff to the v3 branch after it is merged as long as there are tests that verify it works. :)

Nice work.

[adityavohra7]

Hi @danez! Thanks for the review! I've rebased on master and added a simple test case for a component with defaultProps and flow type annotations for props.

The only reason I didn't add the "combining of defaultProps and flow types" behavior in the form of a handler is that there'd be an implicit requirement for this handler to run last (because it depends on the results of the props and flow handlers), whereas all the current handlers are pretty much independent of each other? If we prefer that, I don't mind changing the current post-processing step to a handler!

[adityavohra7]

Friendly bump! @danez, what do you think about the updated PR? 😊

[danez]

I merge this now as is, I think having it hardcoded is okay for now. We will see if someone complains about wanting to disable this behaviour. If so we can still convert it to a handler without having todo a major version.
Thanks for your work.