storybookjs / storybook

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

custom-elements-manifest -> attributes are overriden by members #18858

Open sonntag-philipp opened 1 year ago

sonntag-philipp commented 1 year ago

Describe the bug The members that are contained in the manifest override other defined properties. As all attributes in FAST are also members of a class, all attributes are always overriden by the members while analyzing the manifest.

See the extractArgTypesFromElements method defined by storybook/web-components. The order used to define the class overrides the attributes.

Solution proposal I would like to change the order of the map method to this:

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'),
    }
  );
};

This ensured in first tests that properties and members are overriden by attributes, thus correctly displayed in the Storybook.

Pull Request I'm open to create a PR for this change. I just wanted to verify that a change like this is accepted before I submit a PR.

Environment Info

  System:
    OS: Windows 10 10.0.22000
    CPU: (16) x64 AMD Ryzen 7 5700U with Radeon Graphics
  Binaries:
    Node: 16.16.0 - ~\AppData\Roaming\nodejs\node.EXE
    npm: 8.11.0 - ~\AppData\Roaming\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.22000.120.0), Chromium (100.0.1185.39)
  npmPackages:
    @storybook/addon-actions: ^6.5.9 => 6.5.9
    @storybook/addon-essentials: ^6.5.9 => 6.5.9
    @storybook/addon-links: ^6.5.9 => 6.5.9
    @storybook/addons: ^6.5.9 => 6.5.9
    @storybook/builder-webpack4: ^6.5.9 => 6.5.9
    @storybook/core-common: ^6.5.0 => 6.5.9
    @storybook/csf-tools: ^6.5.0 => 6.5.9
    @storybook/manager-webpack4: ^6.5.9 => 6.5.9
    @storybook/store: ^6.5.0 => 6.5.9
    @storybook/test-runner: ^0.5.0 => 0.5.0
    @storybook/theming: ^6.5.9 => 6.5.9
    @storybook/web-components: ^6.5.9 => 6.5.9
YannDuv commented 7 months ago

Hi, I get why the override is helpful for members and properties because they live in the same "namespace" of a class. But I think we should prevent the rest from being overriden. I have this example, where I want to allow the user of my web component to pass an icon either with a name through a property or either with a custom SVG through a slot named icon. Here, I would like both option, icon the property and icon the slot, to be visible on the <ArgTypes/>. Do you think it's doable here ?

EDIT: I just saw that my point is the same as exposed in this issue : #19799