From ad5d6eb7cb306c4fe90054d0688027caabdfd59b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 18 Aug 2026 20:27:29 +0000 Subject: [PATCH 1/2] css: keep the lower bound when simplifying clamp() When the center of a clamp() was not above the maximum, the clamp() arm removed the minimum instead of the maximum, so clamp(10px, 5px, 20px) printed min(20px, 5px) and resolved to 5px instead of 10px. The arm now applies the maximum, then the minimum: a comparable minimum folds the result to one value, an incomparable one is kept as max(min, center), and a center that cannot be compared with the maximum leaves the clamp() unchanged, as before. --- src/css/values/calc.rs | 56 +++++++++++++++---------------------- test/js/bun/css/css.test.ts | 35 +++++++++++++++++++++++ 2 files changed, 58 insertions(+), 33 deletions(-) diff --git a/src/css/values/calc.rs b/src/css/values/calc.rs index 414f8c40941c..7a3dbff055d3 100644 --- a/src/css/values/calc.rs +++ b/src/css/values/calc.rs @@ -340,48 +340,30 @@ impl Calc { Ok(Calc::Function(Box::new(MathFunction::Max(reduced)))) } CalcUnit::Clamp => { - let (mut min, mut center, mut max) = input.parse_nested_block(|i| { + let (min, mut center, max) = input.parse_nested_block(|i| { let min = Self::parse_sum(i, parse_ident)?; i.expect_comma()?; let center = Self::parse_sum(i, parse_ident)?; i.expect_comma()?; let max = Self::parse_sum(i, parse_ident)?; - Ok((Some(min), center, Some(max))) + Ok((min, center, max)) })?; - // According to the spec, the minimum should "win" over the maximum if they are in the wrong order. - let cmp = if let (Some(mx), Calc::Value(cv)) = (&max, ¢er) { - if let Calc::Value(mv) = mx { - protocol::PartialCmp::partial_cmp(&**cv, &**mv) - } else { - None - } - } else { - None + // The minimum wins over a smaller maximum, so the maximum is applied first. + let Some(center_vs_max) = Self::partial_cmp_args(¢er, &max) else { + return Ok(Calc::Function(Box::new(MathFunction::Clamp { + min, + center, + max, + }))); }; - - // If center is known to be greater than the maximum, replace it with maximum and remove the max argument. - // Otherwise, if center is known to be less than the maximum, remove the max argument. - if let Some(cmp_val) = cmp { - if cmp_val == Ordering::Greater { - let val = max.take().unwrap(); - center = val; - } else { - min = None; - } + if center_vs_max == Ordering::Greater { + center = max; } - - Ok(match (min, max) { - (None, None) => center, - (Some(min), None) => { - Calc::Function(Box::new(MathFunction::Max(arr2(min, center)))) - } - (None, Some(max)) => { - Calc::Function(Box::new(MathFunction::Min(arr2(max, center)))) - } - (Some(min), Some(max)) => { - Calc::Function(Box::new(MathFunction::Clamp { min, center, max })) - } + Ok(match Self::partial_cmp_args(¢er, &min) { + None => Calc::Function(Box::new(MathFunction::Max(arr2(min, center)))), + Some(Ordering::Less) => min, + Some(_) => center, }) } CalcUnit::Round => input.parse_nested_block(|i| { @@ -1014,6 +996,14 @@ impl Calc { } } + /// Orders two values when their units allow it. + fn partial_cmp_args(a: &Self, b: &Self) -> Option { + match (a, b) { + (Calc::Value(a), Calc::Value(b)) => protocol::PartialCmp::partial_cmp(&**a, &**b), + _ => None, + } + } + /// PERF: /// I don't like how this function requires allocating a second ArrayList /// I am pretty sure we could do this reduction in place, or do it as the diff --git a/test/js/bun/css/css.test.ts b/test/js/bun/css/css.test.ts index 6eba357f68e4..5ecfac95ab76 100644 --- a/test/js/bun/css/css.test.ts +++ b/test/js/bun/css/css.test.ts @@ -187,6 +187,41 @@ describe("css tests", () => { minify_test(`a { rotate: calc(NaN * 1deg) }`, `a{rotate:0deg}`); minify_test(`a { transition-duration: calc(NaN * 1s) }`, `a{transition-duration:0s}`); }); + describe("clamp() simplification", () => { + // The cases lightningcss asserts in its calc tests. + minify_test(`.foo { border-width: clamp(1px, 2px, 3px) }`, `.foo{border-width:2px}`); + minify_test(`.foo { border-width: clamp(1px, 10px, 3px) }`, `.foo{border-width:3px}`); + minify_test(`.foo { border-width: clamp(5px, 2px, 10px) }`, `.foo{border-width:5px}`); + minify_test(`.foo { border-width: clamp(100px, 2px, 10px) }`, `.foo{border-width:100px}`); + minify_test(`.foo { border-width: clamp(5px + 5px, 5px + 7px, 10px + 20px) }`, `.foo{border-width:12px}`); + minify_test(`.foo { border-width: clamp(1px, 2pt, 1in) }`, `.foo{border-width:2pt}`); + minify_test(`.foo { border-width: clamp(1em, 2px, 4vh) }`, `.foo{border-width:clamp(1em,2px,4vh)}`); + minify_test(`.foo { border-width: clamp(1em, 2em, 4vh) }`, `.foo{border-width:clamp(1em,2em,4vh)}`); + minify_test(`.foo { border-width: clamp(1em, 2vh, 4vh) }`, `.foo{border-width:max(1em,2vh)}`); + minify_test(`.foo { border-width: clamp(1px, 1px + 2em, 4px) }`, `.foo{border-width:clamp(1px,1px + 2em,4px)}`); + minify_test(`.foo { width: clamp(-100px, 0px, 50% - 50vw) }`, `.foo{width:clamp(-100px,0px,50% - 50vw)}`); + // The center is equal to one of the bounds. + minify_test(`a { width: clamp(2px, 2px, 3px) }`, `a{width:2px}`); + minify_test(`a { width: clamp(1px, 2px, 2px) }`, `a{width:2px}`); + // The minimum wins over a smaller maximum. The two WPT rows only pass when the minimum is + // compared with the center after the maximum has replaced it. + minify_test(`a { width: clamp(20px, 15px, 10px) }`, `a{width:20px}`); + minify_test(`a { width: clamp(30px, 100px, 20px) }`, `a{width:30px}`); + minify_test(`a { width: clamp(-10px, 100px, -30px) }`, `a{width:-10px}`); + // A lower bound that cannot be compared at parse time is kept, whether or not the center + // was above the maximum. + minify_test(`a { width: clamp(10vw, 5px, 20px) }`, `a{width:max(10vw,5px)}`); + minify_test(`a { width: clamp(10vw, 30px, 20px) }`, `a{width:max(10vw,20px)}`); + // A center that cannot be compared with the maximum leaves the clamp() alone, even when + // it could be compared with the minimum. + minify_test(`a { width: clamp(10px, 5px, 20vw) }`, `a{width:clamp(10px,5px,20vw)}`); + // Other value types. + minify_test(`a { width: clamp(10%, 5%, 20%) }`, `a{width:10%}`); + minify_test(`a { width: clamp(10%, 25%, 20%) }`, `a{width:20%}`); + minify_test(`a { rotate: clamp(0deg, 45deg, 30deg) }`, `a{rotate:30deg}`); + minify_test(`a { transition-duration: clamp(1s, 500ms, 2s) }`, `a{transition-duration:1s}`); + minify_test(`a { width: calc(clamp(1px, 2px, 3px) + 1px) }`, `a{width:3px}`); + }); describe("calc stack overflow", () => { // https://github.com/oven-sh/bun/issues/20128 minify_test(`a { width: calc(100% - 2 - 1) }`, `a{width:calc(100% - 2 - 1)}`); // ideally 100% - 3 From 7e15aeb3fffd4cb0f4525702702b496afd62d039 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 19 Aug 2026 06:26:23 +0000 Subject: [PATCH 2/2] ci: retrigger