hudochenkov / postcss-styled-syntax

PostCSS syntax for CSS-in-JS like styled-components
MIT License
69 stars 4 forks source link

Potential support for double-slash comments. #11

Open tanner-ropp opened 1 year ago

tanner-ropp commented 1 year ago

Hello! I see in the readme that there is hesitancy to adding support for double-slash comments within styled components, but with it being a syntax error rather than a rule, it can create some roadblocks in the developer experience. Because they aren't parsed properly, the developer doesn't have an explicit explanation of what caused the error (for example the no double slash comments rule). In larger organizations where it might not be widespread knowledge that we should not be using double-slash comments in styled components, there is unclear messaging on how to move past this issue. As a workaround currently (in my own fork), I'm modifying the tokenizer to classify double slash comments the same as block comments so developers don't encounter this. Is this, or a similar solution where we can have the double-slash moments parsed normally, something you would consider merging? I'd be happy to write a PR. Thanks!

tanner-ropp commented 1 year ago

I find this messaging:

Screen Shot 2023-03-27 at 1 36 38 PM

to be more explicit and helpful than this messaging:

Screen Shot 2023-03-27 at 1 36 29 PM

let me know what you think! This package is really helpful and I would love to be able to use it in our project. Thanks again

hudochenkov commented 1 year ago

Hello! I see in the readme that there is hesitancy to adding support for double-slash comments within styled components, but with it being a syntax error rather than a rule, it can create some roadblocks in the developer experience.

Hesitance comes from few points:

  1. I don't want to spend time implementing and maintaining it.
  2. For a 4 years previous CSS-in-JS syntax for PostCSS existed I don't remember much complain about it not supporting double-slash comments.
  3. Standard CSS comments work great. While it is possible to use double-slash comments in Styled Components, I see no point of encouraging it.

Because they aren't parsed properly, the developer doesn't have an explicit explanation of what caused the error (for example the no double slash comments rule). In larger organizations where it might not be widespread knowledge that we should not be using double-slash comments in styled components, there is unclear messaging on how to move past this issue.

I agree messaging is not clear. It is comming directly from PostCSS default parser. I tried to change as minimal as possible in the parser. Because I'm not a parser expert, and I would rather rely on PostCSS authors experience. Even for future updates of a parser.

Stylelint with default parser also shows “Unknown word” error. no-invalid-double-slash-comments could detect only few cases, and only if parser could parse styles.

Is this, or a similar solution where we can have the double-slash moments parsed normally, something you would consider merging? I'd be happy to write a PR. Thanks!

I'm not sure it worth the effort to complicate parser with double-slash comments. As history showed us, it is not a big deal that they are not supported.

Also, adding support for it will make future update to parser harder. As the idea is to backport changes from PostCSS default parser back to this parser as CSS spec evolves.

mctrafik commented 1 year ago

-1 for this feature request. We are so happy this is a default rule in the checker because all minifiers strip out whitespace and if there are any // in the CSS the remainder of it gets considered a comment.

Obvious example:

body {
  color: red;
}
// override body color
body {
  color: blue;
}

gets minified to:

body{color:red;}// override body color body{color:blue;}

and as you can imagine doesn't have the desired effect.