Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSS: Properties with leading asterisks are being greedily parsed as selectors instead of as properties. #3272

Closed
zstarvit opened this issue Jul 25, 2023 · 5 comments

Comments

@zstarvit
Copy link

CSS properties with leading asterisks appear to be triggering a greedy consumption mechanism and the text from the asterisk through the end of the block or the start of the next nested block is consumed as though it were a selector or set of selectors instead of as a property.

Minimal Reproduction: https://esbuild.github.io/try/#YgAwLjE4LjE2AABlAGVudHJ5LmNzcwAuaXRlbSB7CiAgKndpZHRoOiAxMDAlOwogIGhlaWdodDogMXB4Owp9

Properties with leading asterisks aren't valid CSS for pretty much all browsers (see https://stackoverflow.com/questions/1690642/purpose-of-asterisk-before-a-css-property) however, there are many older CSS libraries that use them because they interacted with IE7 and were ignored in all other browsers.

Expected Outcome:
Anything but this, really. I'd be perfectly happy with them being omitted, being output as-is, or anything else you want to do with them. But as it is right now as soon as an asterisked property is encountered, every bit of text until the end of the block is consumed and re-written as though it were all a new selector declaration.

@evanw
Copy link
Owner

evanw commented Jul 25, 2023

This was a regression when esbuild added support for CSS nesting based on a work-in-progress specification. This happens because a non-identifier causes esbuild to parse a selector, and ; is normally considered to be a part of an invalid selector. For example, adding a top-level ; before a CSS rule will disable that following rule. This edge case you bring up is clearly an esbuild bug as this comment implies that it should still somehow be treated as an invalid declaration, not a nested selector.

Unfortunately the CSS working group is still actively changing the syntax (the last update was four days ago): w3c/csswg-drafts#8834. Ideally they will update the specification soon with the new behavior and then esbuild can just implement that. Right now the CSS nesting specification is outdated so I'm not actually sure what I'm supposed to implement here.

Chrome's source code has a flag called semicolon_aborts_nested_selector that it sets to true in CSSParserImpl::ConsumeNestedRule. So I'm guessing that maybe selector parsing is supposed to be modified such that ; is not allowed in a nested selector but is still allowed in a top-level selector.

@zstarvit
Copy link
Author

What can we do as a workaround in the meantime? Is there a configuration setting to have esbuild straight-pipe css?

In our particular case we're using less to compile all the css with @import (inline) statements, so esbuild shouldn't really have to touch the css for transformation purposes, as it's already been transformed.

@evanw
Copy link
Owner

evanw commented Jul 25, 2023

One workaround in the meantime is using the version of esbuild before CSS nesting support was introduced, which is version 0.17.9.

@evanw
Copy link
Owner

evanw commented Jul 25, 2023

I noticed you posted a comment on a linked issue. To be clear: this is an esbuild bug but probably not a spec bug. Some CSS spec somewhere probably already says why your edge case should work. For example, your edge case already works in Chrome, Firefox, and Safari even though they implement nesting. So something about what browsers implement is different than what esbuild is doing so the CSS specification that browsers are following is probably correct.

My problem is that I don't know where the right specification (or draft of that specification) is. I assume this is just because I'm not familiar with how the CSS working group works. I personally find the CSS specification process confusing because there isn't a single canonical specification like JavaScript, just seemingly different versions and/or drafts of various specifications that link to various revisions of each other. I also don't know which specification to follow or where to find the latest unpublished drafts (or even if I should be trying to do that in the first place). But this is probably just my fault.

In any case I'm going to try to have esbuild follow what the Chrome source code is doing to fix your issue in advance of the CSS nesting specification being updated.

@zstarvit
Copy link
Author

Thanks.

And yeah, I was pretty close to not posting it at all, because of course some people smarter than me have probably already seen it blow up in a test bed. But then I figured you can never be too sure, right?

For now though, 0.17.9 is working and producing correct looking output. I'll keep my eye out and upgrade back up whenever this issue is resolved though, as I don't really know what other fixes there are between 0.18.x and 0.17.9.

@evanw evanw closed this as completed in 517fd5c Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants