digdir / designsystemet

Designsystemet
https://designsystemet.no
MIT License
84 stars 38 forks source link

Wrong prop types displayed in Storybook props table #2760

Open mimarz opened 1 week ago

mimarz commented 1 week ago

When declaring a default value for native props in our preview story, wrong type is displayed in the props table. It uses the type for the value declared in the preview story instead of the actual type on the component.

Notes;

elaffen commented 1 week ago

Why should you list the children prop unless it's something else than the default ReactNode?

mimarz commented 1 week ago

Aye, having tested a bit this looks to be related specifically to the children prop.

Its a good point of declaring jsdoc for children for more composed component such as Accordion, where we expect the children to be multiple Accordion.Item, but for something like Button maybe not so, we still want that to be?

I agree we should document when children is expected to be a set of specific sub-components.

Lets do some testing with the skipChildrenPropWithoutDoc option. Maybe that can be a quick fix for now and we'll get some proper jsdoc for the children prop sorted later.

elaffen commented 1 week ago

Your propFilter prevents the children from being Docgenerated. That is why you need to define the prop yourself or tweak the filter. https://github.com/digdir/designsystemet/blob/6c59fc5652c8c78ef4b0b94faea05f2618989dab/apps/storybook/.storybook/main.ts#L19

But because you have specified that children arg in the Alert-story, Storybook guesses that children is of type 'string' https://github.com/digdir/designsystemet/blob/6c59fc5652c8c78ef4b0b94faea05f2618989dab/packages/react/src/components/Alert/Alert.stories.tsx#L24

If you change the value of children in the story to true children: true, in Alert.stories.tsx line 24, Storybook will list the prop as boolean.

mimarz commented 1 week ago

Your propFilter prevents the children from being Docgenerated. That is why you need to define the prop yourself or tweak the filter.

https://github.com/digdir/designsystemet/blob/6c59fc5652c8c78ef4b0b94faea05f2618989dab/apps/storybook/.storybook/main.ts#L19

But because you have specified that children arg in the Alert-story, Storybook guesses that children is of type 'string'

https://github.com/digdir/designsystemet/blob/6c59fc5652c8c78ef4b0b94faea05f2618989dab/packages/react/src/components/Alert/Alert.stories.tsx#L24

If you change the value of children in the story to true children: true, in Alert.stories.tsx line 24, Storybook will list the prop as boolean.

yeah, good catch! We added that filter because of performance problems locally with react-docgen scanning for types everywhere. We'll have to fix this somehow so that we get the correct types for children on all our components :)