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

feat(css_parser): parse CSS-wide keywords and disallow their usage as needed #1361

Merged
merged 2 commits into from
Dec 29, 2023

Conversation

faultyserver
Copy link
Contributor

@faultyserver faultyserver commented Dec 29, 2023

Summary

Closes #1359. Part of #268.

This adds the CSS-wide keywords (including new and old ones, for now) as keywords in the parser and prevents their usage in <custom-ident>s, as defined in the spec.

There are a few exceptions to this that the spec calls out, is inconsistent about, or that we need to handle specially.

  • @page is defined with an <ident> for a name, but is case-sensitive. We use a <custom-ident> to preserve the casing more easily, but that means we need to explicitly allow the keywords, since they are allowed by the spec.
  • Class and ID names in selectors are also case-sensitive <ident>s, so we use <custom-ident> but need to allow the keywords so that things like .inherit are still valie.
  • The name for @layer is also an <ident>, but the draft spec calls out that the keywords "cause the rule to be invalid at parse time". No parsers I've found follow this behavior and just treat it like a regular <ident> accordingly. But it's noted in various places that we can come back and re-address in the future if needed.

I also used this new distinction for keywords to give slightly better error messages when a keyword is used where it's not allowed. It'll now tell you "'initial' is a CSS-wide keyword and cannot be used here" as well as saying it expected a specific kind of identifier in that spot. 

Test Plan

Updated all the snapshots that use <custom-ident> to cover cases with the keywords and ensure that they error out.

Copy link

netlify bot commented Dec 29, 2023

Deploy Preview for biomejs canceled.

Name Link
🔨 Latest commit 82474c3
🔍 Latest deploy log https://app.netlify.com/sites/biomejs/deploys/658f1eba43cd330008e5df0b

@github-actions github-actions bot added A-Parser Area: parser A-Tooling Area: internal tools L-CSS Language: CSS labels Dec 29, 2023
Copy link
Contributor

Parser conformance results on

js/262

Test result main count This PR count Difference
Total 49701 49701 0
Passed 48721 48721 0
Failed 980 980 0
Panics 0 0 0
Coverage 98.03% 98.03% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 40 40 0
Passed 37 37 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.50% 92.50% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 6322 6322 0
Passed 2036 2036 0
Failed 4286 4286 0
Panics 0 0 0
Coverage 32.20% 32.20% 0.00%

ts/babel

Test result main count This PR count Difference
Total 662 662 0
Passed 592 592 0
Failed 70 70 0
Panics 0 0 0
Coverage 89.43% 89.43% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 17646 17646 0
Passed 13452 13452 0
Failed 4192 4192 0
Panics 2 2 0
Coverage 76.23% 76.23% 0.00%

@faultyserver faultyserver force-pushed the faulty/css-wide-keywords branch from 0b1baf4 to 82474c3 Compare December 29, 2023 19:32
@faultyserver
Copy link
Contributor Author

I believe the benchmark failing is a pre-existing issue unrelated to this PR. I'll try to fix that in a separate PR since it's not part of this change.

@faultyserver faultyserver merged commit 62a7a2b into main Dec 29, 2023
18 of 19 checks passed
@faultyserver faultyserver deleted the faulty/css-wide-keywords branch December 29, 2023 19:51
@Conaclos Conaclos added the A-Changelog Area: changelog label Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Changelog Area: changelog A-Parser Area: parser A-Tooling Area: internal tools L-CSS Language: CSS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CSS Parser: CSS-wide keywords (initial, inherit, etc.)
3 participants