runem / lit-analyzer

Monorepository for tools that analyze lit-html templates
MIT License
317 stars 36 forks source link

[vscode-lit-plugin] html `autocomplete` attribute type errors with type `string` #267

Open michaelwarren1106 opened 2 years ago

michaelwarren1106 commented 2 years ago

These days, the current Typescript type for the autocomplete property on HTMLInputElement is string;

https://github.com/microsoft/TypeScript/blob/main/lib/lib.dom.d.ts#L7033

However, when I create a lit-html template in a component such as:

@property({ type: String}) autocomplete: string;

render(){
  <input autocomplete=${this.autocomplete} />
}

VSCode shows a type error from lit-plugin about type string being incompatible with type "on" | "off" | "address1" | ... and a hundred more....

I dug into this a bit, and it seems like this is because lit-plugin is using a pre-compiled/bundled version of Typescript and not the one from VSCode itself OR from the project you're in. The bundle.js file in the extensions folder for VSCode shows the names of the actual allowed types for autocomplete and its every single string.

image

Technically, the type in lit-plugin is correct. However, in practice, components that need to pass this property down to a child native input would just use string since there are so many allowed types for autocomplete. Having to manually create the union type (since no intellisense actually displays the whole list anywhere) is a pain, and not really worth the development effort. string should be an allowed type for autocomplete

Would it be possible to allow one of the following to be true:

@claviska and I have been getting red squiggle type errors in VSCode for a while and would like to be able to resolve them without ts-ignore

43081j commented 2 years ago

Much of this is generated from a data set we then infer our types from.

In this case and a couple others (e.g. target), I agree we should loosen the type and allow any string.

The plugin does use your project's typescript iirc but the attribute types are not related to the property types. We provide the types for known attributes via a data set Microsoft or someone maintains, while the properties are standard typescript types published in typescripts dom lib.

I'll have a look at it if I can find time soon

michaelwarren1106 commented 2 years ago

is there a way that the types from Typescripts own lib.dom.d.ts can be used? does the analyzer plugin have to bundle a separate dataset for type inference?

43081j commented 2 years ago

they shouldn't really be interchangeable. the type of a property isn't necessarily the same type as its attribute (since all attributes are strings).

it is a widely used data set actively maintained and is accurate, we don't maintain it and don't change it (without looking i can't remember who does, probably microsoft/vscode).

the solution to this and any other inconsistency with the attribute types is to add another exception on our end like we do with target. we override the official data set to loosen the type because the one defined in the data set is too strict.