custom-elements-manifest icon indicating copy to clipboard operation
custom-elements-manifest copied to clipboard

Add support for enum Values in Attributes/Properties

Open jogibear9988 opened this issue 4 years ago • 13 comments

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 avatar May 08 '21 20:05 jogibear9988

#52

jogibear9988 avatar May 09 '21 07:05 jogibear9988

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

justinfagnani avatar May 27 '21 19:05 justinfagnani

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 avatar May 27 '21 19:05 jogibear9988

@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?

jogibear9988 avatar May 27 '21 19:05 jogibear9988

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

thepassle avatar May 30 '21 08:05 thepassle

i would think like this:

/**
 * BlaBla
 */
export enum aa{
  /**
   * Description 1
   */
  'abc' = 1,
  'def' = 2
}

jogibear9988 avatar May 30 '21 08:05 jogibear9988

How would that get correctly linked to an attribute?

thepassle avatar May 30 '21 08:05 thepassle

I don't get your question?

How is any property correctly linked to a attribute?

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

jogibear9988 avatar May 30 '21 08:05 jogibear9988

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
      }

jogibear9988 avatar Feb 13 '22 08:02 jogibear9988

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!

jfbrennan avatar Oct 17 '22 16:10 jfbrennan

@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

jogibear9988 avatar Oct 18 '22 09:10 jogibear9988

An inspiration from Custom Data for HTML Language Service:

  • Type is separate from, the enum values.
  • The enum values are specified by a reference to a separate valueSet object to avoid duplication, if the same values are used for another element's attribute.
{
  "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."
        }
      ]
    }
  ]
}

prantlf avatar Apr 05 '23 08:04 prantlf

What about this issue? Thanks!

next-juanantoniogomez avatar Feb 15 '24 11:02 next-juanantoniogomez