Chart docs: annotate types

[dlabrecq]

The react-charts docs contain some complex object types. For example, the ChartArea component shows AnimatePropTypeInterface as the animate type below.

The Victory docs show the animate type as type: boolean || object.
See https://formidable.com/open-source/victory/docs/victory-area/#animate

Note: There are far more complex examples, but using animate for simplicity.

I don't want to override the existing Victory property types, just to add a description for react-docs. I would prefer to document properties using an annotation, if possible?

That may allow us to simplify the animate description / type as a boolean instead of AnimatePropTypeInterface. It would also isolate us from Victory changing the underlying property type and breaking our component wrappers.

[redallen]

What's wrong with overriding the existing Victory props to make animate?: boolean? What type of @ annotation would you like to use, and how would you like it to render?

[dlabrecq]

If animate were a boolean, that would generate an error because the underlying Victory component has a different type. Ideally, we wouldn't override types at all.

Overriding props to be a simple boolean, string, etc. is not going to work. There are far more complex objects with nested properties.

[redallen]

It wouldn't generate an error if we Omit<VictoryProps, 'animate'> and then pass to victory something like animate ? {object Victory expects} : {}. Still, what type of annotation would you like to use, and how would you like it to render?

[dlabrecq]

We should be able to import Victory interfaces in order to take advantage of existing property types and documentation. Ideally, pass-thru props would remain unchanged -- we shouldn't have to redefine prop types for react-docs.

Currently, we're overridding all Victory property types, and duplicating @type/victory documentation, simply to generate react-docs. It's a pain to keep in sync with Victory changes. And after all that, types like AnimatePropTypeInterface still don't resolve to anything useful.

It may be helpful if we could use some sort of annotation in order to define the type just for our documentation? For example, @type could be used to simplify the property type as boolean | object instead of AnimatePropTypeInterface -- as shown on Victory's site. Similar to how some JS doc tools use @example to format their doc layouts.

If we could at least do that, then we could leave the Victory pass-thru props untouched. Although, I still don't want react-docgen to display boolean | object as union -- that's also not useful.

I don't really have a good solution, but it's a problem we must solve. The react-docs are unreadable as is. Our users must go to Victory's site to view documentation, instead.

[redallen]

Oh, I completely understand that issue. That requires extending react-docgen to follow imports to resolve types. I can open that issue upstream.

[redallen]

Looks like a PR is already opened to add support for this. Awaiting its merge and a new package release.

[redallen]

Still awaiting its update and merge. Can likely dedicate a day this week to fix it upstream.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[dlabrecq]

The unreadable chart docs are still an ongoing issue.

Inheriting types would be ideal, but if we could somehow provide our own prop types, that would help us clarify the docs.

[redallen]

Yeah, this issue has become adding annotations like /* @proptype custom string of type */ and /* @hide */ for internal props. I'd like to follow Javadoc syntax or something of the like.

Are there any more annotations you think might be useful @dlabrecq @dlabaj @tlabaj ?

[tlabaj]

@redallen I agree with following Javadoc syntax . @deprecated would be very useful to have.

[dlabrecq]

That would be fine by me.

@deprecated, @hide, etc. would be a good start. If we could add them, just when we chose to override, that would be even better.

[dlabrecq]

I'm going to reopen this to ensure we apply annotations to the chart docs.

These are the supported annotations: https://github.com/patternfly/patternfly-org/blob/feat/org-redesign/packages

[dlabrecq]

Tried using @propType, but does not appear to work. Created https://github.com/patternfly/patternfly-react/issues/5304 to track it.