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

Basic React Hooks support (useImperativeHandle only) #454

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JaredReisinger
Copy link

@JaredReisinger JaredReisinger commented Jun 2, 2020

Add detection of useImperativeHandle()-defined methods on components, including test cases and README documentation.

Additional background: I have a component I've just refactored from a class-based implementation to the new hooks-style implementation, and am publishing the documentation via Styleguidist. Somewhat to my surprise, the once-public methods on the class, which are now exposed via useImperativeHandle(), are no longer showing up in the generated documentation. Digging in, this appears to be the correct place to get those methods exposed.

Note that this does not address the desire to document custom hooks, nor does it find useCallback() handlers as those aren't exposed in an imperative way and thus shouldn't be considered a part of a component's public API.

Add detection of `useImperativeHandle()`-defined methods on components, including test cases and README documentation.

Signed-off-by: Jared Reisinger <jaredreisinger@hotmail.com>
Signed-off-by: Jared Reisinger <jaredreisinger@hotmail.com>
@JaredReisinger
Copy link
Author

JaredReisinger commented Sep 17, 2020

@danez, has react-docgen gone into maintenance mode? I know that for the most part, there's not a lot to do since Javascript hasn't changed that much. That said, React added hooks in 16.8.0 (February 6, 2019), 19 months ago, so adding support for this isn't exactly "cutting edge" at this point.

I've tried to make sure the proposed change is well documented and tested... is there anything else I can do to make it easier or less-risky to merge?

danez
danez approved these changes Dec 12, 2020
Copy link
Collaborator

@danez danez left a comment

Really sorry for the delay. It looks good to me, was just wondering about the note in the readme.


If you are using React Hooks, react-docgen will now also find component methods defined directly via the `useImperativeHandle()` hook.

> **Note**: react-docgen will not be able to grab the type definition if the type is imported or declared in a different file.
Copy link
Collaborator

@danez danez Dec 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think this could work with the work that has been done in #464?

Copy link
Author

@JaredReisinger JaredReisinger Dec 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know.... I'll take a look at #464 and try to understand what it's doing. At first blush, it seems like yes, this might exactly solve that problem, I'll just need to understand what changes I'll need to make to use the new functionality (or whether things have changed in such a way that I don't have to do anything, and just get the new behavior for free 🤞 ).

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

Successfully merging this pull request may close these issues.

None yet

2 participants