postcss / sugarss

Indent-based CSS syntax for PostCSS
MIT License
708 stars 39 forks source link

Ignored line with a comment #54

Closed do7be closed 7 years ago

do7be commented 7 years ago

I found a bug. When building with a comment it seems to ignore the line, without an error.

> a // b
  font-size: normal
Jessidhia commented 7 years ago

This snippet seems to reproduce the problem:

body
  > a // comment
    font-size: normal

However, this only happens in combination with cssnano used as a plugin with sugarss as a parser. When cssnano runs, it reduces the body > a selector to (an invalid) >a; but if the comment is moved to the next line, it correctly reduces to body>a.

Something seems to be generating a not exactly correct AST?

ai commented 7 years ago
body
  > a // comment
    font-size: normal

compiles to correct:

body {
  > a /* comment */ {
    font-size: normal
  }
}

maybe we have a cssnano issue here? @ben-eb

ai commented 7 years ago

While we are thinking about issue here is a hot fix (by deleting all comments):

const postcss = require('postcss')
module.exports = postcss.plugin('no-comments', () => {
  return root => {
    root.walkComments(i => i.remove())
  }
})

Use this plugin before cssnano

Jessidhia commented 7 years ago

Interestingly, the same-line comment is not preserved in my output, even without cssnano.

This could be a potential interaction with yet another plugin, that then ultimately breaks with cssnano. 🤔

ai commented 7 years ago

@ben-eb AST for > a // comment looks correct too:

Rule {
  raws: 
   { before: '\n  ',
     selector: 
      { value: '> a ',
        raw: '> a /* comment */',
        sss: '> a // comment' } },
  type: 'rule',
  …
ai commented 7 years ago

@do7be I tested your source and everything was find in SugarSS. This input source:

> a // b
  font-size: normal

compiles to:

> a /* b */ {
  font-size: normal
}
ben-eb commented 7 years ago

@ai It's not valid CSS; a child combinator should separate two compound selectors. I think this is more of a Sass construct, which cssnano doesn't support.

https://drafts.csswg.org/selectors-4/#child-combinators