styled-components / vim-styled-components

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

Media templates are not highlighted #41

Closed dbow closed 6 years ago

dbow commented 6 years ago

This is maybe another instance related to the caveats in the README but the pattern recommended for media queries in the docs doesn't get highlighted correctly by this plugin:

screen shot 2018-05-24 at 12 06 42 pm

(the media template literals are not highlighted as CSS)

I guess it's not really highlighted correctly in the documentation either :P but it'd be aweesome if it was!

fleischie commented 6 years ago

Thank you for your patince so far...

I personally would not recommend this way of adding styles:

This is also super difficult to implement, as I would need to dynamically store at least two different names to highlight simple css rules (size-attributes, and media-query-constructor).

I'd say this is a WONTFIX for me, as I don't really see the advantages of this behavior. Sorry, if this is not the answer you had hoped for.

Thanks for contributing an issue, though. ✌️

dbow commented 6 years ago

Thanks for taking a look! It sounds like your issues are with the pattern for media queries recommended by the styled components documentation? But the issue I'm highlighting is really with nested template strings I think. Like you could simplify it down to just:

const phones = (...args: Array<any>) => css`
  @media (max-width: 768px) {
    ${css(...args)};
  }
`;

const div = styled.div`
  width: 200px;

  ${phones`
    width: 100px;
  `};
`;

and see the underlying issue is the nested template string? or, that's my guess.

This pattern is also recommended in the tips and tricks section, FWIW

i'm sure i'm not thinking of something, but couldn't you just syntax highlight any nested template strings inside of a styled-components template string as CSS? i.e.

styled.div`
  /* CSS */

  ${anything`
    /* also CSS */
  `};
`;
fleischie commented 6 years ago

Hey again.

I was just ranting previously about the "magic"-ness about it. But I see, what you mean about the more general case.

I am still torn on the "make every tagged template expression" contain css. As this is was the very first approach of this plugin and it didn't really work well.

Tell you what: I'll keep this in mind and play some time again with this idea. Let's see, whether I can make it work... Should I re-open this or don't you mind it being closes?

dbow commented 6 years ago

Oooo that makes a lot of sense :) Yeah, I can totally imagine that approach would be full of corner cases... but are there lots of corner cases once you've identified a styled components block? Like, inside a styled-components block, if there is a nested tagged template literal, assume it's CSS?

It's not a super high priority - happy to leave open or closed, whatever is easiest for you!

fleischie commented 6 years ago

Hmm, 🤔.

I will think about this approach with an open issue. 😉

fleischie commented 6 years ago

@dbow Hello again.

I had time to think about (and tinker with) the media templates, but came to the conclusion, that it's not feasible to support highlighting for this:

I hope you understand. :v: