Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

customElement decorator should use type string for tagName #291

Closed
sorvell opened this issue Nov 5, 2018 · 8 comments
Closed

customElement decorator should use type string for tagName #291

sorvell opened this issue Nov 5, 2018 · 8 comments

Comments

@sorvell
Copy link
Member

sorvell commented Nov 5, 2018

This is currently keyof HTMLElementTagNameMap but this doesn't work nicely in TypeScript right now and the result is you have to cast to any. Instead we should just use string for now.

@sorvell sorvell added this to the 0.7.0 milestone Nov 5, 2018
@sorvell sorvell self-assigned this Nov 5, 2018
sorvell pushed a commit that referenced this issue Nov 5, 2018
@jhpratt
Copy link

jhpratt commented Nov 25, 2018

Seconding this. I just ran into this issue while testing out lit element on an existing component.

@nrdobie
Copy link

nrdobie commented Nov 28, 2018

It looks like the issues is that TypeScript is converting keyof HTMLElementTagNameMap to "object" | "a" | "abbr" | "acronym" | "address" | "applet" | "area" | "article" | "aside" | "audio" | "b" | "base" | "basefont" | "bdo" | "big" | "blockquote" | "body" | "br" | "button" | "canvas" | "caption" | "center" | "cite" | "code" | "col" | "colgroup" | "data" | "datalist" | "dd" | "del" | "dfn" | "dir" | "div" | "dl" | "dt" | "em" | "embed" | "fieldset" | "figcaption" | "figure" | "font" | "footer" | "form" | "frame" | "frameset" | "h1" | "h2" | "h3" | "h4" | "h5" | "h6" | "head" | "header" | "hgroup" | "hr" | "html" | "i" | "iframe" | "img" | "input" | "ins" | "isindex" | "kbd" | "keygen" | "label" | "legend" | "li" | "link" | "listing" | "map" | "mark" | "marquee" | "menu" | "meta" | "meter" | "nav" | "nextid" | "nobr" | "noframes" | "noscript" | "ol" | "optgroup" | "option" | "output" | "p" | "param" | "picture" | "plaintext" | "pre" | "progress" | "q" | "rt" | "ruby" | "s" | "samp" | "script" | "section" | "select" | "slot" | "small" | "source" | "span" | "strike" | "strong" | "style" | "sub" | "sup" | "table" | "tbody" | "td" | "template" | "textarea" | "tfoot" | "th" | "thead" | "time" | "title" | "tr" | "track" | "tt" | "u" | "ul" | "var" | "video" | "wbr" | "xmp" for the declaration file instead of keeping the keyOf. Probably just an incorrect setting in TypeScript.

@jhpratt
Copy link

jhpratt commented Nov 28, 2018

@nrdobie That's because that long list is what keyof HTMLElementTagNameMap is equivalent to. That's not a wrong setting at all; lit-element currently requires you to manually extend the interface. I don't think that should be necessary, and switching to string would suffice (rather than casting to any).

@nrdobie
Copy link

nrdobie commented Nov 28, 2018

@jhpratt what I meant by incorrect setting was TypeScript reducing it down to that list of strings rather then preserving the keyof statement. Based on the current expected behavior of lit-element, preserving the keyof would make the behavior work as expected. Changing to string does also work as well but also changes the behavior.

@dorivaught dorivaught modified the milestones: 0.7.0, 1.0.0 Nov 29, 2018
@legowerewolf
Copy link

Just ran into this, would be a nice fix. @nrdobie how would changing to string change the behavior?

@nrdobie
Copy link

nrdobie commented Dec 7, 2018 via email

@vladnicula
Copy link

vladnicula commented Dec 12, 2018

Can someone explain the reasoning for having keyof HTMLElementTagNameMap there? I don 't fully understand why this wasn't a string in the first place.

@sorvell
Copy link
Member Author

sorvell commented Dec 12, 2018

Fixed via #292.

@sorvell sorvell closed this as completed Dec 12, 2018
acarapetis added a commit to acarapetis/curve-shortening-demo that referenced this issue May 27, 2019
This cast was previously necessary for weird reasons: see
lit/lit-element#291
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants