styled-components / vim-styled-components

Vim bundle for http://styled-components.com based javascript files.
300 stars 24 forks source link

Improved yats compatibility #51

Closed tomaskallup closed 5 years ago

tomaskallup commented 5 years ago

I've been diging around and was able to find way to fix issues mentioned in #37

I'm unsure if this is the most elegant solution, as there is styledTypescriptPrefix defined, but the regex looks like it matches styled<Type>('div'), which is usefull, but doesn't seem to match:

// There is no styled match
styled<{ someProp: boolean}>('div')`

`;

Sadly, I have not been able to fix the example above :/

I've also been able to fix a case for css attribute defined by function:

styled.div<{ someProp: boolean }>`
    color: ${props => props.someProp ? 'black' : 'white'}
`;

EDIT: The above is invalid syntax and does not work. I've reverted that commit and I was able to fix the proper use:

styled<{ someProp: boolean }, 'div'>('div')`
    color: ${props => props.someProp ? 'green' : 'red'}
`;
fleischie commented 5 years ago

Hey @tomaskallup

Sorry for taking so long, but I was now able to look into what you have been doing lately. I think this seems to work at least for the both example files already in the repo (examples/issues-37.ts and examples/typescript.ts) and also for the amended syntax you had.

Would you say that this PR is functioning properly as you would expect? Because then I would do an actual code review. :smile:

Super duper thank you for taking the time to work on this issue! :bowing_man:

tomaskallup commented 5 years ago

Hey!

No big deal, I've been using my fork and so far it's been great. Although I haven't tested it with other typescript plugins, so that's something to be done, to make sure it doesn't break them. So yes, I would say it's working properly!

And no problem, I'm glad to help! 🙋🏻‍♂️

fleischie commented 5 years ago

@tomaskallup Is there anything I can do to help you with this PR? I can look into the regexp to match more complex interface definitions, for example.

Because what I lately experienced was a extension of an existing interface with a custom one inline:

styled<ExistingInterface & { newField: boolean }, 'h1'>('h1')`

And similarly an alternative is also not unthinkable:

styled<ExistingInterface | { alternativeInterfaceAttribute: any }, 'h1'>('h1')`
tomaskallup commented 5 years ago

@fleischie I'm sorry for not responding before, this just fell deep into my backlog, as I was using my fork and it was working fine. The regex is the main thing, as this one will be really complex 😕 So I would by glad, if you would help with that.

The problem is, we might need to match something like this:

styled<{
  someAttribute: {
    someNestedAttribute: any;
  },
  anotherAttribute: any
}, 'h1'>('h1')`css here`

Although in this case it should be extracted into a interface for readability. Maybe we should just start with the simple interface, something like your case, but alsou take into account the posibility of newlines, like in my case.

I'm not sure if this example contains every possible option, but it should be good enough for now:

styled<{ myAttribute: boolean,
  a: MyCustomType;
  b: string
}, 'h1'>('h1')

As a second example, you can use one of yours and if these match, that should give a nice starting point.

fleischie commented 5 years ago

Hey @tomaskallup I made a PR on your repo containing the improved typescript regex. Please let me know here or there whether you need help and/or I can help you.

:v:

tomaskallup commented 5 years ago

@fleischie Thanks for the regex, I believe we are now ready to merge this 👍

tomaskallup commented 5 years ago

I take that back, seems like manualy fixing conflicts messed up my repos history, I'll try to fix that.

tomaskallup commented 5 years ago

@fleischie Now we are ready 😄 I also resolved a conflict to make everything go right.