Skip to content

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

rozsazoltan
Copy link

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 like fit-content. I believe spaces are only necessary if there's a digit either before or after the hyphen.

decodeArbitraryValue('min(fit-content,calc(100dvh-4rem))')

This way, the result of the following arbitrary value will also be correct:

<div class="min-h-[min(fit-content,calc(100dvh-4rem))]"></div>
.min-h-\[min\(fit-content\,calc\(100dvh-4rem\)\)\] {
  min-height: min(fit-content, calc(100dvh - 4rem));
}

@rozsazoltan
Copy link
Author

rozsazoltan commented Jun 4, 2025

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.

@rozsazoltan
Copy link
Author

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.
@rozsazoltan
Copy link
Author

rozsazoltan commented Jun 5, 2025

Another possible case is when there is text before it, and an operator follows it - for example: -1, or (2*3), etc.

1 - )    // illegal - not relevant
1 - (    // is possible
1 - -1   // is possible
1 - +1   // is possible
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.

1 - calc (  // illegal - not relevant
1 - calc(   // is possible
1 - theme(  // is possible (not standard)
// 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.

@rozsazoltan rozsazoltan marked this pull request as ready for review June 5, 2025 10:48
@rozsazoltan rozsazoltan requested a review from a team as a code owner June 5, 2025 10:48
@rozsazoltan rozsazoltan changed the title fix: add whitespace around math operators only when adjacent to number fix: don't break CSS keywords when formatting math expressions Jun 5, 2025
@wongjn
Copy link
Collaborator

wongjn commented Jun 5, 2025

A few other edge cases that don't work which we should perhaps consider. Mainly scientific notation and calc-size() since they may include - in their notations:

['min(-3.4e-2-var(--foo),calc-size(auto))', 'min(-3.4e-2 - var(--foo), calc-size(auto))'],
Expected: "min(-3.4e-2 - var(--foo), calc-size(auto))"
Received: "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)'
],
Expected: "clamp(-10e3 - var(--foo), calc-size(max-content), var(--foo) + -10e3)"
Received: "clamp(-10e3 - var(--foo), calc - size(max-content), var(--foo) + -10e3)"

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.

Incorrect formatting of valid hyphenated values placed inside a CSS function: fit-content becomes fit - content
3 participants