-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
[fix] do not collapse whitespace-only css vars #7303
Conversation
This should also fix the issue with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I commented on a related issue, so I could understand this PR.
And I think it's LGTM!
@@ -145,6 +145,8 @@ class Declaration { | |||
? this.node.value.children[0] | |||
: this.node.value; | |||
|
|||
if (first.type === 'Raw' && /^\s+$/.test(first.value)) return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked that the below statement is a syntax error. So this regex is ok.
div {
--foo:;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@geoffrich Perhaps it would be useful to put a code comment here as to why we check this, since this applies specifically to chrome only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added an explanatory comment, thanks for the suggestion!
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.
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.Note that 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.Before submitting the PR, please make sure you do the following
[feat]
,[fix]
,[chore]
, or[docs]
.Tests
npm test
and lint the project withnpm run lint