Feature request: fail on unset required property

[rictic]

I seem to recall lit-analyzer used to have the concept of a "required property" which was defined as a property whose type did not include undefined which was not set with an initializer. There was a rule that would diagnose when an element was used from a template without setting a required property.

Is that rule still around? If it was removed was that because of unforseen issues, or would it be a good candidate for re-adding / reimplementing?

[runem]

You are totally right, it had! :-) I disabled it because, in my experience, often I would have properties that weren't required, but without default values. The reason was also partly because it's impossible to analyze default values on elements declared with type definitions. I'm thinking about reintroducing the rule, but only for members where the @requiredjsdoc is explicitly added. What do you think about that solution?

[rictic]

The tricky thing about @required is that it's just a suggestion. There's lots of ways to create an element without specifying the fields, even within a pure LitElement app e.g. imperatively. Once you start involving other template systems or just pure HTML then there's no way at all.

I wonder if there's a softer-sounding version of @required we could give that would let people know that this isn't guaranteed. @requested? @softRequired?

[justinfagnani]

often I would have properties that weren't required, but without default values

Shouldn't those be declared as optional fields?

[runem]

@justinfagnani In my specific case these were class fields with their values set using a decorator, therefore removing undefined from the ts-type because I knew they would be set immediately (eg. in Typescript: myProp: MyType!). Analyzing this would incorrectly see it as a required field. The biggest problem I experienced appeared when analyzing ts-declaration files, because in this context we have no way of finding a default value thus incorrectly interpreting all class fields with a type that doesn't include undefined as required. A 'solution' here would be to treat all class fields found in declaration files as optional, but this could be incorrect as well. To avoid too many false positives I'm leaning towards being explicit using jsdoc.

@rictic If the owner of the element uses @required in order to explicitly state in the API that the value of a given property must be set by the consumer, I think it should be up to the consumer of the element to disable this rule if the value is set in another way than through the lit-html template. I might be misunderstanding you, but I actually think @required provides the clearest meaning in this case :-)

[matthias-ccri]

Is it feasible to just annotate the type with undefined, if lit-analyzer decided to consider non-nullable types required again?

class MyThing extends LitElement {
    @property({type: Number})
    delay?: number = 300; // using `?` yields the type `number | undefined`
    // ...
}

Inside the class, the type will be number | undefined even though it's initialized to 300. This will force you to handle the nullable case. But isn't this correct? Because the consumer could set the value to undefined at any time, since the property is indeed optional:

render(html`<my-thing .delay=${123}></my-thing>`, document.body)
render(html`<my-thing .delay=${undefined}></my-thing>`, document.body)

Let me know if this is off-base:

• if the property is indeed optional, then its type should include undefined.
• if the property is required, then its type should be non-nullable.
• if the property is initialized with a value, and you don't want a consumer to change it, then should it really be a public property? I don't know the exact use case there.
• if you want to have a true "default value" (which kicks in when the property is omitted), then the property's type should be optional, and the default value can be applied with traditional techniques (resolving the default at all usage sites), or using getters:

class MyThing extends LitElement {
    @property({type: Number})
    delay: number;
    get defaultedDelay (): number {
        return this.delay ?? DEFAULT_DELAY;
    }
...

This is messy but it's what you have to do...

A property initializer isn't a true "default value", since it goes away as soon as it's changed. It's an "initial value", not a "default value". I'm not sure if there is a valid use case for property initializers. Because, if a property is required, then you shouldn't need an initializer, and if the property is optional, then an initializer is ineffective and you should use one of the other default patterns shown above.

While @rictic is right that there are ways to bypass type restrictions, this has always been the case, especially at runtime. For my codebase, it would be nice to be able to require properties on a component at usage sites that do use lit-html and typescript.