open-wc / custom-elements-manifest

Custom Elements Manifest is a file format that describes custom elements in your project.
https://custom-elements-manifest.open-wc.org/
234 stars 43 forks source link

Extend mixin vanilla js, missing attributes #145

Closed reno1979 closed 2 years ago

reno1979 commented 2 years ago

Checklist

Completing the items above will greatly improve triaging time of your issue.

Expected behavior When I use a mixin for retrieving a Custom Element class, and extend using a mixin. I expect the attributes from the mixin I'm extending on te become available.

Some code to demonstrate:

class MyElementA extends getClassA(HTMLElement){}
class MyElementAB extends getClassAB(MyElementA){};

function getClassA(superClass = HTMLElement){

    return class extends superClass {
        static get observedAttributes() {
            return ['a'];
        }

        set a(val) {
            this._a = val;
        }

        get a() {
            return this._a;
        }
    }
}

function getClassAB(superClass = getClassA()){

    return class extends superClass {
        static get observedAttributes() {
            return ['b'];
        }

        set b(val) {
            this._b = val;
        }

        get b() {
            return this._b;
        }
    }
}
customElements.define('my-mixin-element-a', MyElementA);
customElements.define('my-mixin-element-ab', MyElementAB);
//customElements.define('my-element-a', getClassA);
//customElements.define('my-element-ab', getClassAB);

The result has no attributes at all:

{
  "schemaVersion": "1.0.0",
  "readme": "",
  "modules": [
    {
      "kind": "javascript-module",
      "path": "src/my-element.js",
      "declarations": [
        {
          "kind": "class",
          "description": "",
          "name": "MyElementA",
          "mixins": [
            {
              "name": "getClassA",
              "module": "src/my-element.js"
            }
          ],
          "superclass": {
            "name": "HTMLElement"
          },
          "tagName": "my-mixin-element-a",
          "customElement": true
        },
        {
          "kind": "class",
          "description": "",
          "name": "MyElementAB",
          "mixins": [
            {
              "name": "getClassAB",
              "module": "src/my-element.js"
            }
          ],
          "superclass": {
            "name": "MyElementA",
            "module": "src/my-element.js"
          },
          "tagName": "my-mixin-element-ab",
          "customElement": true
        }
      ],
      "exports": [
        {
          "kind": "custom-element-definition",
          "name": "my-mixin-element-a",
          "declaration": {
            "name": "MyElementA",
            "module": "src/my-element.js"
          }
        },
        {
          "kind": "custom-element-definition",
          "name": "my-mixin-element-ab",
          "declaration": {
            "name": "MyElementAB",
            "module": "src/my-element.js"
          }
        }
      ]
    }
  ]
}

When I uncomment the last two lines of the example code, the result changes to this (and has the expected attributes)

{
  "schemaVersion": "1.0.0",
  "readme": "",
  "modules": [
    {
      "kind": "javascript-module",
      "path": "src/my-element.js",
      "declarations": [
        {
          "kind": "class",
          "description": "",
          "name": "MyElementA",
          "mixins": [
            {
              "name": "getClassA",
              "module": "src/my-element.js"
            }
          ],
          "superclass": {
            "name": "HTMLElement"
          },
          "tagName": "my-mixin-element-a",
          "customElement": true,
          "attributes": [
            {
              "name": "a",
              "inheritedFrom": {
                "name": "getClassA",
                "module": "src/my-element.js"
              }
            }
          ],
          "members": [
            {
              "kind": "field",
              "name": "a",
              "inheritedFrom": {
                "name": "getClassA",
                "module": "src/my-element.js"
              }
            }
          ]
        },
        {
          "kind": "class",
          "description": "",
          "name": "MyElementAB",
          "mixins": [
            {
              "name": "getClassAB",
              "module": "src/my-element.js"
            }
          ],
          "superclass": {
            "name": "MyElementA",
            "module": "src/my-element.js"
          },
          "tagName": "my-mixin-element-ab",
          "customElement": true,
          "attributes": [
            {
              "name": "b",
              "inheritedFrom": {
                "name": "getClassAB",
                "module": "src/my-element.js"
              }
            },
            {
              "name": "a",
              "inheritedFrom": {
                "name": "MyElementA",
                "module": "src/my-element.js"
              }
            }
          ],
          "members": [
            {
              "kind": "field",
              "name": "b",
              "inheritedFrom": {
                "name": "getClassAB",
                "module": "src/my-element.js"
              }
            },
            {
              "kind": "field",
              "name": "a",
              "inheritedFrom": {
                "name": "MyElementA",
                "module": "src/my-element.js"
              }
            }
          ]
        },
        {
          "kind": "mixin",
          "description": "",
          "name": "getClassA",
          "members": [
            {
              "kind": "field",
              "name": "a"
            }
          ],
          "attributes": [
            {
              "name": "a"
            }
          ],
          "parameters": [
            {
              "name": "superClass",
              "default": "HTMLElement"
            }
          ]
        },
        {
          "kind": "mixin",
          "description": "",
          "name": "getClassAB",
          "members": [
            {
              "kind": "field",
              "name": "b"
            }
          ],
          "attributes": [
            {
              "name": "b"
            }
          ],
          "parameters": [
            {
              "name": "superClass",
              "default": "getClassA()"
            }
          ]
        }
      ],
      "exports": [
        {
          "kind": "custom-element-definition",
          "name": "my-mixin-element-a",
          "declaration": {
            "name": "MyElementA",
            "module": "src/my-element.js"
          }
        },
        {
          "kind": "custom-element-definition",
          "name": "my-mixin-element-ab",
          "declaration": {
            "name": "MyElementAB",
            "module": "src/my-element.js"
          }
        },
        {
          "kind": "custom-element-definition",
          "name": "my-element-a",
          "declaration": {
            "name": "getClassA",
            "module": "src/my-element.js"
          }
        },
        {
          "kind": "custom-element-definition",
          "name": "my-element-ab",
          "declaration": {
            "name": "getClassAB",
            "module": "src/my-element.js"
          }
        }
      ]
    }
  ]
}

The above result is only retrieved when all 4 lines are uncommented:

customElements.define('my-mixin-element-a', MyElementA);
customElements.define('my-mixin-element-ab', MyElementAB);
customElements.define('my-element-a', getClassA);
customElements.define('my-element-ab', getClassAB);
thepassle commented 2 years ago

The reason it's not getting the attributes/members for the mixin is because the mixin is not exported. (a customElements.define 'counts' as an export in this case as well, because it makes your element globally available). If something is not exported from a module, it's essentially private, and shouldn't be added to the manifest.

For inheritance that should work a bit different though, because class A can still use a FooMixin, and the inherited members/attrs/etc should still be documented on class A.

I think it's just a matter of switching the order of the applyInheritancePlugin and the removeUnexportedDeclarationsPlugin (only remove unexported declarations after applying inheritance). I'll take a look at this today.

reno1979 commented 2 years ago

@thepassle That actually makes sense, thanks for looking into this. FYI the generated custom-elements.json will be used for Storybook (support v1.0 manifest >= v6.4.0)

thepassle commented 2 years ago

Published in v0.5.7, thanks for reporting