Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@internal by default #187

Open
nedredmond opened this issue Oct 9, 2022 · 9 comments
Open

@internal by default #187

nedredmond opened this issue Oct 9, 2022 · 9 comments

Comments

@nedredmond
Copy link

nedredmond commented Oct 9, 2022

I am using this package to create a manifest for my stencil project in order to generate docs in Storybook.

A major headache is marking every variable that is not a prop (which is already indicated by @Prop()) with the @internal or @omit decorators, or they will be treated just like props and appear as controls in the Storybook story (I know this is just as much Storybook's fault for ignoring the "private" field, among others).

I also have to mark all internal functions, including render(), which I'd think would be omitted by the framework integration plugin.

I didn't see any config options to assume @internal by default unless otherwise indicated. Is there an easy way to do this, would I need to copy the plugin and implement it myself?

Thank you for this excellent work, by the way! My issues are minimal-- I am just trying to make things as easy as possible for the developers working on my project.

@thepassle
Copy link
Member

The render method could be added to the method denylist I suppose:
https://github.com/open-wc/custom-elements-manifest/blob/master/packages/analyzer/src/features/framework-plugins/stencil/stencil.js#L6

Could you provide a code example of the other issue you mention, every variable thats not a prop? Why would you not want those documented exactly?

@nedredmond
Copy link
Author

nedredmond commented Oct 10, 2022

@thepassle, thank you for your quick response. I am going to make an effort to make the issue as clear as possible. If I am stating the obvious, I'm doing it only for the purposes of leaving nothing ambiguous, and no condescension is intended. Again, I thank you and the team for your efforts here.

My custom element has exposed fields-- props-- that are exposed on the interface and mutable by consuming web developers. My custom element also has an internal state, so I need internal variables. These are not exposed. There are also functions that I expose and internal functions that I do not.

In Storybook, I want to expose and document props that a consuming developer has access to, not my internal state. Implementation details are not relevant to a Storybook story, and it's confusing to users when those internal variables are not differentiated in any way from the API available to them.

I would think that the custom-elements-manifest is supposed to document a component's API. If I'm mistaken about that, that's at least how Storybook is interpreting it for the purposes of generating documentation. Therefore, an easy way to hide implementation details from the manifest would be very useful to users of this package.

Right now, when I run cem analyze --stencil, there is no difference between fields marked with @Prop() and those without any decorators, which is counter to my expectations. See this example stencil class:

@Component({
  tag: 'my-example',
  shadow: true,
})
export class Example {
  /** This is an exposed prop that a consumer can manipulate */
  @Prop() path: string;

  /** This is not an exposed prop. This is an internal implementation detail. */
  _URL = 'http://my-website.com/';


  /** This is not an exposed function. This is an internal implementation detail. */
  createUrl = (): string => `${this._URL}${this.path}`;

  /** Another internal detail. No reason to expose this to the consumer. */
  private classes = formatClasses(
    'font-whatever',
    'text-default',
  );

  /** Implementation detail again. */
  render() {
    return <a class={this.classes} href={this.createUrl()}>Click Me</a>;
  }
}

From this code, this custom-elements.json is generated:

{
    "schemaVersion": "1.0.0",
    "readme": "",
    "modules": [
      {
        "kind": "javascript-module",
        "path": "src/components/my-component/example.tsx",
        "declarations": [
          {
            "kind": "class",
            "description": "",
            "name": "Example",
            "members": [
              {
                "kind": "field",
                "name": "path",
                "type": {
                  "text": "string"
                },
                "description": "This is an exposed prop that a consumer can manipulate"
              },
              {
                "kind": "field",
                "name": "_URL",
                "type": {
                  "text": "string"
                },
                "default": "'http://my-website.com/'",
                "description": "This is not an exposed prop. This is an internal implementation detail."
              },
              {
                "kind": "field",
                "name": "createUrl",
                "description": "This is not an exposed function. This is an internal implementation detail."
              },
              {
                "kind": "field",
                "name": "classes",
                "privacy": "private",
                "description": "Another internal detail. No reason to expose this to the consumer."
              },
              {
                "kind": "method",
                "name": "render",
                "description": "Implementation detail again."
              }
            ],
            "attributes": [
              {
                "name": "path",
                "fieldName": "path",
                "type": {
                  "text": "string"
                }
              }
            ],
            "tagName": "my-example",
            "events": [],
            "customElement": true
          }
        ],
        "exports": [
          {
            "kind": "js",
            "name": "Example",
            "declaration": {
              "name": "Example",
              "module": "src/components/my-component/example.tsx"
            }
          },
          {
            "kind": "custom-element-definition",
            "name": "my-example",
            "declaration": {
              "name": "Example",
              "module": "src/components/my-component/example.tsx"
            }
          }
        ]
      }
    ]
}

As you can see, there is no difference between property fields and others in the analysis; however, the non-prop fields are not part of the element's API.

Finally, here is how this is interpreted by Storybook:
Screenshot from 2022-10-09 20-30-18
As you can see, every variable is added as a "Property" of the web component and given a control as if it were part of the component's exposed API, which is not the case.

Therefore, an easy way to interpret fields with @Prop() as properties and those without as @internal or @omitted would be a boon to my and my team.

Thanks again for your attention.

EDIT:
As a bonus, here's the generated documentation! The unexposed variables create a lot of noise here, making the generated documentation difficult to use.
Screenshot from 2022-10-09 20-43-16

@nedredmond
Copy link
Author

It does look like it's intended to work this way, so it's strange that it's writing all of the fields as props!

https://github.com/open-wc/custom-elements-manifest/blob/master/packages/analyzer/src/features/framework-plugins/stencil/stencil.js#L61

nedredmond added a commit to nedredmond/custom-elements-manifest that referenced this issue Oct 10, 2022
Per the conversation in issue open-wc#187, added `render` to the denylist for Stencil.
@nedredmond
Copy link
Author

I see the real issue here. The stencil extension is correctly identifying attributes from the Prop decorator. Storybook, however, is generating docs from every member instead of every attribute.

I'll go submit an issue there instead! Thanks again for all of your great work.

@nedredmond
Copy link
Author

nedredmond commented Oct 10, 2022

Is there a reason attributes are missing many of the fields that are mapped for members? That includes description and default.

Second, is is there a reason Stencil properties are mapped to attributes and not properties? Or not both?

@thepassle
Copy link
Member

Is there a reason attributes are missing many of the fields that are mapped for members? That includes description and default.

Could you post a code snippet or a playground reproduction? I dont see this properties/attributes mentioned in your earlier snippet

Stencil properties should be available both in the members and attributes array of the class

@nedredmond
Copy link
Author

Sorry this is a new thread relating to getting the expected output in Storybook from my custom elements manifest.

I am seeing that for Storybook, attributes (which are the fields I expect) are written to the argTypes dictionary, then overwritten by members, which contains all fields (including internal state).

I tried deleting members from the JSON, then realized attributes only maps name, fieldname, and type, and not the other fields I'd need in my documentation (like description and default, etc).

In order to get the docs I expect without internal fields, I copied missing keys from members to attributes, then deleted members. You can see that here: storybookjs/storybook#19414

You can see the difference in the path attribute in the snippet above. I was just wondering if there was a reason some details are left out the objects of one array and not the other.

@thepassle
Copy link
Member

Sorry, but this thread has become pretty confusing for me.

If this is about the analyzer documenting pseudo-private properties (e.g. properties that are — in the case of Stencil — not denoted with a @Prop decorator, or for example prefixed with an underscore e.g. _foo) should still be documented because they are not truly private. Only properties that are (in TS) denoted with the private keyword or (standard JS syntax) prefixed with a # are truly private, and can not be interacted with at runtime.

If you would like to still remove pseudo-private properties, you can do so with a custom plugin.

@Audie80
Copy link

Audie80 commented Apr 30, 2024

I have the same problem: CEM documents properties that are interpreted by Storybook, whereas I would only like attributes.
I use vanilla WebComponents.

My component :

/**
 * @element ux-modal
 * @class UxModalBase
 * @extends {BaseShadowComponent}
 */
export class UxModalBase extends BaseShadowComponent{
  static get customName() {
    return 'ux-modal';
  }
}

Generate CEM file :

{
              "kind": "field",
              "name": "customName",
              "static": true,
              "readonly": true,
              "inheritedFrom": {
                "name": "UxModalBase"
              }
            },

And the result in Storybook :
Capture d’écran de 2024-04-30 11-25-12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants