:checked causing error

[benadamstyles]

Hi, I was directed here from sass/node-sass#1719 (comment)

Since upgrading to node-sass@4.x, which I think meant also upgrading to sass2scss@1.1.0, I have been getting the following error:

Error: property "checked" must be followed by a ':'

with the following code:

div
  :checked ~ p
    display: none

I can fix this with:

div
  input:checked ~ p
    display: none

However, the first syntax is valid and should work (I think!). I don't know if this bug was introduced by f03b821 but it seems related. Let me know if I can help with any further info. Thanks!

[mgreter]

Please check with http://www.sassmeister.com/ ! AFAICS it is invalid and the correct fix would be:

div
  \:checked ~ p
    display: none

From your linked issues I see you're talking about property_syntax. Can you make your example compile with official ruby sass cli? So far I'm not even sure if property_syntax will actually fix the issue!

[benadamstyles]

Ah, interesting. I guess my thinking was a little muddled – I suppose what I really meant was that it's valid CSS. I assumed that it would be supported in SASS, but you are right of course, it isn't, at least in Sassmeister. From reading your code around about here, however, it seems you do check for pseudo selectors without an accompanying element selector. Should this code not handle the above use case? If I understand correctly, sass2scss converts to SCSS "manually" so we are not actually using Sass's SASS parser anyway?

[mgreter]

Over time that logic got more and more complex. Actually the property_syntax seems to clarify some of the issues I had with understanding how the full logic works. Maybe also take a look at the sample in #26 and you might see why it is not so easy to distinguish the case you're talking about from BEM (AFAICS).

[benadamstyles]

Yeah I totally see the difficulty. I wonder if the list of conditions here already has the necessary logic. I don't think there are any pseudo selectors that overlap possible BEM prefixes such as margin, border etc. Do you think this is worth pursuing? It's not a common case, and given that :checked can only currently apply to inputs anyway, maybe it's not a big deal. It's possible that this issue would occur with other pseudo selectors, however.

[mgreter]

We follow sass logic, so in that regard it seems we behave as ruby sass does? The condition you mention is really just a hack and I would really like to remove it if possible, but I did not see another way to do it at the time I've added it. Also please see this simplified case, which gives another error message in sass:

:checked
  display: none

Results in:

Error: Properties are only allowed within rules, directives, mixin includes, or other properties.
       If ":checked" should be a selector, use "\:checked" instead.
        on line 1 of test.sass

So I don't think there is anything we can do here. You may want to suggest that directly with ruby sass, but I guess they have good reasons to do it the way they do it ...

[benadamstyles]

Yep, fair enough. It does seem strange that this has only just started causing errors – this was working fine before I upgraded from node-sass 3 to 4 – but if the behaviour has moved towards ruby sass behaviour, then I guess it's the right direction. Thanks for your time!