Commit 8c77989
authored
Ensure math operators are surrounded by whitespace in arbitrary values (#20011)
This PR fixes an issue where some `calc(…)` expressions become invalid
after we canonicalize them to a different syntax.
Let's say you start with:
`left-[calc(-1*(var(--my-var1)+var(--my-var2)))]`, then the produced AST
for this candidate looks like this:
```js
[
{
"kind": "functional",
"root": "left",
"modifier": null,
"value": {
"kind": "arbitrary",
"dataType": null,
"value": "calc(-1 * (var(--my-var1) + var(--my-var2)))"
},
"variants": [],
"important": false,
"raw": "left-[calc(-1*(var(--my-var1)+var(--my-var2)))]"
}
]
```
Notice that the `+` in between the vars already contain spaces.
And the generated CSS looks like this:
```css
.left-\[calc\(-1\*\(var\(--my-var1\)\+var\(--my-var2\)\)\)\] {
left: calc(-1 * (var(--my-var1) + var(--my-var2)));
}
```
Again, the `+` has spaces aroudn it.
However, we canoncialize this syntax where we remove the `calc(-1 *
<value>)` and move the `-` in front:
`-left-[(var(--my-var1)+var(--my-var2))]`, which should behave the same,
but it did not. The parsed value for this looks like:
```js
[
{
"kind": "functional",
"root": "-left",
"modifier": null,
"value": {
"kind": "arbitrary",
"dataType": null,
"value": "(var(--my-var1)+var(--my-var2))"
},
"variants": [],
"important": false,
"raw": "-left-[(var(--my-var1)+var(--my-var2))]"
}
]
```
Notice that the `+` does not contain spaces around it. That's because we
add them when we parse arbitrary values and when we are in a `calc(…)`
expression. But the `calc(<value> * -1)` is added later, so at this
point, no spaces are added yet.
This also means that the generated CSS currently looks like:
```css
.-left-\[\(var\(--my-var1\)\+var\(--my-var2\)\)\] {
left: calc((var(--my-var1)+var(--my-var2)) * -1);
}
```
Which is invalid.
To solve this, we will make sure to add the whitespace around operators
when we re-insert the `calc(<value> * -1)`. With this fix, the CSS looks
like this:
```css
.-left-\[\(var\(--my-var1\)\+var\(--my-var2\)\)\] {
left: calc((var(--my-var1) + var(--my-var2)) * -1);
}
```
Which is correct again.
---
There are a few other issues that need a bit more work, but are not
required for this fix.
1. Can we get rid of the additional `(` and `)` parens? E.g.:
```diff
- left-[calc(-1*(var(--my-var1)+var(--my-var2)))]
- -left-[(var(--my-var1)+var(--my-var2))]
+ -left-[var(--my-var1)+var(--my-var2)]
```
Right now this means that we should use `calc((<value>) * -1)` instead
of
`calc(<value> * -1)` since `<value>` can be an expression on its own.
This
could lead to unwanted behavior, but this will be a follow up PR _if_
it's
worth it.
2. Why did we even allow this canoncialization from A to B if it's not
the same result?
This last question is a bit more scary, but it has to do with how we
compare results. We create a signature where we normalize a bunch of
values to make sure that we can compare them. As a silly example
`calc(var(--a) + var(--b))` and `calc(var(--b) + var(--a))` will result
in the same value, therefor should have the same signature and should be
swappable.
But what's happening is that the signature of
`left-[calc(-1*(var(--my-var1)+var(--my-var2)))]` and
`-left-[(var(--my-var1)+var(--my-var2))]` result in:
```
/* Signature of: left-[calc(-1*(var(--my-var1)+var(--my-var2)))] */
.x {
left: calc((var(--my-var1)+var(--my-var2))*-1);
}
/* Signature of: -left-[(var(--my-var1)+var(--my-var2))] */
.x {
left: calc((var(--my-var1)+var(--my-var2))*-1);
}
```
This gets rid of whitespace to store less data, but this is obviously
wrong now, so we need to improve the signatures around this. That said,
that will be a follow up PR as well because this requires much more
testing.
But this PR at least fixes the #20010 issue because we will properly
insert the whitespace around the math operators.
Fixes: #20010
## Test plan
1. Added a regression test to make sure that the new canonicalized
syntax results in the correct CSS. Before the fix, the test would fail:
<img width="623" height="106" alt="image"
src="https://github.com/user-attachments/assets/c470ce38-c3fa-4080-92ca-8a3e509f700b"
/>
2. Existing tests still pass1 parent b4db3b9 commit 8c77989
3 files changed
Lines changed: 13 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
| 33 | + | |
33 | 34 | | |
34 | 35 | | |
35 | 36 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1109 | 1109 | | |
1110 | 1110 | | |
1111 | 1111 | | |
| 1112 | + | |
| 1113 | + | |
| 1114 | + | |
1112 | 1115 | | |
1113 | 1116 | | |
1114 | 1117 | | |
| |||
1121 | 1124 | | |
1122 | 1125 | | |
1123 | 1126 | | |
| 1127 | + | |
| 1128 | + | |
| 1129 | + | |
| 1130 | + | |
1124 | 1131 | | |
1125 | 1132 | | |
1126 | 1133 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
24 | 24 | | |
25 | 25 | | |
26 | 26 | | |
| 27 | + | |
27 | 28 | | |
28 | 29 | | |
29 | 30 | | |
| |||
450 | 451 | | |
451 | 452 | | |
452 | 453 | | |
453 | | - | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
454 | 458 | | |
455 | 459 | | |
456 | 460 | | |
| |||
0 commit comments