webcomponents / custom-elements-manifest

A file format for describing custom elements
BSD 3-Clause "New" or "Revised" License
356 stars 26 forks source link

Add support for enum Values in Attributes/Properties #51

Open jogibear9988 opened 3 years ago

jogibear9988 commented 3 years ago

So the manifest should list the possible values, so a designer could use them. also a min/max for a number could be usefull

I've the following Properties for a Property in my designer: https://github.com/node-projects/web-component-designer/blob/master/src/elements/services/propertiesService/IProperty.ts

jogibear9988 commented 3 years ago

52

justinfagnani commented 3 years ago

I think that if it's valuable to have enums for attributes, it'd also be valuable for properties (and methods parameters, etc), especially considering that most custom element attributes are going to be associated with properties. So I think we should probably encode this into the Type interface... however there's a very slippery slope when dealing with types on how much we encode into the manifest directly and how much we leave to a full type system like TypeScript's.

So far we've really tried to limit how much of a type system we're building in the manifest. We obviously describe classes, and functions, but the types of parameters and fields, etc. are left up to simple string descriptions with references out to other files like .d.ts files. I think it'd be good to preserve this approach in general, but one area where it falls short is in describing interfaces, enums, and constraints that are not even encodable yet in TypeScript, but might be useful for tools.

I do think we should consider adding interfaces as a supported kind, and enums make sense to me too given their prevalence in attributes. Constraints maybe could fit within extensions... I'm not sure we'd be able to cover all of the constraints that a tool might need.

So maybe we should add a "type" declaration kind. A type could be an interface, enum, or just a type string that's referenced by name from other type fields.

A common type string definition:

{
    "kind": "type",
    "name": "OptionalFooArray",
    "text": "Array<Foo> | undefined"
}

An enum

{
    "kind": "type",
    "name": "OneOrTwoEnum",
    "values": [
      {
        "value": "one",
        "label": "One",
      },
      {
        "value": "two",
        "label": "Two",
      }
    ]
}

An interface:

{
    "kind": "type",
    "name": "",
    "members": [
      {
        "kind": "field",
        "name": "foo",
        "type": "string"
      },
      {
        "kind": "method",
        "name": "bar",
        "parameters": [
          {
            "name": "baz",
            "type": "number",
          }
        ],
        "return": {
          "type": "string"
        }
      },
    ]
}

This may look like an increase in complexity for the manifest format, but I think that an interface kind is just a subset of the class kind and if we didn't add an interface kind, people would just uses classes for that purpose. A common text type kind seems good just from a deduplication standpoint. Otherwise manifest generators will have to expand and inline complex named types, or assume that consuming tools can resolve source references.

For enums I can see how they're useful for tools like designers and storybook-like tools, but they do open a door towards more and more complex type object in this format... another option is to have the tool assume TypeScript type syntax and parse and read the type strings in the manifest. An enum would then just be 'one' | 'two' | 'three', etc.

cc @thepassle @rictic

jogibear9988 commented 3 years ago

If we would enums only in this format:

   'one' | 'two' | 'three'

It would not capture number enums, where each number would have a different meaning.

jogibear9988 commented 3 years ago

@justinfagnani

if we do this

    {
        "kind": "type",
        "name": "OneOrTwoEnum",
        "values": [
          {
            "value": "one",
            "label": "One",
          },
          {
            "value": "two",
            "label": "Two",
          }
        ]
    }

shouldn't we add a description for the enum values?

thepassle commented 3 years ago

How would a developer document this, so that tools can pick up on the enum?

jogibear9988 commented 3 years ago

i would think like this:

/**
 * BlaBla
 */
export enum aa{
  /**
   * Description 1
   */
  'abc' = 1,
  'def' = 2
}
thepassle commented 3 years ago

How would that get correctly linked to an attribute?

jogibear9988 commented 3 years ago

I don't get your question?

How is any property correctly linked to a attribute?

in lit you could use @property({type: aa})

jogibear9988 commented 2 years ago

What about a possible "Values" structure like this:

  export interface ValueDescription
  {
       name: string,
       min ?: number|string, //optional, needed?
       max ?: number|string, //optional, needed?
       values: [
              {
                    value: number|string|bigint|all js types,
                    name?: string, // a short name of the value, for example if the value is of type number, what means 1 what 2,...
                    description?: string
              }, ...
          ]
  }

and then every definition could link to such a description via the name of it

      export interface PropertyLike {
        name: string
        valueDescription?: string
      }
jfbrennan commented 1 year ago

I came here to start building a manifest for my custom elements and it seems support for documenting attributes is very limited. FWIW, I was expecting to see something like this (assume a Tooltip component):

"attributes": [
            {
              "name": "open",
              "type": "boolean",
              "description": "If present, the tooltip will be shown."
            },
            {
              "name": "pos",
              "type": "enumerated",
              "description": "Sets the position of the tooltip.",
              "values": ["top", "right", "bottom", "left"],
              "default": "top"
            }
          ]

Hopefully this shines some more light on developers' needs. Looking forward to seeing this project progress and gain widespread adoption!

jogibear9988 commented 1 year ago

@jfbrennan if you have a initial version of your manifest, you could test your package in my designer: https://github.com/node-projects/web-component-designer it should be able to load your package via the manifest

prantlf commented 1 year ago

An inspiration from Custom Data for HTML Language Service:

{
  "tags": [
    {
      "name": "holy-grail",
      "description": "Provides an extended \"Holy Grail\" layout as a web component.",
      "attributes": [
        {
          "name": "size",
          "description": "`size` {`string`} - \n\nProperty: size\n\nDefault: `null`\n\nDisables the responsive behaviour by forcing the width to be detected as \"small\", \"medium\" or \"large\".",
          "valueSet": "sizes",
          "references": [
            {
              "name": "Documentation",
              "url": "https://github.com/prantlf/holy-grail-layout#attributes"
            }
          ]
        }
      ]
    }
  ],
  "valueSets": [
    {
      "name": "sizes",
      "values": [
        {
          "name": "small",
          "description": "Force the responsive behaviour as if the screen width were \"small\" - narrower than 480px."
        },
        {
          "name": "medium",
          "description": "Force the responsive behaviour as if the screen width were \"medium\" - wider than 479px and narrower than 768px."
        },
        {
          "name": "large",
          "description": "Force the responsive behaviour as if the screen width were \"large\" - wider than 767px."
        }
      ]
    }
  ]
}
next-juanantoniogomez commented 7 months ago

What about this issue? Thanks!