[docs] Generate default values for docs from the unstyled components #23614
Conversation
|
|
||
| Object.keys(unstyledReactAPI.props).forEach((prop) => { | ||
| if (unstyledReactAPI.props[prop].defaultValue) { | ||
| reactAPI.props[prop] = unstyledReactAPI.props[prop]; |
There was a problem hiding this comment.
Shouldn't you just add the defaultValue? Sounds confusing if description, types etc. mismatch that we use the prop API from unstyled only if it has a default value.
There was a problem hiding this comment.
If the prop has a default value, then we have a bunch of checks related to validity like:
Checking JSDOC - JSDOC @default annotation not found for '${propName}'. for example.
That's why I decided to copy all info.
There was a problem hiding this comment.
I had the same question that Sebastian, I came with this rationale: the current logic could help us enforce that unstyled and styled have the same API. If the implement diverges, a developer might open an issue to mention how the documentation is wrong, this could be a signal for us to "realign" the implementation. I don't know if that works in practice but it seems to have potential.
Maybe we could shorten the feedback loop (skip the developer part) by testing for differences in the object and fail if something is wrong.
There was a problem hiding this comment.
My first thouht was that it should look something like this:
reactAPI.props = { ...reactAPI.props, ...unstyledReactAPI.props };
so that the unstyled override and be consistent. The main reason why I didn't just override the main component props definition with unstyled is that sometimes the styled component could add default, that the unstyled didn't have so this seemed wrong. That's why I decided it's probably safer if the unstyled override some props only if it needs to, the only case we know so far is if it has defaults..
Not sure if it is the best way forward, but I think is safest at this moment as we don't know if there will be other use-cases we will find with the next components...
There was a problem hiding this comment.
the current logic could help us enforce that unstyled and styled have the same API.
That would not make sense for many APIs. For example, some components have props that are related to styling only. So these props should not propagate to the unstyled version or vice versa.
Same for the description. In the styled version we might want to add more information or even remove information that we handled internally.
In any case, unstyled and styled do not have the same interface. Therefore their documentation will not be equal. We will encounter this once we split more components. Though it's not a problem at the moment.
There was a problem hiding this comment.
We will encounter this once we split more components. Though it's not a problem at the moment.
Agree, it's probably what will happen, I guess our best option it to try the path that leads to the maximum consistency and to diverge anytime it's no longer possible to hold :).
There was a problem hiding this comment.
Should we then for the moment keep this as is?
There was a problem hiding this comment.
Yeah, just wanted some clarification whether this "works for now"-code or "correct"-code.
This PR generates the default values on the docs from the unstyled component if available. Resolves #23308 (comment)