option to prevent css single-line comment consumption

[sbfaulkner]

This is possibly a feature request...

We've run into some minification cases where tolerance of single-line comments in css (ie. consumeToEndOfSingleLineComment with warning) causes problems -- ie. causes css to be ignored where we'd rather the "//..." just be emitted intact since it's not a comment anyway and let the browser cope with it

Maybe this could be an option for esbuild?

[sbfaulkner]

I realize this is kinda horrific, but it's an example of what we ran into.

.owl-controls,//use styles below to disable ugly selection .customNavigation a{
  -webkit-user-select:none;
  -khtml-user-select:none;
  -moz-user-select:none;
  -ms-user-select:none;
  -webkit-tap-highlight-color:transparent;
  user-select:none
}
...

[sbfaulkner]

further context... the place we ran into this was actually css that was pre-minified.
the end-result was that everything from the // to end-of-line was dropped

I think though that the result should have been to drop to the end of the next CSS construct - either declaration or block.

so when given...

.first,// comment a{color:black}.red{background:red}
.blue{background:blue}

we currently get...

.first,.blue{background:blue}

I'm wondering if we should instead get...

.first,.red{background:red}.blue{background:blue}

browser testing (or spec-reading) needed to confirm, I suppose

[geoffharcourt]

I would definitely expect the current behavior. A one line, double-slash comment makes the rest of the line a comment. It seems like the other minification needs to make that comment have a start and end.

[hyrious]

@sbfaulkner Wait, ↓ this is the real comment part?

.first,// comment a{color:black}.red{background:red}

I don't think any css parser would handle this line as you expected.

[evanw]

Yes, I can remove the single-line comment consumption. It's not technically part of the specification so I don't think it needs to be behind an option, but I'd want to at least keep the warning. I'm also somewhat horrified that this is an issue but I understand the desire to avoid a behavior change.

so when given...

.first,// comment a{color:black}.red{background:red}
.blue{background:blue}

we currently get...

.first,.blue{background:blue}

I'm wondering if we should instead get...

.first,.red{background:red}.blue{background:blue}

I believe the correct interpretation here would be this:

.red{background:red}
.blue{background:blue}

The specification says this here:

For legacy reasons, the general behavior of a selector list is that if any selector in the list fails to parse (because it uses new or UA-specific selector features, for instance), the entire selector list becomes invalid.

But I will change esbuild to pass this through unmodified instead of attempting to interpret it so that the browser can ignore it. Trying to remove invalid CSS rules goes against the nature of CSS and could make it hard to use esbuild with possible future CSS syntax (although I agree it'd be highly unlikely for CSS to start using // as a selector given that it's a comment in lots of compile-to-CSS languages).

[sbfaulkner]

Thanks @evanw

@hyrious that was one possible interpretation at least partially based on opinions like https://www.xanthir.com/b4U10

@geoffharcourt the // is NOT a comment in css at all but error handling causes it to sort of be interpreted as one and as that link describes the error handling semantics could lead to something like what I suggested, or as Evan suggested more likely ignore the preceding selector as well as the block after

[sbfaulkner]

I believe the correct interpretation here would be this:

looks like you're 100% correct in Chrome at least, @evanw