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

Empty CSS variables are incorrectly minified #7288

Closed
adamwathan opened this issue Feb 19, 2022 · 5 comments
Closed

Empty CSS variables are incorrectly minified #7288

adamwathan opened this issue Feb 19, 2022 · 5 comments
Labels
bug compiler Changes relating to the compiler

Comments

@adamwathan
Copy link

Describe the bug

This is valid CSS:

.my-class {
  --my-var: ;
}

Importantly, the value of the variable is a single space character. SvelteKit is minifying this CSS (in development and production) to this:

.my-class {
  --my-var:;
}

This is syntactically invalid and breaks the semantics of the original CSS.

Unfortunately this breaks compatibility with Tailwind CSS, as we rely on empty variables to make certain types of styles composable, which you can read about in this blog post if would like a deeper explanation: https://adamwathan.me/composing-the-uncomposable-with-css-variables/

esbuild used to have the same bug but it was fixed a long time ago: evanw/esbuild#760

I've confirmed that esbuild is minifying this code safely:

/* Input */
.a {
  --blur: ;
}

/* Output */
.a{--blur: }

...so I'm not sure where in the chain this is happening.

Reproduction

Here's a basic SvelteKit project that demonstrates the issue:

https://github.com/adamwathan/sveltekit-css-minification-bug/blob/main/src/routes/index.svelte

Logs

No response

System Info

System:
    OS: macOS 12.0
    CPU: (10) arm64 Apple M1 Max
    Memory: 1.34 GB / 64.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.13.0 - /var/folders/7p/9j_d6ys502scd371yx3nwfkm0000gn/T/fnm_multishells/57296_1645277079681/bin/node
    Yarn: 1.22.17 - /var/folders/7p/9j_d6ys502scd371yx3nwfkm0000gn/T/fnm_multishells/57296_1645277079681/bin/yarn
    npm: 8.1.0 - /var/folders/7p/9j_d6ys502scd371yx3nwfkm0000gn/T/fnm_multishells/57296_1645277079681/bin/npm
  Browsers:
    Brave Browser: 98.1.35.100
    Chrome: 98.0.4758.102
    Chrome Canary: 101.0.4897.0
    Firefox: 94.0.2
    Safari: 15.0
    Safari Technology Preview: 15.4
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.17
    @sveltejs/kit: next => 1.0.0-next.278
    svelte: ^3.46.0 => 3.46.4

Severity

blocking all usage of SvelteKit

Additional Information

No response

@dominikg
Copy link
Member

likely a bug in svelte itself, see css output tab in this repl https://svelte.dev/repl/06b3ee703d9542abb7a8bc35ff301f4b?version=3.46.4

@bluwy bluwy transferred this issue from sveltejs/kit Feb 19, 2022
@bluwy
Copy link
Member

bluwy commented Feb 19, 2022

It also looks like a Chrome only bug. Safari and Firefox renders fine. (Reminds me of this esbuild release)

@bluwy bluwy added bug compiler Changes relating to the compiler labels Feb 19, 2022
@geoffrich
Copy link
Member

From my research in an earlier issue, this is technically a Chrome bug and will be fixed in the next release (99): #7152 (comment)

@adamwathan
Copy link
Author

Looks like the change to the spec that makes --foo:; and --foo: ; equivalent is quite recent (in terms of spec changes anyways).

Personally I think it would be best to change this in Svelte regardless for better compatibility, it's a change lots of other tools have had to make as well since historically --foo:; and --foo: ; were not equivalent.

This is what the spec said last time I looked a couple of months ago:

Note: While must represent at least one token, that one token may be whitespace. This implies that --foo: ; is valid, and the corresponding var(--foo) call would have a single space as its substitution value, but --foo:; is invalid.

...so I wouldn't be surprised if this sneaky problem shows up a lot for a while. It'll be a while before Electron is on 99 for example I bet.

@adamwathan
Copy link
Author

I'm going to close this in favor of #7152 regardless though, thanks for pointing me there!

dummdidumm pushed a commit that referenced this issue Mar 3, 2022
Fixes #7152, see also #7288

--foo:; used to be an invalid CSS custom property value, while -foo: ; was valid. By collapsing the whitespace in these declaration values, we were breaking scenarios where an empty custom property was desired. The spec was updated to trim whitespace and treat these values identically, but Chromium browsers still treat --foo; as invalid. This was recently fixed and will be released in Chrome 99, but this would still be a good fix to maintain backwards compatibility.
himanshiLt pushed a commit to himanshiLt/svelte that referenced this issue Mar 3, 2022
Fixes sveltejs#7152, see also sveltejs#7288

--foo:; used to be an invalid CSS custom property value, while -foo: ; was valid. By collapsing the whitespace in these declaration values, we were breaking scenarios where an empty custom property was desired. The spec was updated to trim whitespace and treat these values identically, but Chromium browsers still treat --foo; as invalid. This was recently fixed and will be released in Chrome 99, but this would still be a good fix to maintain backwards compatibility.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug compiler Changes relating to the compiler
Projects
None yet
Development

No branches or pull requests

4 participants