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

Expected ";" but found "}" [css-syntax-error] #3016

Closed
rejhgadellaa opened this issue Mar 23, 2023 · 4 comments
Closed

Expected ";" but found "}" [css-syntax-error] #3016

rejhgadellaa opened this issue Mar 23, 2023 · 4 comments

Comments

@rejhgadellaa
Copy link

rejhgadellaa commented Mar 23, 2023

I have a library that has the following css:

@layer rgui, app;
@layer rgui {
  @layer reset, defaults, components, utilities, important;
}
@layer rgui.components {
  @layer base;
}

I bundle the library, which uses postcss + cssnano to minify css. It removes the last semicolon (;) for every block, resulting in something like:

@layer rgui, app;
@layer rgui {
  @layer reset, defaults, components, utilities, important
}
@layer rgui.components {
  @layer base
}

(Unfortunately, cssnano doesn't seem to have an option to preserve the last semicolon. On the other hand, not having the semicolon is supported syntax AFAIK?)

When I use the library in vite (which uses esbuild to minify the css), I get the following warnings:

▲ [WARNING] Expected ";" but found "}" [css-syntax-error]

    <stdin>:1:85:
      1 │ ...components, utilities, important}@layer rgui.components{@layer b...
        │                                    ^
        ╵                                    ;


▲ [WARNING] Expected ";" but found "}" [css-syntax-error]

    <stdin>:1:120:
      1 │ ...es, important}@layer rgui.components{@layer base}@layer rgui.reset{
        │                                                    ^
        ╵                                                    ;

and the following output in my css files:

@layer rgui,
app;
@layer rgui;
@layer rgui.components;

In other words, the layers are not defined correctly, resulting in some serious styling issues.

Note: I tried disabling css minification in vite.config.js via the very recently added build.cssMinify option, and that fixes the issue, making me believe that esbuild is the cause here. If my assumption is incorrect, I'll happily file this ticket wherever it does belong (pointers to where'd that might be are appreciated!)

Thanks!


Update forgot to add version info;

vite @ 4.2.1
+-- esbuild @ 0.17.12
@evanw
Copy link
Owner

evanw commented Mar 24, 2023

I agree that what esbuild does here is wrong, and should be fixed. I checked and browsers respect this syntax so esbuild obviously should too. But I'm having a little bit of trouble figuring out exactly what is happening here.

https://drafts.csswg.org/css-syntax-3/#consume-an-at-rule says that ; is a successful parse while EOF is a parse error. So it seems correct that esbuild is reporting a parse error here.

I think perhaps the problem is that esbuild needs to implement the error-recovery part of the specification here: https://www.w3.org/TR/CSS22/syndata.html#parsing-errors. I actually wasn't aware of this part of the specification. There are parts of this algorithm that esbuild definitely doesn't do yet (e.g. automatically close unterminated string literals). I'm guessing that maybe the edge case in this issue falls under the "Unexpected end of style sheet" part.

@evanw
Copy link
Owner

evanw commented Mar 24, 2023

After investigating, it looks like this only happens when minification is enabled. So it's a minification bug instead of a parsing bug. The problem is that esbuild is incorrectly trimming this at-rule without any children. That's used to be ok until @layer was introduced. Empty @layer rules still have side-effects and cannot be trimmed.

@evanw evanw closed this as completed in c78d896 Mar 24, 2023
@rejhgadellaa
Copy link
Author

Thank you for the quick response, investigation and fix! 💖

@rejhgadellaa
Copy link
Author

Just wanted to check; The esbuild version that vitejs currently uses still prints the warnings (missing ;). Is this intended?

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