Use JSX.IntrinsicElements instead of custom HTMLTags and SVGTags

[devrelm]

I'm not sure if there's a good reason we're maintaining our own list of element tags, but from what I can tell, we should be able to just use the JSX.IntrinsicElements types straight from the react type defs.

This PR kills tags.d.ts in favor of using JSX.IntrinsicElements, which contains both HTML and SVG elements (and thus replaces both our HTMLTags and SVGTags.) JSX.IntrinsicElements also maps from element names to prop-types, rather than from names to element-types. This means that we will output the actual detailed props used by each individual html component rather than HTMLProps, which contains all props for all elements (i.e. href on div, which shouldn't be there.)

[mxstbr]

Waaaaiiiitttt can we use that in the JavaScript somehow too so we don't have to maintain our own whitelist?

[devrelm]

@mxstbr I don't think so, since JSX.IntrinsicElements is just a type in the react.d.ts file. It's only used for type-checking, and doesn't actually transpile into javascript.

I went searching for a physical whitelist within React's repo, but as best I can tell React itself hasn't kept a whitelist of elements since ~0.14 — maybe even earlier (see facebook/react#2746).

I was about to ask why style-components enforces a whitelist, but I guess it doesn't really look like it does. Instead it just has a list used to support the styled.tagname helpers (for styled.div, styled.button, etc.) It might be worth duplicating that list to our d.ts in case react.d.ts adds a tagname that we don't have a helper for. If we want to have one shared list for both our d.ts file and our styled.tagname helpers, then we'll have to add it as part of our build process.

[quantizor]

@mxstbr you guys don't actually need to implement a whitelist. Could implement a getter on the default styled export that inspects the requested key and does something with it. Or just do away with that pattern entirely like styled('div') which probably is better in the long run.

[kitten]

@probablyup We can't just define a getter for arbitrary input. That requires proxies and their support is not far spread enough. The polyfill requires a list of possible keys which brings us full circle. We've discussed this before and decided against it.

@mxstbr We've also discussed this point before. We could use an internal file in React with a list of all elements, but they might remove it at any time (Or have moved/removed it already? As @devrelm stated, it's gone now). This also is a problem when someone is switching to a minified bundle or does anything similar. In that case the list will be duplicated as well...

I think the only sane and realistic solution is to add it to our v3 babel plugin. I already have an idea on how to structure it so that preprocessing and non-preprocessing SC code can be used at the same time. We can then also remove the tag list from the SC code for preprocessing.

[devrelm]

@philpl @mxstbr Are there any more action items on this that I should take care of? There was a bit of discussion there and ideas proposed, but I can't tell if any decisions were made.

[kitten]

cc @styled-components/typers

[patrick91]

Looks good to me, the only issue I can see is that if the react.d.ts list gets updated but we don't update styled-components to support the new tags (or remove one) there can be some discrepancies between the typings and the actual implementation, but I don't see this happening often (also, we'd need to update our elements list when, and if, we do change the list of elements we support).

So for me it's +1

PS. When I did the initial typings I didn't know about the existence of JSX.IntrinsicElements, so that the main reason why we have our own list :)

[patrick91]

So good 👍

[Igorbek]

I like it, I wanted to have this some time ago but didn't have time to address it.

[Igorbek]

@patrick91 your concern is legit, but, to me, it is acceptable risk. Moreover, it would push people to file issues if something is not supported from the list of jsx elements. 😃

[kitten]

Thanks @devrelm for contributing and thanks @Igorbek and @patrick91 for reviewing it this quickly 👍