-
Notifications
You must be signed in to change notification settings - Fork 4.6k
fix: don't break CSS keywords when formatting math expressions #18220
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
base: main
Are you sure you want to change the base?
Conversation
Inside CSS functions, whitespace is added before and after every math operator; I believe this should only be necessary if there is a number before or after the operator. A number or another operator or function? Or maybe it's easier to just list the exceptions. I'll leave it as a draft for now. |
decodeArbitraryValue('min(fit-content,calc(100dvh-4rem))')
decodeArbitraryValue('min(fit-content,calc(100dvh-4rem)-calc(50dvh-2px))') Now: min(fit-content, calc(100dvh - 4rem))
min(fit-content, calc(100dvh - 4rem)-calc(50dvh - 2px)) And if, in addition to checking for numbers, we also check whether a math function follows it: let rest = (input.slice(i + 1) || '').trimEnd()
let next = input[i + 1] || ''
// ...
// If there's no number before or after it and it's not followed by a math function, don't add spaces.
else if (
next.length > 0 &&
!/\d/.test(prev + next) &&
!MATH_FUNCTIONS.some(fn => rest.startsWith(fn + '('))
) {
result += char
continue
} Will be: min(fit-content, calc(100dvh - 4rem))
min(fit-content, calc(100dvh - 4rem) - calc(50dvh - 2px)) |
There can be no case that would cause concern about the operator being followed by no character.
Another possible case is when there is text before it, and an operator follows it - for example: -1, or (2*3), etc.
next !== '(' && next !== '+' && next !== '-' Also, when dealing with functions, we need to consider that formatting is not only required for standard functions, but actually for anything that looks like a function.
// Original for only CSS functions
!MATH_FUNCTIONS.some(fn => rest.startsWith(fn + '('))
// New for standard function declaration
!/^[a-zA-Z_$][a-zA-Z0-9_$]*\(/.test(input.slice(i + 1)) I believe it's unnecessary to declare every single case, since introducing a new function should come with updating this accordingly. Also, this way the process can work better with custom syntax as well. |
A few other edge cases that don't work which we should perhaps consider. Mainly scientific notation and ['min(-3.4e-2-var(--foo),calc-size(auto))', 'min(-3.4e-2 - var(--foo), calc-size(auto))'],
[
'clamp(-10e3-var(--foo),calc-size(max-content),var(--foo)+-10e3)',
'clamp(-10e3 - var(--foo), calc-size(max-content), var(--foo) + -10e3)'
],
|
Fixes #18219
Summary
In an arbitrary value, if there's a non-numeric character both before and after a hyphen, there's no need for a space.
Test plan
decodeArbitraryValue
will correctly format special CSS values likefit-content
. I believe spaces are only necessary if there's a digit either before or after the hyphen.This way, the result of the following arbitrary value will also be correct: