Highlighting stops working when generic is used

[CarloPalinckx]

The highlighting seems to top working if a generic is applied. The docs show the same issue, watch how "color" is green in the lower example.

[fleischie]

Hey @CarloPalinckx , thanks for the report.

Yes, that indeed does not highlight correctly. I guess the next version that is mentioned in the docs is supported, though. And thus I can recommend the following alternative as a quick-fix:

const Title = styled<{ isActive: boolean }>('h1')`
  color: ${props => props.isActive ? props.theme.primaryColor : props.theme.secondaryColor};
`;

I will try to make an estimation of how much extra work it would take to support both versions.

[fleischie]

Although as I think about it, do you happen to know what the pros/cons/differences between the two documented versions are?

I can make it work fairly easy™ (I guess, it's just a matter of adding a matching rule changing the order of the (Component) and <Interface> sections). But I'm afraid as I don't know all the details of the topic I might miss something or worse introduce regressions.

So if you might be able to elaborate I'd be super duper appreciative. If not, that's ok as well I can ask around/investigate further. Just let me know how much you want me to include you in the issue-resolvement. 😉

[CarloPalinckx]

There should be no difference between the two documented versions. IE:

styled.h1<Foo>``

and

styled('h1')<Foo>``

However,

styled<Foo>('h1')`

Is something different :) since placing the generic type there does not enrich the resulting type with Foo but rather overwrites the inference for "h1"

styled<Foo>('h1')``

results in a styled component with type Foo compared to:

styled.h1<Foo>``

which should result in a component with HTMLDivElement & Foo.

TL:DR, they're different 😄

[fleischie]

OK. It was a long day and maybe I'm not as receptive as I need to be, but are you implying that both are viable to use but only the interface-last syntax is the one we want to use to create styled-templated-props-components?

I mean I'm just working on an editor plugin to highlight the syntax correctly but erm, should I just support both versions?

😪

[CarloPalinckx]

styled('h1')<Foo>``

and

styled.h1<Foo>``

Are the ones that aren't supported yet. And they are both valid styled-components 🙂

[fleischie]

@CarloPalinckx, can you do me the favor and check the latest develop branch? I did the changes I previously mentioned.

Let me know, if I can do anything to improve something. :v:

NOTE: What makes you think the coloring in the above mentioned docs were done using vim (just curious). And that color is colored differently (oh the irony) is somewhat out of my control. Somewhere the css definition changes cssTextProp to be of different meaning and not take on the default cssTextProp -> cssProp -> StorageClass syntax classification.

[CarloPalinckx]

I don't think it's vim tbh, but the same issue happens like in the screenshots above. So I figured it would be the best example for the issue 😅

I'm pretty new to vim, how can I check out a specific branch (with Plug)?

[fleischie]

Ah, ok. No worries.

Type in Plug 'styled-components/vim-styled-components', { 'branch': 'develop' } to use the develop branch specifically (and not the main branch).

[fleischie]

I also just now realized there are additional interface definition possibilities (indexable types and optional attributes) and hence have updated the branch. Just fyi. :+1:

[CarloPalinckx]

I still seem to be getting some issues, I get typescriptStringB as a result when checking the syntax type on "display" for instance/

[fleischie]

Yeah. Whoops, I forgot about named interfaces and had a local change to support this already but totally forgot to update the remote branch.

I updated develop now, the above-mentioned version should work better now (but probably not perfect because I think I found even more edge cases I missed). Thanks for the continued feedback.

Edit: The edge-cases I mentioned were:

• styled.div.attrs({ some: somesome })<Interface | { bool: boolean }>
• styled(Button).attrs({ role: 'presentation' })<{ tmp?: number } & Interface>
• etc.

I need to take more time to investigate more edge-cases, probably as well.

[fleischie]

I also added support now to the extended API for typescript as mentioned in the 👆 comment:

• .attrs()
• .extend (which is deprecated but who am I to judge?)

[CarloPalinckx]

This is awesome man 😎

[CarloPalinckx]

I guess this can be closed then right? 🙂

[fleischie]

Oh yes, I completely forgot, that I wanted to release that change. Yes, closing is cool! :v: