-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
feat: add hook to find docgen props #1371
Conversation
0072cbd
to
5c162cc
Compare
@mickaelzhang great, looks promising. One more idea would be to patch output:
in case you can inject metadata to children |
Thanks !! It would be great indeed to have an example of output! |
@droganov The output of {
propName: {
defaultValue: null,
description: 'Prop name description',
name: 'propName',
parent: 'path/to/component.ts',
required: true,
type: 'string',
},
anotherPropName: { ... },
...
} I edited the description of the PR with an example of output I'm not sure that understand your example 😅 If it's about the example that I give, you could completely change the logic if you want, the example that I give is the shadowed component so you have the complete control over that! @aarondancer @ejuo @chunksnbits I just saw that you guys also upvoted the original issue, do you guys have an opinion on this? |
@mickaelzhang |
@mickaelzhang I thought in case it outputs N rows for N props, we could inject corresponding prop to each row as static member. Then we could iterate children knowing component name, displayName and path, and create local component that abstract that problem away. We still could do so with the hook I think. |
You change ({ of, children }) => {
const result = Object.entries(of).map(renderPropAndInjectMetadata);
return typeof children === 'function' ? children(result) : result;
} Then we can filter rendered rows without hooking any paths manually: <Props of={UiComponent}>
{renderedProps => children.filter(propRow => propRow.componentName === 'UiComponent')}
</Props> I mean when |
@droganov Ah yes, I understand now! So you have a case where you want to filter in the This is a bit different than the case describe in my PR then, what I'm trying to solve is having a way to filter on all component at once with props definitions from other components. What you are describing is possible right now by shadowing the |
@droganov The use case that you describe is interesting. Maybe you could open an issue so we can discuss about this specific use case and if it should be a built-in feature? For this PR in particular, it seems like it look good to all of you? We can move to the review part then! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me 👍
It would also be cool if someone could add a small example to the examples directory to help others quickly know how to filter props. 👍 |
Can you please add to the documentation the explanation on how to filter props? Thanks 🙏 |
@maryiam I will when I can find a bit of time! :) |
Ok thank you @mickaelzhang. Also, it would be great to mention the released version on which this feature will be available. 🙌 |
@maryiam PR opened on the documentation, doczjs/docz-website#98 Let me know if you have any suggestion to make it clearer 👍 |
Description
In some cases, we want to have access to data that we have in docgen.
Some possible use case would be to filter some general props (similarly to https://sproutsocial.com/seeds/components/alert#properties with the system props) or if you inherit props from HTMLElement and you don't want it to show.
An example of
useComponentProps
's output would be:Related issue:
#895
Use case example
extends HTMLElement
=> too many props (100+ maybe?)Subject to change if it doesn't fit all use cases.