styled-components / webstorm-styled-components

styled-components highlighting support in IntelliJ editors
https://plugins.jetbrains.com/plugin/9997-styled-components
MIT License
375 stars 19 forks source link

Improvements to parsing, code completion, inspections #7

Closed undeadcat closed 7 years ago

undeadcat commented 7 years ago

Hi! I'm from the WebStorm team at JetBrains. Thank you for creating this plugin! We really appreciate your efforts!

We were actually planning to release a similar plugin to support Styled Components within the nearest days when we saw this plugin. We would love it if you could accept our implementation that solves some of the issues you've mentioned in the readme and join the efforts with us to make support for Styled Components in WebStorm amazing.

Here's how the implementation in this PR is different:

  1. When building injected content we replace the string interpolation argument with a fake identifier to avoid breaking the CSS syntax structure as much is possible. For example, in case of
    let someCss = styled(Box) `
    color: red;
        ${getNestedSelector()} {
        color: green
    }`;

    the current implementation would generate

    
    sel {
    color: red;
    }

sel { fakeprop: initial { color: green } initial; }

which places the elements that should be nested side-by-side, significantly changing the code semantics.
This new version generates
```css
div {
  color: red;
  EXTERNAL_FRAGMENT {
    color: green
  }
}

This keeps nested elements nested (as they should be) and also allows code completion to work correctly. image

  1. Added tests that check injected content and for https://github.com/styled-components/webstorm-styled-components/issues/6

  2. Set the minumum compatible IDE version to 2017.2 (2017.2 AND all later versions should be supported). Reason for this is there were API changes in 2017.1, I think it may be made to work with earlier versions, but not sure it's worth it without a specific need.

The code is written in Kotlin and uses intellij-gradle-plugin to build it but this is only because our code was originally written this way. I really don't have a preference here and since this is your project, I'll gladly convert it to Java or use your preferred build tool if you do =)

mxstbr commented 7 years ago

@undeadcat this is so awesome, very happy to see you're working on support!

I've invited you to the organization on GitHub, that should make collaboration easier-let me know who else to invite.

daedlock commented 7 years ago

@undeadcat that's a real game changer!!! thanks for the support. I will test it myself and release this ASAP! Thanks again.

undeadcat commented 7 years ago

@mxstbr or @daedlock, When you have time, please look over the changes to see that they make sense. I'm not going to merge this (but thanks for giving me the rights anyway!), you should check that it works and doesn't break any scenarios you intended to support.

You can add @prigara (https://github.com/prigara) She is our marketing manager, she will be able to help with documentation and how best to publish the plugin to the plugins repository

mxstbr commented 7 years ago

I don't know Kotlin or Java, so I'll defer to @daedlock for the review. Have added @prigara!

oliverturner commented 7 years ago

This is just awesome. Thanks so much @undeadcat and @daedlock!

daedlock commented 7 years ago

@undeadcat Just tested the plugin on 2017.1. It works like charm and fixes all problems in my implementation! Although there are a few of points we need to reference, some of which where mentioned in the TODO section in the readme

  1. Add support for css tagged templates. We currently support styled.*, styled(), *.extends.

  2. Smart indent closing backtick when hitting enter while the cursor is at styled.div`|.

  3. When reformatting the the document using Action Pane > Reformat Code, the CSS blocks shall be formatted and the closing backticks should be placed on the same indent level as the opening one with indent applied from users code style prefs.

Example:

class X {
    constructor() {
      const Box = styled.div`
margin: 0;
padding: 0;
`;
    }
}

When reformatted using 4-space code style scheme for JS & LESS, it should be:

class X {
    constructor() {
        const Box = styled.div`
            margin: 0;
            padding: 0;
        `;
    }
}

Do you prefer adding the top points to this PR or shall I create separate issues for them?

undeadcat commented 7 years ago

@daedlock,

  1. Sorry, i'm not sure I understand what you're saying. Do you mean to say it doesn't work for you? It works for me. image

or that it works, but shouldn't be supported? I was looking at API usage examples here : https://www.styled-components.com/docs/api and it includes css``.

  1. This isn't specific to styled-components. There is currently a limitation of the formatter in IDEA/WebStorm that injections that contain more than one part (e.g, interpolation arguments) cannot be formatted. You can see it using this js code:
//language=HTML
let withoutArguments = `
    <div>
        <div></div>
    </div>`

//language=HTML
let withoutArguments = `<div><div>${withArgument()}</div></div>`

At the moment it doesn't seem like something that can be addressed on the plugin side with the current IDE versions. We have plans to improve this starting with 2017.3 You can follow this issue

daedlock commented 7 years ago
Sorry, i'm not sure I understand what you're saying.
Do you mean to say it doesn't work for you?
It works for me.

Nevermind that! It works fine.

At the moment it doesn't seem like something that can be addressed on the plugin side with the current IDE version.
We have plans to improve this starting with 2017.3

Would be great to have that. Hopefully this would be already fixed in the next WebStorm release.

For now, I think the code looks great! Will add a release and update the README!

Thanks a lot @undeadcat for the amazing work!

prigara commented 7 years ago

Cool! Thank for merging. @daedlock I've also main some new screenshots, I will add them to the Readme later today.

mxstbr commented 7 years ago

Yessss, this is great!