Refactoring and v1.0.0

[runem]

I have been spending the last couple of weeks on refactoring/improving many parts of the analyzer. Most notable, I have been improving performance, jsdoc-support and member-merging. Some of the requirements meant that I had to change core parts of the analyzer, eg. to support lazy-evaluation for performance improvements. Soon I'll be finishing up the work, push it to the branch refactor, and afterwards I will begin preparing a version 1.0.0 release.

The following is a description of what have changed. Feel free to give your feedback, thoughts or additional needs. It would be especially interesting to get some feedback on my examples of how component members are merged now.

Performance

In the past, the entire component, with all of its features, was analyzed when running the analyzer. This means, that an IDE-plugin discovering custom element tag names (eg. on startup) had to analyze all elements, which could result in performance issues. The analyzed components would often be of no use, because only a subset of components are actually used in a given source file. In order to address performance issues, I'm implementing the following 4 things in WCA:

• Lazy evaluation of component features: Features are now only analyzed when needed.
• Lazy evaluation of types: Types are now only returned and analyzed when needed.
• Specify relevant features: It's now possible to specify which features should be analyzed. For example, "methods" could be excluded.
• Caching: It's now possible to cache features per node in the inheritance tree. That means that components using the same subclass/mixin will see a big performance gain.

Library exports

In order to use WCA in the browser such as https://runem.github.io/web-component-analyzer, I will make some changes to what this library exports. Specifically, I will:

1. Separate the CLI and analyzer logic into two separate bundles (because the CLI depends on node-specific modules which can't be loaded in the browser)
2. Export both esm and commonjs modules
3. Export transformers (such as json and markdown transformers) as a part of the analyzer and not the CLI

Diagnostics and metadata

In the past, WCA was able to emit both documentation and diagnostics. In order to clean up things, I'm removing the functionality to emit diagnostics. Instead, it's now possible for WCA to add flavor-specific metadata to analyzed features that can be read programmatically. For example, the content of LitElement's @property decorator will now be available to read on the result. Thus lit-analyzer will now be able to do more things with LitElement-specific information, eg. to emit diagnostics.

JSDoc

I will look into supporting @extends, @mixes, @example, @method / @function, @typedef. I haven't started working on them, but the architecture supports it now :-) Existing supported jsdoc tags will work better because of improved merging logic.

The architecture

I have been making a lot of changes to the way components are analyzed. It's now possible for flavors to hook into much more than before in a much cleaner flow. This image should give an overview of the architecture:

Analyzing and Merging

Each flavor finds features on components. Features can be "properties", "attributes", "slot", eg. Multiple features can be emitted per property (eg. if you have both a constructor assignment and a class field that reference the same property). I'm making a lot of changes to how these features are merged.

Here are some of the highlights:

• Each feature emitted will be emitted with an priority (low, medium, high)
• Features are sorted and merged from highest to lowest priority
• For example, properties found in the constructor are "low" priority and class fields are "high" priority
• A given field on a feature (such as required) prefers the value of the first non-undefined value found
• In TS-file the type checker is preferred over the @type JSDoc. In JS-file the @type JSDoc is preferred over the type checker
• In TS-file constructor assignments are not checked (this is more aligned with what Typescript does)
• An attribute with same name as a property is always connected to the property (this might be unwanted behavior in some cases?)

Here are some examples so you can see how merging will work. They are a bit contrived, but that's on purpose because I want to show how overlapping fields and comments are merged.

Examples using Javascript

Example 1

/**
 * @element
 * @prop {"red"|"green"} foo - Test 1
 */
class MyElement extends HTMLElement {
    /**
     * Test 2
     */
    foo = "green";

    constructor () {
        super();

        /**
         * Test 3
         */
        this.foo = "red";
    }
}

Result

Prop Name	Attr Name	Type	Description	Default	Required	Visibility
foo		"red" | "green"	Test 1	"green" (*)

• The default value is not 100% correct in this case.

Example 2

/**
 * @element
 * @prop {MyType} foo
 */
class MyElement extends LitElement {

    static get properties () {
        return {
            /**
             * Test 1
             * @required
             */
            foo: { type: Object, attribute: false },
        };
    }

    constructor () {
        super();

        /**
         * Test 2
         * @type {"blue"|"red"}
         * @protected
         */
        this.foo = {}
    }
}

Result

Prop Name	Attr Name	Type	Description	Default	Required	Visibility
foo		MyType	Test 1	{}	yes	protected

Example 3

/**
 * @element
 * @prop {("blue" | "red)} foo - Test 1
 */
class MyElement extends LitElement {
    static get properties () {
        return {
            /**
             * @private
             */
            foo: { type: String, attribute: "bar" }
        }
    }

    static get observedAttributes () {
        return ["foo"];
    }

    constructor () {
        super();

        /**
         * Test 3
         * @type {string}
         * @required
         */
        this.foo = "bar 2";
    }
}

Result

Prop Name	Attr Name	Type	Description	Default	Required	Visibility
foo	bar	string	Test 1	"bar 2"	yes	private

Examples using Typescript

Example 1

/**
 * @element
 * @prop {"red"|"green"} foo - Test 1
 */
class MyElement extends HTMLElement {
    /**
     * Test 2
     */
    foo: MyType;

    constructor () {
        super();

        /**
         * This is ignored, because we are a TS-file
         * @required
         */
        this.foo = "red";
    }
}

Result

Prop Name	Attr Name	Type	Description	Default	Required	Visibility
foo		MyType	Test 1

Example 2

/**
 * @element
 */
class MyElement extends LitElement {

    foo: "red" | "green" = "green";

    static get properties () {
        return {
            /**
             * @private
             */
            foo: { type: String, attribute: "bar" }
        }
    }
}

Result

Prop Name	Attr Name	Type	Description	Default	Required	Visibility
foo	bar	"red" | "green"	Test 2	"green"		private

Example 3

/**
 * @element
 * @prop {string} foo - Test 1
 */
class MyElement extends HTMLElement {

    /**
     * Test 2
     */
    foo: number = 123;


    static get observedAttributes () {
        return ["foo"];
    }
}

Result

Prop Name	Attr Name	Type	Description	Default	Required	Visibility
foo	foo	number	Test 1	123

[hsablonniere]

Hey @runem ;-)

Great summary 🎉 👏

My feedbacks as a nested list:

• Performance 👍
• Lib exports
  • Separate the CLI and analyzer logic into two separate bundles (because the CLI depends on node-specific modules which can't be loaded in the browser) 👍 😍
• JSDoc
  • I would love to have @mixes, @example, @method and @typedef
• The architecture
  • Merge inherited component features 👍 NICE!
• Analyzing and Merging
  • Each feature emitted will be emitted with an priority (low, medium, high) 😉 clever, I love it

About JavaScript examples (sorry I can't judge TS):

Example 1:

The result table seems misaligned. I was expecting this:

Prop Name	Attr Name	Type	Description	Default	Required	Visibility
foo		"red"|"green"	Test 1	"green"

• I like the fact that for default, you used quotes for strings.
• I guess you forgot to escape the | for the markdown table 😉
• If required is not provided, should we put no?
• If visibility is not provided, should we put public?

Example 2:

• Great, I can set MyType!!
• This example extends HTMLELement, is it supposed to pick up static get properties () from LitElement?

Example 3:

• This example extends HTMLELement, is it supposed to pick up static get properties () from LitElement?
• Seems like you missed a quote after red in the example.
• I expected default to be "bar 2"

[runem]

Thank you so much for your feedback! :-)

I've now fixed the problem in the example code 👍

Add 'public' if visibility is not provided?
I think this depends on the transformer transforming the output. I left the fields empty in the tables to indicate that they are "undefined" in the output, but the markdown/json transformer could "force" public if we want to.

Below, I tried to output what WCA currently outputs using the markdown format. As you see, it only shows the "Visibility" column if one of the rows contain a non-public member. I think this works pretty well :-)

Add 'no' if required is false?
The markdown formatter will add required in the 'Default' column when member is required.

Pick up static get properties () when class doesn't extend LitElement?
At the moment the analyzer picks up static get properties () even though the class doesn't extend LitElement. I made it this way because I'm not entirely sure if I can follow all inheritance trees (eg. edge case mixins that the parser doesn't understand).

Below is actual markdown output from the analyzer on the refactor branch

example-1

Properties

Property	Type	Default	Description
foo	"red"|"green"	"green"	Test 1

example-2

Properties

Property	Visibility	Type	Default	Description
foo	protected	MyType	{}	Test 1

example-3

Attributes

Attribute
foo

Properties

Property	Attribute	Visibility	Type	Default	Description
foo	bar	private	("blue" | "red)	"bar 2"	Test 1

[hsablonniere]

Add 'public' if visibility is not provided?

I think this depends on the transformer transforming the output. I left the fields empty in the tables to indicate that they are "undefined" in the output, but the markdown/json transformer could "force" public if we want to.

Makes sens 👍

Add 'no' if required is false?

The markdown formatter will add required in the 'Default' column when member is required.

OK

Pick up static get properties () when class doesn't extend LitElement?

At the moment the analyzer picks up static get properties () even though the class doesn't extend LitElement. I made it this way because I'm not entirely sure if I can follow all inheritance trees (eg. edge case mixins that the parser doesn't understand).

I totally understand how it could be difficult. I guess users will probably rarely have this edge case. It's not worth it to invest in this.

Below is actual markdown output from the analyzer on the refactor branch

Perfect 👍 Great work @runem 👏 👏 👏 👏

I'm ready to try this with my components (I'm also looking forward for the fix about ternary operator) 😉

[runem]

I just released v0.1.21 that includes the ternary operator fix 👍

I also released a beta version of the above 🎉 It would be really helpful to me if you would like to test it on the component that you were having trouble with in #124 - thank you so much for you help so far :-)

You can test the beta version by running this:

npx web-component-analyzer@1.0.0-next.1 analyze my-element.js

[hsablonniere]

I saw the 0.1.21 so I directly started testing it and it works great!!!

• I can now remove lots of hand crafted markdown tables since the generated list of props is not polluted because of the ternary operator bug 🎉 thanks so much.
• I still have a few pb with the description not being picked up where I thought it would

But hey, I missed this message about 1.0.0-next.1 so now I know what I'm about to try ;-)

See you in a few minutes/hours...

[hsablonniere]

I'll directly try the 1.0.0-next.9 ;-)

[hsablonniere]

With 1.0.0-next.9

• I can set my custom type, that's awesome 🎉
• I can set my description in the header even if I have a getter/setter, a bug I was about to file this morning 👍
• I can set my description in the header with Object properties instead of on the static get properties 👍
• Seems like I can finally document my methods 😄
  • Seems like LitElement's render(), firstUpdated() are always picked up in the list of methods
  • I can "unlist" them with @private for now.
  • My "private underscore prefixed" methods are considered private by default 👍

Questions:

1. Should methods from LitElement be ignored automatically?
2. Is there a way to document some types about my events? How do other people do? I'd like to document the type of the CustomEvent.detail property.
3. Have you worked on the @mixes yet?

Sorry if my questions look too impatient 😄 this is great work 👍 👏

[hsablonniere]

With this:

/**
 * Some docs
 *
 * @attr {string|number} datetime - Date as ISO string or timestamp
 */
export class CcDatetimeRelative extends HTMLElement {

  static get observedAttributes () {
    return ['datetime'];
  }

  get datetime () {
    return this.getAttribute('datetime');
  }

  set datetime (value) {
    this.setAttribute('datetime', value);
  }

I get this:

| Property   | Attribute  | Type             | Description                     |
|------------|------------|------------------|---------------------------------|
| `datetime` | `datetime` | `string \| null` | Date as ISO string or timestamp |

• Seems like the markdown preview of my IDE and the notes addon from storybook don't need the escape of the | inside backticks.
• I was expecting string|number. When I remove the getter/setter, I get string|null. Should I add required to remove the null?

[hsablonniere]

BTW: I tried the color type ;-) Works great in JSON but I guess the markdown template does not use it yet.

[runem]

Thank you so much for your feedback, I really appreciate it! 🤗

string | null:
Great catch with! This was a problem where i forgot to merge the typeHint when merging an attributes with property. It should be fixed with 1.0.0-next.11 👍

LitElement-specific methods:
I'm already excluding HTMLElement/LitElement-specific methods by default. Are you including/patching your own version of LitElement, or are you using the library directly? I tried to change the exclusion-check to check for more cases of LitElement in 1.0.0-next.11, so that version might work for you 👍

Document event type:
If you are using Typescript you can use the generic detail like: new CustomEvent<number>({detail: 123}), however I'm not sure at the moment how to document events, because you can also dispatch non-custom-events (without the detail), and In that case, it wouldn't be correct to always assume that the type is the type of detail. Right now I actually support this JSDoc syntax:

/**
 * @fires {string} change - This is a change event
 */

and

/**
 * @type {string}
 */
this.dispatchEvent(new CustomEvent("my-event"));

(with 1.0.0-next.11)

This will however assume the type "string" to be the detail type which I'm not sure is the best solution (because of the above) 😊

Include type in CSS property markdown:
I forgot to include it! That's included in 1.0.0-next.11 👍

@mixes jsdoc:
I'll take a look later today to see if it's a low hanging fruit 😊

[runem]

@hsablonniere I'm not that experienced using the @mixes jsdoc, - do you have an example that I can test while developing the functionality? ☀️

[hsablonniere]

OK, so I tried with 1.0.0-next.11:

string | null:

• String|Number is fixed 👍
• It works in JSON but the markdown output has a \ before the |

LitElement-specific methods:

• FYI: I'm using the LitElement import directly (in JavaScript not with TypeScript)
• Here's the source of one of our components
• Like with our other components, the markdown output for this has the render() method.
• I never see the methods in the JSON ouput but I guess we're still deciding about that in the custom-elements.json thread
  • SPOILER ALERT: I want this :p

Document event type:

• I never thought of documenting {CustomEvent<MyType>}, I love the idea and it works well with 1.0.0-next.11.
• I think it implies the type of detail to be MyType in a good way.
• I agree with you: documenting {String} for an event should not imply it's a CustomEvent with detail as a String. It's too implicit and does not handle cases where you dispatch a non CustomEvent.
• FYI: I have a helper function to dispatch my custom events so I'll always use the @fires from JSDoc.

Include type in CSS property markdown:

• Works well, I only have one for now but it works!

@mixes jsdoc:

Ah ah, I was expecting feedbacks from you, I don't really know what I'm doing with this :p

• From what I'm reading in https://jsdoc.app/tags-mixes.html and https://jsdoc.app/tags-mixin.html I think it applies to what I'm doing with: https://github.com/CleverCloud/clever-components/blob/master/stories/mixins/with-resize-observer.docs.md
• Maybe we could open a thread about this here or elsewhere to get feedbacks from others. I think Vaadin has similar use cases.

[runem]

| in markdown:
I included the backslash for | because else it's interpreted as a the markdown table character | and the table will be malformed. If you include it in github markdown it will look fine 😊 Do you have problems in other markdown parsers where it doesn't work?

LitElement-sepcific methods:
Aha! The render method found in cc-toggle.js is not properly excluded. In 1.0.0-next.12 it should be fixed, and LitElement-specific methods will be interpreted as protected. However, I'm considering making it possible to exclude/include these methods using CLI options instead of forcing these methods to be protected 👍

@mixes jsdoc:
I will open a seperate issue on this to get some feedback

[web-padawan]

I think Vaadin has similar use cases.

We use @mixes in our code, as pointed above.
BTW I'm planning to check the new version tomorrow using these branches:

https://github.com/vaadin/vaadin-accordion/tree/next
https://github.com/vaadin/vaadin-details/tree/next

[hsablonniere]

| in markdown:

I guess there is not solution for this because:

• Markdown has no real standard which sucks (Hello AsciiDoc 😉)
• GFM breaks the table
• markdown-it used by the Storybook notes addon keeps the backslash

I say, let's keep the current behaviour, I have a plan to move away from this Storybook addon and use the JSON output at some point.

LitElement-sepcific methods:

• The fix in 12 is perfect, I only see my methods.
• 👍 for CLI params

@mixes jsdoc:

Great!!

[hsablonniere]

@web-padawan I knew you were not that far ;-)

[hsablonniere]

@runem I'm using the exact same pattern as what @web-padawan is doing with https://github.com/vaadin/vaadin-element-mixin/blob/master/vaadin-element-mixin.html

The mixin:

export function withMyMixin (ParentClass) {
  return class extends ParentClass {
    static mixinMethodFoo() {}
    mixinMethodBar() {}
  }
}

Using the mixin:

export class MyComponent extends withMyMixin(HTMLElement) {
  // ...
}

I guess the only differences are naming conventions and case sensitive code formatting ;-)

[hsablonniere]

Sorry, we should wait for a thread for this, I'll copy/paste the example...

[hsablonniere]

I just tried 1.0.0-next.13.

• I don't have any usage for @example yet but I tested it and it works well 👍
• I just discovered that my mixin is automatically discovered and put in the markdown 👍

About methods, as you know I don't use TypeScript and I was wondering how far we could go with JSDoc to document function object/array types...

This is a method on my map component:

/**
 * This description works well and is picked up by WCA
 * @param {Point[]} points - This description is ignored but I'm OK with it
 * @param {PointsOptions} options - This description is ignored but I'm OK with it
 */
addPoints (points, options = {}) {}

The thing is that I get any in the markdown output:

| Method      | Type                                     | Description                                         |
|-------------|------------------------------------------|-----------------------------------------------------|
| `addPoints` | `(points: any[], options?: any) => void` | This description works well and is picked up by WCA |

[runem]

@hsablonniere I support both your mixin pattern and what can be found in https://github.com/vaadin/vaadin-accordion 👍

@web-padawan I'm very interested in hearing if everything works for you. I tested it on https://github.com/vaadin/vaadin-accordion and chose to implement support for readonly properties such as getters likes get items(): VaadinAccordionPanel[]. It's included in 1.0.0-next.14 ☀️

I also see that the DetailsStateMixin in https://github.com/vaadin/vaadin-details looks like this, but ControlStateMixin is not picked up, because Base is not being resolved correctly by WCA. I'm on it 👍 (update: the pattern is supported in 1.0.0-next.15)

export const DetailsStateMixin = <T extends VaadinDetailsBase>(base: T): DetailsControlState => {
  const Base = ControlStateMixin(base as VaadinDetailsBase);

  class VaadinDetailsStateMixin extends Base

@hsablonniere Documenting primitive types like @param {number} points should work, but I will take a look at supporting the @param jsdoc tag :-)

Also, if I were to output param descriptions, do you have an idea for how to include it in the markdown? Here are two suggestions:

Method	Type	Description
addPoints	(points: any[], options?: any) => void	This description works well and is picked up by WCA points: Description of first parameter options: Description of second parameter

Method	Type	Description	Parameters
addPoints	(points: any[], options?: any) => void	This description works well and is picked up by WCA	points: Description of first parameter options: Description of second parameter

[hsablonniere]

@runem About function params, I would be OK with not having the param description at all but I think I prefer the first example with one description block and a list of params in the same block.

[hsablonniere]

Also, I will try the 1.0.0-next.15 because I think I have one use case of @readonly.

[hsablonniere]

I was not able to make @readonly work with JSDoc. Tried on the static get properties and the setter.

[hsablonniere]

Small bug I noticed in the last component I'm updating.

With this small test case:

export class MyComponent extends LitElement {
  firstUpdated () {
    myChartLib.init({
      fooMethod() {
        // Hello
      },
      barMethod: () => {
        // Hello
      }
    })
  }
}

The method fooMethod is picked up:

## Methods

| Method      | Type         |
|-------------|--------------|
| `fooMethod` | `() => void` |

I had an example where I used this method pattern in a config object for charts.js and the method was picked up by WCA. As you can see, when I use an arrow function, it solves the problem.

[hsablonniere]

Sorry if I pollute this thread too much but I need to thank you @runem for all your efforts, I just finished refining our docs and it's awesome... 👍

I used to have a broken list of props, I used to manually write a markdown table to fix this. Many other small details are improved now!!! 👏

Example before:

Example after:

In case you're interested in how your tool improved our codebase, here's the commit:

CleverCloud/clever-components@aca6a36

My next steps:

• Upgrade my storybook to use docs mode and the json output.
• Try to invoke wca programmatically in dev mode (Question: What is the entry point to parse a source string instead of files? #118).

[runem]

@hsablonniere It looks great! I'm really happy so see it working that well!! 🎉

Wrong methods getting picked up

Whoops, great catch! This is because I don't check that the method is a member of the class 👍 Fixed in 1.0.0-next.16

@readonly jsdoc

Sorry, forgot to mention that this only works for properties at the moment :-)

Documenting methods

I just finished improving support for methods/jsdoc in1.0.0-next.16. It was quite challenging to get this working, but I managed to solve it! It hope it works for you 👍

Now this code:

/**
  * This description works well and is picked up by WCA
  * @param {Point[]} points - This description is ignored but I'm OK with it
  * @param {PointsOptions} options - This description is ignored but I'm OK with it
  */
  addPoints (points, options = {}) {}

Will result in this table:

Method	Type	Description
addPoints	(points: Point[], options?: PointsOptions): void	This description works well and is picked up by WCA points: This description is ignored but I'm OK with it options: This description is ignored but I'm OK with it

I will soon release 1.0.0 so if we don't find any more problems during the next couple of days I plan on releasing 🎉

[hsablonniere]

It works, thanks very much @runem 👍 👏

[web-padawan]

Is the methods data also included to JSON output? I would like to use it for api-viewer.

Same question about the mixins.

[runem]

@web-padawan No, these are not included yet. We still haven't agreed on an optimal format for documenting methods and mixins, but I will include them in the format as soon as we have some more agreement on https://github.com/webcomponents/custom-elements-json/ :-)

[web-padawan]

@runem another question about the mixins: even though they are not included, can we at least get the properties implemented in mixins documented on the custom element?

E.g. I have a separate mixin for handling disabled property, etc.

[hsablonniere]

@web-padawan No, these are not included yet. We still haven't agreed on an optimal format for documenting methods and mixins, but I will include them in the format as soon as we have some more agreement on https://github.com/webcomponents/custom-elements-json/ :-)

I guess it makes sense but I think we should consider having custom extensions to the format. In OpenAPI, you can specify properties with "x-somethin".

[hsablonniere]

I may want to document the i18n keys used by my component for example.

[runem]

@web-padawan properties found on mixins are documented on the custom element, so if you don't see the disabled property declared in a mixin, it's because WCA doesn't pick up on your mixin. I'm working on supporting as many mixin-patterns as possible, so I'm very interessted in seeing the code that doesn't for work you :-)

Be aware that currently there is one big limitation in WCA which I will work on addressing after v1.0.0. The limitation is that when consuming libraries shipped without Typescript definition files, the library code is not analyzed at all (for example using a mixin that is declared in a library). This is due to how the Typescript-parser works. There is, however, a solution to this 👍

In addition, everything seems to be working fine when I try WCA on for example the next branch of vaadin-details (please correct me if I'm wrong). Here it correctly follows the mixin DetailsStateMixin to ControlStateMixin which is found in this declaration file from node_modules:

./node_modules/@vaadin/control-state-mixin/control-state-mixin.d.ts

export interface ControlStateInterface {
    autofocus: boolean;
    disabled: boolean;
    tabIndex: number | null;
    focus(): void;
    blur(): void;
}

This is the analyzed inheritance tree:

{ VaadinDetails } <-- ( DetailsStateMixin ) <-------------------------- { DetailsBase }          
                      { LitElement } <----- { ControlStateInterface }   { VaadinElement }        
                      { UpdatingElement }                               { VaadinElement_base }   
                      { HTMLElement }   

Below follows the entire output which includes properties found on mixins

vaadin-details

<vaadin-details> is a Web Component creating an expandable panel
similar to <details> HTML element.

Mixins: DetailsStateMixin, ControlStateMixin

Attributes

Attribute	Type	Description
focus-ring	Boolean	Set when the element is focused using the keyboard.
focused	Boolean	Set when the element is focused.

Properties

Property	Attribute	Type	Default	Description
autofocus		boolean
disabled		boolean
opened	opened	boolean	false	When true, the panel content is expanded and visible
tabIndex		number | null

Methods

Method	Type
blur	(): void
focus	(): void

Events

Event	Description
opened-changed	Fired when the opened property changes.

Slots

Name	Description
	Slot fot the collapsible details content.
summary	Slot for the summary content.

CSS Shadow Parts

Part	Description
content	The wrapper for the collapsible details content.
summary	The element used to open and close collapsible content.
summary-content	The wrapper for the slotted summary content.
toggle	The element used as indicator, can represent an icon.

[web-padawan]

@runem thanks, will try how it works for us.

[runem]

Version 1.0.0 is out now 🎉 I'm really grateful for all of your help! It has been very, very valuable to me 🤗

I will close this issue now, but I'm creating some follow up issue for [discussion] @mixins jsdoc and support dependencies without TS-typings.