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

Use esbuild's CSS parser #329

Merged
merged 11 commits into from
Mar 18, 2022
Merged

Use esbuild's CSS parser #329

merged 11 commits into from
Mar 18, 2022

Conversation

natemoo-re
Copy link
Member

Changes

  • Switches CSS parsing/scoping from github.com/tdewolff/parse/css to a vendor'd version of esbuild.
  • Why vendor/fork esbuild? As far as I can tell, it is the only up-to-date CSS parser in the Go ecosystem (meaning it supports nesting and other modern CSS features). With a proper public plugin system, we wouldn't have to pull this code into our codebase, but esbuild's public API is intentionally limited.
  • Closes 🐛 BUG: Nesting At-Rules break all proceeding CSS #210, closes [WIP] Allow external scoping #294
  • Note while this PR allows us to parse modern syntax like nesting, @container, and @layer, we aren't down-leveling any of these features automatically.

Testing

Tests have been updated to reflect esbuild's CSS output, which is slightly more terse.
New tests have been added to ensure we support modern syntax like nesting, @container, and @layer.

Docs

Internal change only

@changeset-bot
Copy link

changeset-bot bot commented Mar 14, 2022

🦋 Changeset detected

Latest commit: 62ecc5e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@astrojs/compiler Minor

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

styles: []string{".title.astro-DPOHFLYM{font-family:fantasy;font-size:28px;}.body.astro-DPOHFLYM{font-size:1em;}"},
styles: []string{".title.astro-DPOHFLYM{font-family:fantasy;font-size:28px}.body.astro-DPOHFLYM{font-size:1em}"},
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of small test changes like this. esbuild doesn't emit trailing ;

Comment on lines -33 to -34
p := css.NewParser(bytes.NewBufferString(n.FirstChild.Data), false)
out := ""
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of our custom handling based on tokenization is gone. This should significantly reduce the maintenance cost of CSS scoping.

esbuild's CSS AST is battle-tested and supports a lot more than the previous package.

Comment on lines +33 to +36
// Use vendored version of esbuild internals to parse AST
tree := css_parser.Parse(logger.Log{AddMsg: func(msg logger.Msg) {}}, logger.Source{Contents: n.FirstChild.Data}, css_parser.Options{MinifySyntax: false, MinifyWhitespace: true})
// esbuild's internal `css_printer` has been modified to emit Astro scoped styles
result := css_printer.Print(tree, css_printer.Options{MinifyWhitespace: true, Scope: opts.Scope})
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty much it. Rather than modify the tree AST (too complex), we modified the css_printer to handle our scoping when printing selector nodes.

Comment on lines +244 to +271
{
name: "nesting media",
source: ":global(html) { @media (min-width: 640px) { color: blue } }html { background-color: lime }",
want: "html{@media (min-width: 640px){color:blue}}html{background-color:lime}",
},
{
name: "nesting combinator",
source: "div { & span { color: blue } }",
want: "div.astro-XXXXXX{& span.astro-XXXXXX{color:blue}}",
},
{
name: "nesting modifier",
source: ".header { background-color: white; &.dark { background-color: blue; }}",
want: ".header.astro-XXXXXX{background-color:white;&.dark{background-color:blue}}",
},
{
name: "@container",
source: `@container (min-width: 200px) and (min-height: 200px) {
h1 {
font-size: 30px;
}
}`,
want: "@container (min-width: 200px) and (min-height: 200px){h1.astro-XXXXXX{font-size:30px}}",
},
{
name: "@layer",
source: "@layer theme, layout, utilities; @layer special { .item { color: rebeccapurple; }}",
want: "@layer theme,layout,utilities;@layer special{.item.astro-XXXXXX{color:rebeccapurple}}",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New tests for modern CSS draft features. There's no toggle or user setting, we just know how to parse it now.

@@ -0,0 +1,21 @@
MIT License

Copyright (c) 2020 Evan Wallace
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything below here is vendor'd from esbuild's internals, with the exception of some modifications to css_printer.

Comment on lines +321 to +448
// class selector
whitespace = canDiscardWhitespaceAfter
}
if sel.TypeSelector.Name.Text == "*" {
p.print(fmt.Sprintf(".astro-%s", p.options.Scope))
scoped = true
} else {
p.printNamespacedName(*sel.TypeSelector, whitespace)
}
switch sel.TypeSelector.Name.Text {
case "body", "html":
scoped = true
default:
if !scoped {
p.print(fmt.Sprintf(".astro-%s", p.options.Scope))
scoped = true
}
}
}

for i, sub := range sel.SubclassSelectors {
whitespace := mayNeedWhitespaceAfter

// There is no chance of whitespace between subclass selectors
if i+1 < len(sel.SubclassSelectors) {
whitespace = canDiscardWhitespaceAfter
}

switch s := sub.(type) {
case *css_ast.SSHash:
p.print("#")

// This deliberately does not use identHash. From the specification:
// "In <id-selector>, the <hash-token>'s value must be an identifier."
p.printIdent(s.Name, identNormal, whitespace)
if !scoped {
p.print(fmt.Sprintf(".astro-%s", p.options.Scope))
scoped = true
}

case *css_ast.SSClass:
p.print(".")
p.printIdent(s.Name, identNormal, whitespace)
if !scoped {
p.print(fmt.Sprintf(".astro-%s", p.options.Scope))
scoped = true
}

case *css_ast.SSAttribute:
if !scoped {
p.print(fmt.Sprintf(".astro-%s", p.options.Scope))
scoped = true
}
p.print("[")
p.printNamespacedName(s.NamespacedName, canDiscardWhitespaceAfter)
if s.MatcherOp != "" {
p.print(s.MatcherOp)
printAsIdent := false

// Print the value as an identifier if it's possible
if css_lexer.WouldStartIdentifierWithoutEscapes(s.MatcherValue) {
printAsIdent = true
for _, c := range s.MatcherValue {
if !css_lexer.IsNameContinue(c) {
printAsIdent = false
break
}
}
}

if printAsIdent {
p.printIdent(s.MatcherValue, identNormal, canDiscardWhitespaceAfter)
} else {
p.printQuoted(s.MatcherValue)
}
}
if s.MatcherModifier != 0 {
p.print(" ")
p.print(string(rune(s.MatcherModifier)))
}
p.print("]")

case *css_ast.SSPseudoClass:
p.printPseudoClassSelector(*s, whitespace)
if s.Name == "global" || s.Name == "root" {
scoped = true
}
}
}

if !scoped {
p.print(fmt.Sprintf(".astro-%s", p.options.Scope))
}

// It doesn't matter where the "&" goes since all non-prefix cases are
// treated the same. This just always puts it as a suffix for simplicity.
if sel.NestingSelector == css_ast.NestingSelectorPresentButNotPrefix {
p.print("&")
}
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function has been modified to print our scoped selectors

@natemoo-re natemoo-re self-assigned this Mar 14, 2022
@natemoo-re natemoo-re force-pushed the feat/new-css branch 2 times, most recently from 40e7612 to 10b2eac Compare March 14, 2022 17:28
@matthewp
Copy link
Contributor

lgtm

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

Successfully merging this pull request may close these issues.

🐛 BUG: Nesting At-Rules break all proceeding CSS
2 participants