storybookjs / storybook

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

Web components argTypes mapper makes controls for internal/private members #19414

Open nedredmond opened 2 years ago

nedredmond commented 2 years ago

Describe the bug At least in the case of using @custom-elements-manifest/analyzer interpreting stencil components:

The members array documents all class members, whether they are exposed attributes/properties, internal methods, or otherwise. This line overwrites the attribute argTypes with members (categorized incorrectly(?) as properties), which creates controls for every field and method in the component, no matter what decorators are applied to the field (even "private", which is included in the manifest, is ignored here).

Here is the line: https://github.com/storybookjs/storybook/blob/ea70649db9bce2ed4ad2b1eaa13a380fb8cc8e57/code/renderers/web-components/src/docs/custom-elements.ts#L149

See the problem description and images in the issue I sent to open-wc: https://github.com/open-wc/custom-elements-manifest/issues/187

To Reproduce The repro instructions don't work for this, because sb needs to be applied to an existing stencil project. When connecting a stencil project to Storybook via a custom elements manifest like so:

import { defineCustomElements } from '../dist/esm/loader';
import { setCustomElementsManifest } from '@storybook/web-components';
import manifest from '../custom-elements.json';

defineCustomElements();
setCustomElementsManifest(manifest)

And the custom elements manifest is generated by @custom-elements-manifest/analyzer, we get all fields and functions, private/internal or not, added to the members array in addition to whatever other arrays apply-- most notably attributes, which is where exposed stencil attribute/properties (marked with @Prop() decorator) go.

Storybook then creates argTypes from attributes, then overwrites those with argTypes from members which includes everything. Then we have controls and docs for implementation details that are not exposed and should not be part of this documentation.

Storybook could just create controls from attributes and properties, but at the very least it should look for the private field to omit those from documentation.

Additional context Found a related issue: https://github.com/storybookjs/storybook/issues/15436

nedredmond commented 2 years ago

I kind of addressed this here by filtering out private fields.

However, to prevent having to deliberately mark fields as private, I added a second option that adds the fields that members have that attributes do not, then deletes members.

import { setCustomElementsManifest } from '@storybook/web-components';

export const setCustomElementsManifestWithOptions = (
    customElements: any,
    options: { privateFields?: boolean; mapMembersToAttributes?: boolean },
): void => {
    let { privateFields = true, mapMembersToAttributes } = options;
    if (!privateFields) {
        ...
    }
    if (!mapMembersToAttributes) {
        actOnDeclarations(customElements, declaration => {
            const attrs = declaration.attributes;
            const members = declaration.members;
            attrs.forEach((attr: { name: any; description: any }) => {
                const member = members.find(
                    (member: { name: any; description: any }) =>
                        member.name === attr.name,
                );
                Object.keys(member).forEach(key => {
                    attr[key] = attr[key] ?? member[key];
                });
            });
            delete declaration.members;
        });
    }
    return setCustomElementsManifest(customElements);
};

const actOnDeclarations = (
    customElements: any,
    declarationsFunction: (declaration: any) => void,
) => {
    customElements?.modules?.forEach((module: { declarations: any[] }) => {
        module?.declarations?.forEach(declarationsFunction);
    });
};