fix(parser): accept scroll-state(...) in @container not queries#8855
Conversation
🦋 Changeset detectedLatest commit: ead8c38 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds support for general-enclosed values inside container-query parentheses by extending the grammar union AnyCssContainerQueryInParens with an AnyCssValue alternative. The parser now recognises function-form queries (identifier followed by Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Merging this PR will not alter performance
Comparing Footnotes
|
ematipico
left a comment
There was a problem hiding this comment.
It seems the changes aren't correct because we're now parsing incorrect syntax as correct
| > 22 │ @container (width > 300px) and and (height > 200px) { | ||
| │ ^^^ | ||
| │ ^^^^^^^ | ||
| 23 │ } | ||
| 24 │ | ||
|
|
||
| i Expected one of: | ||
|
|
||
| - ( <container-query> ) | ||
| - ( <size-feature> ) | ||
| - style( <style-query> ) | ||
| - identifier | ||
| - string | ||
| - number | ||
| - dimension | ||
| - ratio | ||
| - custom property | ||
| - function |
There was a problem hiding this comment.
This seems to be a regression. We're now parsing a double and and
| 2: CSS_BOGUS@566..580 | ||
| 0: CSS_BOGUS@566..580 | ||
| 0: CSS_BOGUS@566..580 | ||
| 0: CSS_BOGUS@566..580 |
There was a problem hiding this comment.
We went from 2 bogus nodes to 4 bogus nodes. It seems we worsened the already brittle recovering
|
Restricted container-query function parsing to scroll-state(...) only and removed the generic parse_any_value fallback. All at_rule_container tests are passing. |
.changeset/loose-eyes-roll.md
Outdated
| "@biomejs/biome": patch | ||
| --- | ||
|
|
||
| Fixed [#8840](https://github.com/biomejs/biome/issues/8840). Now the Biome CSS parser correctly parses `not + scroll-state` inside `@container` queries. |
There was a problem hiding this comment.
| Fixed [#8840](https://github.com/biomejs/biome/issues/8840). Now the Biome CSS parser correctly parses `not + scroll-state` inside `@container` queries. | |
| Fixed [#8840](https://github.com/biomejs/biome/issues/8840). Now the Biome CSS parser correctly parses `not + scroll-state` inside `@container` queries. |
|
@ruidosujeira can you please fix the conflict? |
4bc93a8 to
e66c941
Compare
|
@ematipico I managed to resolve the conflicts. |
Summary
Fixes container query parsing to accept not combined with function-style queries like scroll-state(...), matching browser behavior and avoiding parser errors in @container conditions.
Fixes #8840.
Test Plan
always cargo test -p biome_css_parser --test spec_tests at_rule_container
Docs
No docs changes (parser/grammar update only).