[Bug]: Props tables for custom elements are inaccurate

hannajones commented 2 years ago

Describe the bug

When creating web components using Lit, all properties are set up to have corresponding attributes by default. After following the instructions here for generating pops table documentation, I noticed that only attributes where the name had explicitly been set to use kebab case were displaying in the docs tab, despite having both members and attributes information in my custom-elements.json manifest.

Looking at the code for extracting argTypes it's clear that this is due to properties overriding attributes except in those cases where the attribute name differs from the property name. With Lit it's also possible to prevent an observed attribute from being created by specifying { attribute: false } - currently there's no way to determine if attributes are simply missing from the documentation or if their absence is intentional.

This has already been identified as an issue in https://github.com/storybookjs/storybook/issues/18858, but I think the fact that any overwriting is happening at all is the real problem, rather than the order in which overrides are occurring. Even with that proposed change to the order there will still be no way to achieve accuracy. What if a team has a use case where it's more important to document the properties? What if you want to document that both properties and attributes are available, despite the repetition?

I've also run into issues with overwriting when trying to document @cssparts that share a name with a property or attribute. For example, if your component has a header property and attribute and you also want to expose and document part="header" you only end up seeing the CSS shadow part documentation in the table because it overrides everything else. This is avoidable with naming/annotation changes, but it seems less than ideal to have to change a component's interface to get around the limitations of the documentation tooling - especially considering that there's no real issue of naming collisions between CSS shadow parts and properties/attributes.

I've created an example that demonstrates these different issues using slightly modified code from the demo-wc-card example. I'm not sure what the ideal solution is here - maybe just to embrace the repetition for now and in the longer term provide some way for consumers to specify what they want to see in the props table (maybe that exists already and I've missed it, in which case please let me know!).

To Reproduce

https://github.com/hannajones/storybook-wc-args-table-repro

https://636d15e73f8f509c3e377975-gwiszopxhu.chromatic.com/?path=/docs/demo-wc-card--front

System

System:
    OS: macOS 12.5.1
    CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
  Binaries:
    Node: 16.9.1 - ~/.nvm/versions/node/v16.9.1/bin/node
    Yarn: 3.2.4 - ~/.nvm/versions/node/v16.9.1/bin/yarn
    npm: 7.23.0 - ~/.nvm/versions/node/v16.9.1/bin/npm
  Browsers:
    Chrome: 107.0.5304.110
    Firefox: 105.0.3
    Safari: 15.6.1
  npmPackages:
    @storybook/addon-actions: ^6.5.13 => 6.5.13
    @storybook/addon-docs: ^6.5.13 => 6.5.13
    @storybook/addon-essentials: ^6.5.13 => 6.5.13
    @storybook/addon-links: ^6.5.13 => 6.5.13
    @storybook/builder-webpack4: ^6.5.13 => 6.5.13
    @storybook/manager-webpack4: ^6.5.13 => 6.5.13
    @storybook/web-components: ^6.5.13 => 6.5.13

Additional context

No response

usrrname commented 1 year ago

May be duplicate or related storybook/issues/20967

ChellappanRajan commented 8 months ago

Any update on this issue?

robinsummerhill commented 1 month ago

I'm also seeing this issue with 8.3.2 (storybook, @storybook/web-components, @storybook/web-components-vite). If any prop, attribute, css-part, slot shares a name then only one makes it into the Storybook documentation tables.

This makes it pretty useless for documenting web components as it stands.

From a very quick look at the code it seems that an assumption is made that there will never be duplicate entities with the same name e.g. a property and a CSS slot. argTypes is an object keyed by name so can never hold two items with the same name. The relevant bit of code can be seen here:

https://github.com/storybookjs/storybook/blob/0811ca8bd592dbee4eadb94f46fccadda21fe0bf/code/renderers/web-components/src/docs/custom-elements.ts#L153

export const extractArgTypesFromElements = (tagName: string, customElements: CustomElements) => {
  const metaData = getMetaData(tagName, customElements);
  return (
    metaData && {
      ...mapData(metaData.members ?? [], 'properties'),
      ...mapData(metaData.properties ?? [], 'properties'),
      ...mapData(metaData.attributes ?? [], 'attributes'),
      ...mapData(metaData.events ?? [], 'events'),
      ...mapData(metaData.slots ?? [], 'slots'),
      ...mapData(metaData.cssProperties ?? [], 'css custom properties'),
      ...mapData(metaData.cssParts ?? [], 'css shadow parts'),
    }
  );
};

Each type of entity is crammed together into a single argTypes object - presumably the order of overwriting that people are seeing matches the order that the different types of entity are added to argTypes in the code above.

storybookjs / storybook