feat(css_parser,css_formatter): distinguish regular, custom, and dashed identifiers #1353
Conversation
✅ Deploy Preview for biomejs canceled.
|
github-actions
bot
added
A-Parser
| AnyCssValue::CssString(node) => node.format().fmt(f), | ||
| AnyCssValue::CssNumber(node) => node.format().fmt(f), | ||
| AnyCssValue::AnyCssDimension(node) => node.format().fmt(f), | ||
| AnyCssValue::CssRatio(node) => node.format().fmt(f), | ||
| AnyCssValue::AnyCssFunction(node) => node.format().fmt(f), | ||
| AnyCssValue::CssCustomProperty(node) => node.format().fmt(f), |
There was a problem hiding this comment.
CssCustomProperty is now CssDashedIdentifier and just directly wraps the IDENT token, rather than wrapping around an entire CssIdentifier node. This seems to be how most other parsers are handling this as well (SWC, LightningCSS, etc.).
Parser conformance results onjs/262
jsx/babel
symbols/microsoft
ts/babel
ts/microsoft
|
| CssVarFunction = | ||
| 'var' | ||
| '(' | ||
| property: CssCustomProperty | ||
| value: CssVarFunctionValue? | ||
| ')' | ||
|
|
||
| CssVarFunctionValue = | ||
| ',' | ||
| value: CssIdentifier |
There was a problem hiding this comment.
I removed these nodes because they aren't used anymore now that functions in general are more broadly supported (they just become CssSimpleFunction instead).
Maybe in the future we can parse known functions as distinct node types (especially if they have specific syntax), but i think a semantic analyzer will be the better place for that in most cases.
| // NOTE: The CSS Spec uses `<ident>` for the name, but also explicitly states | ||
| // that the name is case sensitive. For simplicity, `<custom-ident>` gets the | ||
| // exact same behavior. | ||
| CssPageSelector = | ||
| type: CssIdentifier? | ||
| type: CssCustomIdentifier? | ||
| pseudos: CssPageSelectorPseudoList |
There was a problem hiding this comment.
This seems to be the only case of the spec using <ident> but saying it's case sensitive. Rather than trying to handle a special case, I figure just using the <custom-ident> makes sense. They have the exact same syntax specification, the latter is just defined as case-sensitive and not a pre-defined CSS keyword.
Summary
Closes #1351. This updates the grammar to use distinct types for the different classes of identifiers, and (mostly) matches the CSS specification for their usage in things like at-rules and selectors.
In a follow up PR, I'll start adding the auto-lowercasing for all tokens/identifiers other than
CssCustomIdentifierandCssDashedIdentifier, since those are the only case-sensitive values and can now be easily implemented directly without having to thread options around.I left a TODO in here that will likely come up in future cases as well, for the
@color-profileat-rule. The name can be<dashed-ident> | device-cmyk, wheredevice-cmykis a contextual literal keyword. Usingparse_dashed_identifierright now would not allow that keyword as a value, and usingparse_custom_identifieris technically too broad (since something like 'myProfile' is not an acceptable identifier here), but I'm not sure what the best way to represent that unique union is. I tried making anAnyCssColorProfileNamethat looked likeCssDashedIdentifier | 'device-cmyk', but I don't think that really works with the current parser structure.Test Plan
All of the snapshots are updated, and the formatting tests succeed without any changes, exactly as expected here.