Canonicalize negative arbitrary values#19858
Conversation
When using `calc(-1 * 2rem)`, where were taking the unit from the `lhs`. But we should be using the unit from the `rhs` instead.
Allows us to _not_ convert from an ast to a string and back
This essentially just sorts calc expressions (but only if it's still correct). This allows us to say that 2 expressions with different argument order is still the same.
This allows us to order arguments in calc expressions to ensure that 2 utilities are still considered the same. It's a separate step and not part of constant folding just because we don't really need to swap the order in the output. That might be too confusing. However, we might want to add that in the future because `mt-[calc(var(--foo)+1px)]` and `mt-[1px+calc(var(--foo))]` would do the same, but would generate 2 CSS classes.
When we use utilities like `-m-8`, we take the value of `m-8` and wrap it in a `calc(<existing> * -1)`. Depending on the value of the `-m-*`, it could be that we eventually end up with `calc(calc(<existing> * <foo>) * <bar>)`. This code allows us to try and constant fold the `<foo>` and `<bar>` parts.
This will compare the candidate AST or string based version and will do so by using the signatures to verify if they are equivalent.
This will do a few things: 1. Constant fold the expression calc expression if any 2. Try moving the `-` inside the utility. E.g. `-mt-[9px]` → `mt-[-9px]` 3. Try moving the `-` outside the utility. E.g. `mt-[calc(-1*var(--foo))]` → `-mt-(--foo)`
These handle: - `calc(x + (y + var(--unknown)))` - `calc(x * (y * var(--unknown)))` With some rules around units vs no units
WalkthroughAdds AST-based canonicalization and constant-folding for CSS 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
|
||
| // Then sort dimensions numerically, and finally by unit for ties. | ||
| if (lhsValue !== rhsValue) { | ||
| return lhsValue - rhsValue > 0 |
There was a problem hiding this comment.
A localeCompare for 2 and 10 would result in 10 2 otherwise, so we compare the actual values here.
| import { canonicalizeCalcExpressions } from './canonicalize-calc-expressions' | ||
|
|
||
| it.each([ | ||
| ['calc(-1 * var(--foo))', 'calc(var(--foo) * -1)'], |
There was a problem hiding this comment.
I don't think the order matters to much but I did:
- Move constants to the end
- When comparing values without unit, sort by the value
- When it's not a dimension (function, var, ...) then sort alphabetically (
localeCompare)
| ['-mt-[var(--my-var)]', '-mt-(--my-var)'], // Keep as-is, but convert to shorthand | ||
| ['mt-[calc(var(--my-var)*-1)]', '-mt-(--my-var)'], // Move `-` out | ||
| ['mt-[calc(-1*var(--my-var))]', '-mt-(--my-var)'], // Move `-` out | ||
| ['-mt-[calc(var(--my-var)*-1)]', 'mt-(--my-var)'], // Move `-` out | ||
| ['-mt-[calc(-1*var(--my-var))]', 'mt-(--my-var)'], // Move `-` out | ||
| ['mt-[calc(-1*calc(-1*var(--my-var)))]', 'mt-(--my-var)'], // Move `-` out | ||
| ['-mt-[calc(-1*calc(-1*var(--my-var)))]', '-mt-(--my-var)'], // Move `-` out |
There was a problem hiding this comment.
These are obviously a bit silly. But needed to make sure that we always have correct utilities. While it's silly, it's also very satisfying to see the simplification here.
| return new DefaultMap((options: SignatureOptions) => { | ||
| let signatures = designSystem.storage[UTILITY_SIGNATURE_KEY].get(options) | ||
|
|
||
| return function hasSameSignature(a: Candidate | string, b: Candidate | string): boolean { |
There was a problem hiding this comment.
We use the same thing in a few places, so wanted to abstract it.
I think a future (but very low priority) is to compute the signature of the incoming candidate ahead of time. But since we're dealing with cached values it should be fine as-is.
| return WalkAction.ReplaceSkip( | ||
| ValueParser.word(`${lhs[0] * rhs[0]}${lhs[1] ?? rhs[1] ?? ''}`), | ||
| ) |
There was a problem hiding this comment.
Added a test for this as well, but calc(2rem * 4) resulted in 8rem, but calc(4 * 2rem) resulted in 8 because we always took the unit from the lhs.
This PR adds a few more canonicalizations for some cases I noticed on our templates.
When dealing with arbitrary values, and the utility is a "negative" utility, then we will try to put the
-inside of the arbitrary value:The idea is that the arbitrary value is already an escape hatch for when a value is not available by default. The
-in front uses an implicitcalc(<expression> * -1)which might be confusion if you have an value like this already.This also can allow for some further optimizations. For example
This PR also improve the constant folding of calc expressions a bit more such that nested calc expressions with 2 constants and an unknown can be folded. Bit of a mouthful, but it allows us to handle this:
Now that we can handle moving the
-into the arbitrary value, there are also cases where we can get the-out of the arbitrary value:Another missing piece that this PR adds is the concept of canonicalizing or normalizing calc expressions. This is a separate step used when calculating the signature for each utility. This allows us to normalize
calc(-1*var(--foo))andcalc(var(--foo)*-1). Without this they would not be considered the same, but now it will.It's only used when comparing values, it won't unify the actual arbitrary values with this logic (at least for now).
With the additional constant folding logic and the canonicalization when comparing signatures it unlocks the necessary power to perform the above transformations.
Test plan
-inside the value, or move the-outside of the arbitrary value.