storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
84k stars 9.23k forks source link

[Bug]: Args table rendering bugs #24084

Open jaydenseric opened 1 year ago

jaydenseric commented 1 year ago

Describe the bug

The args table that renders in story the docs page and in the story controls panel (when in mode expanded: true) has 3 issues when rendering in a sub table displayable parameters for a JSDoc based arg here:

https://github.com/storybookjs/storybook/blob/8580060e7c1bcfd7e1079cf0d440cf29bb88ef60/code/ui/blocks/src/components/ArgsTable/ArgJsDoc.tsx#L93-L103

All 3 issues can be seen in this one screenshot of how the Material UI React component TextField props render:

Screenshot 2023-09-06 at 8 23 37 am
  1. When multiple parameters being looped have the same name, it causes a React render warning about duplicate keys, e.g:

    Warning: Encountered two children with the same key, `event`. Keys should be unique so that components maintain their identity across updates. Non-unique keys may cause children to be duplicated and/or omitted — the behavior is unsupported and could change in a future version.
       at tbody
       at table
       at http://localhost:3003/node_modules/.cache/sb-vite/deps/chunk-C3ACZPIX.js?v=4e0658eb:1445:45
       at ArgJsDoc (http://localhost:3003/node_modules/.cache/sb-vite/deps/chunk-S3UHQVHQ.js?v=4e0658eb:904:19)
       at td
       at tr
       at ArgRow (http://localhost:3003/node_modules/.cache/sb-vite/deps/chunk-S3UHQVHQ.js?v=4e0658eb:1582:61)
       at tbody
       at table
       at http://localhost:3003/node_modules/.cache/sb-vite/deps/chunk-C3ACZPIX.js?v=4e0658eb:1445:45
       at div
       at http://localhost:3003/node_modules/.cache/sb-vite/deps/chunk-C3ACZPIX.js?v=4e0658eb:1445:45
       at ArgsTable (http://localhost:3003/node_modules/.cache/sb-vite/deps/chunk-S3UHQVHQ.js?v=4e0658eb:1655:9)
       at Controls3 (http://localhost:3003/node_modules/.cache/sb-vite/deps/chunk-S3UHQVHQ.js?v=4e0658eb:2077:9)
       at DocsPage (http://localhost:3003/.storybook/DocsPage.tsx:22:7)
       at div
       at http://localhost:3003/node_modules/.cache/sb-vite/deps/chunk-C3ACZPIX.js?v=4e0658eb:1445:45
       at div
       at http://localhost:3003/node_modules/.cache/sb-vite/deps/chunk-C3ACZPIX.js?v=4e0658eb:1445:45
       at DocsPageWrapper (http://localhost:3003/node_modules/.cache/sb-vite/deps/chunk-S3UHQVHQ.js?v=4e0658eb:807:26)
       at ThemeProvider (http://localhost:3003/node_modules/.cache/sb-vite/deps/chunk-C3ACZPIX.js?v=4e0658eb:1471:22)
       at SourceContainer (http://localhost:3003/node_modules/.cache/sb-vite/deps/chunk-S3UHQVHQ.js?v=4e0658eb:1882:26)
       at DocsContainer (http://localhost:3003/node_modules/.cache/sb-vite/deps/chunk-S3UHQVHQ.js?v=4e0658eb:2223:24)
       at CustomDocsContainer (http://localhost:3003/.storybook/preview.tsx:80:73)
       at Docs (http://localhost:3003/node_modules/.cache/sb-vite/deps/chunk-S3UHQVHQ.js?v=4e0658eb:2295:17)
       at MDXProvider (http://localhost:3003/node_modules/.cache/sb-vite/deps/@mdx-js_react.js?v=4e0658eb:28:24)
       at ErrorBoundary (http://localhost:3003/node_modules/.cache/sb-vite/deps/chunk-WQVT4N5M.js?v=4e0658eb:23:5)
       at WithCallback (http://localhost:3003/node_modules/.cache/sb-vite/deps/chunk-3CS6HZOQ.js?v=4e0658eb:15:23)

    Multiple parameters can have the same name when the type has multiple possibilities:

    Screenshot 2023-09-06 at 8 30 57 am

    In this example, here are where the 3 possibilities are defined:

  2. Each parameter's description Markdown is not rendered as HTML.

  3. The HTML table structure is not accessible / semantically correct, because a <td> is used instead of a correct <th scope="row"> for the header cell.

To Reproduce

Make stories for the Material UI React component TextField and view how the prop onChange renders in the story args table in the docs page and in the story controls panel (when in mode expanded: true).

Because Storybook refuses to docgen for components originating in node_modules, you will have to wrap the component TextField in a local project module and use that for the stories.

System

Environment Info:

  System:
    OS: macOS 13.5.1
    CPU: (10) arm64 Apple M1 Max
  Binaries:
    Node: 20.6.0 - ~/.volta/tools/image/node/20.6.0/bin/node
    Yarn: 1.22.19 - ~/.volta/tools/image/yarn/1.22.19/bin/yarn
    npm: 9.8.1 - ~/.volta/tools/image/node/20.6.0/bin/npm
  Browsers:
    Chrome: 116.0.5845.179
    Edge: 116.0.1938.69
    Safari: 16.6
  npmPackages:
    @storybook/addon-actions: ^7.4.0 => 7.4.0 
    @storybook/addon-controls: ^7.4.0 => 7.4.0 
    @storybook/addon-designs: ^7.0.5 => 7.0.5 
    @storybook/addon-docs: ^7.4.0 => 7.4.0 
    @storybook/addon-storysource: ^7.4.0 => 7.4.0 
    @storybook/addon-styling: ^1.3.7 => 1.3.7 
    @storybook/blocks: ^7.4.0 => 7.4.0 
    @storybook/docs-tools: ^7.4.0 => 7.4.0 
    @storybook/manager-api: ^7.4.0 => 7.4.0 
    @storybook/react: ^7.4.0 => 7.4.0 
    @storybook/react-vite: ^7.4.0 => 7.4.0 
    @storybook/theming: ^7.4.0 => 7.4.0

Additional context

Possibly related: https://github.com/storybookjs/storybook/issues/14481

jaydenseric commented 1 year ago

It could also be argued that it's a bug that the exact same parameter information displays multiple times:

Screenshot 2023-09-06 at 8 51 55 am

Identical variations of the same parameter name could be deduplicated.

jaydenseric commented 1 year ago

Also, the parameter type is not rendered! It's important information, and if it was displayed, it would be clear why there are multiple rows rendering with the same parameter name.