From a51b63a05d272dc016d20a53608dd272e4752ef9 Mon Sep 17 00:00:00 2001
From: Adam Wathan <adam.wathan@gmail.com>
Date: Wed, 18 Sep 2024 09:20:04 -0400
Subject: [PATCH] Don't override explicit `leading-*`, `tracking-*`, or
 `font-{weight}` utilities with font-size utility defaults (#14403)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This PR improves how the `text-{size}` utilities interact with the
`leading-*`, `tracking-*`, and `font-{weight}` utilities, ensuring that
if the user explicitly uses any of those utilities that those values are
not squashed by any defaults baked into the `text-{size}` utilities.

Prior to this PR, if you wrote something like this:

```html
<div class="text-lg leading-none md:text-2xl">
```

…the `leading-none` class would be overridden by the default line-height
value baked into the `text-2xl` utility at the `md` breakpoint. This has
been a point of confusion and frustration for people [in the
past](https://github.com/tailwindlabs/tailwindcss/issues/6504) who are
annoyed they have to keep repeating their custom `leading-*` value like
this:

```html
<div class="text-lg leading-none md:text-2xl md:leading-none lg:text-4xl lg:leading-none">
```

This PR lets you write this HTML instead but get the same behavior as
above:

```html
<div class="text-lg leading-none md:text-2xl lg:text-4xl">
```

It's important to note that this change _only_ applies to line-height
values set explicitly with a `leading-*` utility, and does not apply to
the line-height modifier.

In this example, the line-height set by `text-sm/6` does _not_ override
the default line-height included in the `md:text-lg` utility:

```html
<div class="text-sm/6 md:text-lg">
```

That means these two code snippets behave differently:

```html
<div class="text-sm/6 md:text-lg">…</div>
<div class="text-sm leading-6 md:text-lg">…</div>
```

In the top one, the line-height `md:text-lg` overrides the line-height
set by `text-sm/6`, but in the bottom one, the explicit `leading-6`
utility takes precedence.

This PR applies the same improvements to `tracking-*` and
`font-{weight}` as well, since all font size utilities can also
optionally specify default `letter-spacing` and `font-weight` values.

We achieve this using new semi-private CSS variables like we do for
things like transforms, shadows, etc., which are set by the `leading-*`,
`tracking-*`, and `font-{weight}` utilities respectively. The
`text-{size}` utilities always use these values first if they are
defined, and the default values become fallbacks for those variables if
they aren't present.

We use `@property` to make sure these variables are reset to `initial`
on a per element basis so that they are never inherited, like with every
other variable we define.

This PR does slightly increase the amount of CSS generated, because now
utilities like `leading-5` look like this:

```diff
  .leading-5 {
+   --tw-leading: 1.25rem;
    line-height: 1.25rem;
  }
```

…and utilites like `text-sm` include a `var(…)` lookup that they didn't
include before:

```diff
  .text-sm {
    font-size: 0.875rem;
-   line-height: var(--font-size-sm--line-height, 1.25rem);
+   line-height: var(--tw-leading, var(--font-size-sm--line-height, 1.25rem));
  }
```

If this extra CSS doesn't feel worth it for the small improvement in
behavior, we may consider just closing this PR and keeping things as
they are.

This PR is also a breaking change for anyone who was depending on the
old behavior, and expected the line-height baked into the `md:text-lg`
class to take precedence over the explicit `leading-6` class:

```html
<div class="text-sm leading-6 md:text-lg">…</div>
```

Personally I am comfortable with this because of the fact that you can
still get the old behavior by preferring a line-height modifier:

```html
<div class="text-sm/6 md:text-lg">…</div>
```

---------

Co-authored-by: Adam Wathan <4323180+adamwathan@users.noreply.github.com>
Co-authored-by: Jordan Pittman <jordan@cryptica.me>
---
 CHANGELOG.md                                  |   4 +
 .../src/__snapshots__/index.test.ts.snap      |  16 ++-
 packages/tailwindcss/src/ast.ts               |   2 +-
 .../tailwindcss/src/compat/config.test.ts     |  60 +++++----
 packages/tailwindcss/src/utilities.test.ts    | 100 +++++++++++---
 packages/tailwindcss/src/utilities.ts         |  49 +++++--
 packages/tailwindcss/tests/ui.spec.ts         | 126 +++++++++++++++++-
 7 files changed, 305 insertions(+), 52 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 896c9d3ab..0b6fe4a6a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,6 +19,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Skip candidates with invalid `theme()` calls ([#14437](https://github.com/tailwindlabs/tailwindcss/pull/14437))
 - Don't generate `inset-*` utilities for `--inset-shadow-*` and `--inset-ring-*` theme values ([#14447](https://github.com/tailwindlabs/tailwindcss/pull/14447))
 
+### Changed
+
+- Don't override explicit `leading-*`, `tracking-*`, or `font-{weight}` utilities with font-size utility defaults ([#14403](https://github.com/tailwindlabs/tailwindcss/pull/14403))
+
 ## [4.0.0-alpha.24] - 2024-09-11
 
 ### Added
diff --git a/packages/@tailwindcss-postcss/src/__snapshots__/index.test.ts.snap b/packages/@tailwindcss-postcss/src/__snapshots__/index.test.ts.snap
index ee626aeea..2edc7281f 100644
--- a/packages/@tailwindcss-postcss/src/__snapshots__/index.test.ts.snap
+++ b/packages/@tailwindcss-postcss/src/__snapshots__/index.test.ts.snap
@@ -586,7 +586,7 @@ exports[`\`@import 'tailwindcss'\` is replaced with the generated CSS 1`] = `
 @layer utilities {
   .text-2xl {
     font-size: var(--font-size-2xl, 1.5rem);
-    line-height: var(--font-size-2xl--line-height, 2rem);
+    line-height: var(--tw-leading, var(--font-size-2xl--line-height, 2rem));
   }
 
   .text-black\\/50 {
@@ -599,11 +599,20 @@ exports[`\`@import 'tailwindcss'\` is replaced with the generated CSS 1`] = `
 
   @media (width >= 96rem) {
     .\\32 xl\\:font-bold {
+      --tw-font-weight: 700;
       font-weight: 700;
     }
   }
 }
 
+@supports (-moz-orient: inline) {
+  @layer base {
+    *, :before, :after, ::backdrop {
+      --tw-font-weight: initial;
+    }
+  }
+}
+
 @keyframes spin {
   to {
     transform: rotate(360deg);
@@ -633,5 +642,10 @@ exports[`\`@import 'tailwindcss'\` is replaced with the generated CSS 1`] = `
     animation-timing-function: cubic-bezier(0, 0, .2, 1);
     transform: none;
   }
+}
+
+@property --tw-font-weight {
+  syntax: "*";
+  inherits: false
 }"
 `;
diff --git a/packages/tailwindcss/src/ast.ts b/packages/tailwindcss/src/ast.ts
index d64880c6f..fa5dc6f1b 100644
--- a/packages/tailwindcss/src/ast.ts
+++ b/packages/tailwindcss/src/ast.ts
@@ -26,7 +26,7 @@ export function rule(selector: string, nodes: AstNode[]): Rule {
   }
 }
 
-export function decl(property: string, value: string): Declaration {
+export function decl(property: string, value: string | undefined): Declaration {
   return {
     kind: 'declaration',
     property,
diff --git a/packages/tailwindcss/src/compat/config.test.ts b/packages/tailwindcss/src/compat/config.test.ts
index a4477ec14..97da1695a 100644
--- a/packages/tailwindcss/src/compat/config.test.ts
+++ b/packages/tailwindcss/src/compat/config.test.ts
@@ -301,35 +301,49 @@ describe('theme callbacks', () => {
 
     expect(compiler.build(['leading-base', 'leading-md', 'leading-xl', 'prose']))
       .toMatchInlineSnapshot(`
-      ":root {
-        --font-size-base: 100rem;
-        --font-size-md--line-height: 101rem;
-      }
-      .prose {
-        [class~=lead-base] {
-          font-size: 100rem;
+        ":root {
+          --font-size-base: 100rem;
+          --font-size-md--line-height: 101rem;
+        }
+        .prose {
+          [class~=lead-base] {
+            font-size: 100rem;
+            line-height: 201rem;
+          }
+          [class~=lead-md] {
+            font-size: 200rem;
+            line-height: 101rem;
+          }
+          [class~=lead-xl] {
+            font-size: 200rem;
+            line-height: 201rem;
+          }
+        }
+        .leading-base {
+          --tw-leading: 201rem;
           line-height: 201rem;
         }
-        [class~=lead-md] {
-          font-size: 200rem;
+        .leading-md {
+          --tw-leading: 101rem;
           line-height: 101rem;
         }
-        [class~=lead-xl] {
-          font-size: 200rem;
+        .leading-xl {
+          --tw-leading: 201rem;
           line-height: 201rem;
         }
-      }
-      .leading-base {
-        line-height: 201rem;
-      }
-      .leading-md {
-        line-height: 101rem;
-      }
-      .leading-xl {
-        line-height: 201rem;
-      }
-      "
-    `)
+        @supports (-moz-orient: inline) {
+          @layer base {
+            *, ::before, ::after, ::backdrop {
+              --tw-leading: initial;
+            }
+          }
+        }
+        @property --tw-leading {
+          syntax: "*";
+          inherits: false;
+        }
+        "
+      `)
   })
 })
 
diff --git a/packages/tailwindcss/src/utilities.test.ts b/packages/tailwindcss/src/utilities.test.ts
index 5a48fce86..66312ddee 100644
--- a/packages/tailwindcss/src/utilities.test.ts
+++ b/packages/tailwindcss/src/utilities.test.ts
@@ -872,26 +872,26 @@ test('col-start', async () => {
   expect(
     await run(['col-start-auto', 'col-start-4', 'col-start-99', 'col-start-[123]', '-col-start-4']),
   ).toMatchInlineSnapshot(`
-      ".-col-start-4 {
-        grid-column-start: calc(4 * -1);
-      }
+    ".-col-start-4 {
+      grid-column-start: calc(4 * -1);
+    }
 
-      .col-start-4 {
-        grid-column-start: 4;
-      }
+    .col-start-4 {
+      grid-column-start: 4;
+    }
 
-      .col-start-99 {
-        grid-column-start: 99;
-      }
+    .col-start-99 {
+      grid-column-start: 99;
+    }
 
-      .col-start-\\[123\\] {
-        grid-column-start: 123;
-      }
+    .col-start-\\[123\\] {
+      grid-column-start: 123;
+    }
 
-      .col-start-auto {
-        grid-column-start: auto;
-      }"
-    `)
+    .col-start-auto {
+      grid-column-start: auto;
+    }"
+  `)
   expect(
     await run([
       'col-start',
@@ -11446,19 +11446,36 @@ test('font', async () => {
     }
 
     .font-\\[--my-family\\] {
+      --tw-font-weight: var(--my-family);
       font-weight: var(--my-family);
     }
 
     .font-\\[100\\] {
+      --tw-font-weight: 100;
       font-weight: 100;
     }
 
     .font-\\[number\\:--my-weight\\] {
+      --tw-font-weight: var(--my-weight);
       font-weight: var(--my-weight);
     }
 
     .font-bold {
+      --tw-font-weight: 700;
       font-weight: 700;
+    }
+
+    @supports (-moz-orient: inline) {
+      @layer base {
+        *, :before, :after, ::backdrop {
+          --tw-font-weight: initial;
+        }
+      }
+    }
+
+    @property --tw-font-weight {
+      syntax: "*";
+      inherits: false
     }"
   `)
   expect(
@@ -13067,15 +13084,31 @@ test('leading', async () => {
     }
 
     .leading-6 {
+      --tw-leading: var(--line-height-6, 1.5rem);
       line-height: var(--line-height-6, 1.5rem);
     }
 
     .leading-\\[--value\\] {
+      --tw-leading: var(--value);
       line-height: var(--value);
     }
 
     .leading-none {
+      --tw-leading: var(--line-height-none, 1);
       line-height: var(--line-height-none, 1);
+    }
+
+    @supports (-moz-orient: inline) {
+      @layer base {
+        *, :before, :after, ::backdrop {
+          --tw-leading: initial;
+        }
+      }
+    }
+
+    @property --tw-leading {
+      syntax: "*";
+      inherits: false
     }"
   `)
   expect(
@@ -13110,19 +13143,36 @@ test('tracking', async () => {
     }
 
     .-tracking-\\[--value\\] {
+      --tw-tracking: calc(var(--value) * -1);
       letter-spacing: calc(var(--value) * -1);
     }
 
     .tracking-\\[--value\\] {
+      --tw-tracking: var(--value);
       letter-spacing: var(--value);
     }
 
     .tracking-normal {
+      --tw-tracking: var(--letter-spacing-normal, 0em);
       letter-spacing: var(--letter-spacing-normal, 0em);
     }
 
     .tracking-wide {
+      --tw-tracking: var(--letter-spacing-wide, .025em);
       letter-spacing: var(--letter-spacing-wide, .025em);
+    }
+
+    @supports (-moz-orient: inline) {
+      @layer base {
+        *, :before, :after, ::backdrop {
+          --tw-tracking: initial;
+        }
+      }
+    }
+
+    @property --tw-tracking {
+      syntax: "*";
+      inherits: false
     }"
   `)
   expect(
@@ -13697,7 +13747,7 @@ test('text', async () => {
 
     .text-sm {
       font-size: var(--font-size-sm, .875rem);
-      line-height: var(--font-size-sm--line-height, 1.25rem);
+      line-height: var(--tw-leading, var(--font-size-sm--line-height, 1.25rem));
     }
 
     .text-\\[12px\\]\\/6 {
@@ -15188,7 +15238,7 @@ describe('custom utilities', () => {
       "@layer utilities {
         .text-sm {
           font-size: var(--font-size-sm, .875rem);
-          line-height: var(--font-size-sm--line-height, 1.25rem);
+          line-height: var(--tw-leading, var(--font-size-sm--line-height, 1.25rem));
           font-size: var(--font-size-sm, .8755rem);
           line-height: var(--font-size-sm--line-height, 1.255rem);
           text-rendering: optimizeLegibility;
@@ -15350,6 +15400,7 @@ describe('custom utilities', () => {
       ),
     ).toMatchInlineSnapshot(`
       ".bar {
+        --tw-font-weight: 700;
         font-weight: 700;
       }
 
@@ -15359,6 +15410,19 @@ describe('custom utilities', () => {
           text-decoration-line: underline;
           display: flex;
         }
+      }
+
+      @supports (-moz-orient: inline) {
+        @layer base {
+          *, :before, :after, ::backdrop {
+            --tw-font-weight: initial;
+          }
+        }
+      }
+
+      @property --tw-font-weight {
+        syntax: "*";
+        inherits: false
       }"
     `)
   })
diff --git a/packages/tailwindcss/src/utilities.ts b/packages/tailwindcss/src/utilities.ts
index f3414ba13..9a73c1ac0 100644
--- a/packages/tailwindcss/src/utilities.ts
+++ b/packages/tailwindcss/src/utilities.ts
@@ -2960,7 +2960,11 @@ export function createUtilities(theme: Theme) {
           return [decl('font-family', value)]
         }
         default: {
-          return [decl('font-weight', value)]
+          return [
+            atRoot([property('--tw-font-weight')]),
+            decl('--tw-font-weight', value),
+            decl('font-weight', value),
+          ]
         }
       }
     }
@@ -2985,7 +2989,11 @@ export function createUtilities(theme: Theme) {
     {
       let value = theme.resolve(candidate.value.value, ['--font-weight'])
       if (value) {
-        return [decl('font-weight', value)]
+        return [
+          atRoot([property('--tw-font-weight')]),
+          decl('--tw-font-weight', value),
+          decl('font-weight', value),
+        ]
       }
 
       switch (candidate.value.value) {
@@ -3019,7 +3027,11 @@ export function createUtilities(theme: Theme) {
       }
 
       if (value) {
-        return [decl('font-weight', value)]
+        return [
+          atRoot([property('--tw-font-weight')]),
+          decl('--tw-font-weight', value),
+          decl('font-weight', value),
+        ]
       }
     }
   })
@@ -3812,13 +3824,21 @@ export function createUtilities(theme: Theme) {
 
   functionalUtility('leading', {
     themeKeys: ['--line-height'],
-    handle: (value) => [decl('line-height', value)],
+    handle: (value) => [
+      atRoot([property('--tw-leading')]),
+      decl('--tw-leading', value),
+      decl('line-height', value),
+    ],
   })
 
   functionalUtility('tracking', {
     supportsNegative: true,
     themeKeys: ['--letter-spacing'],
-    handle: (value) => [decl('letter-spacing', value)],
+    handle: (value) => [
+      atRoot([property('--tw-tracking')]),
+      decl('--tw-tracking', value),
+      decl('letter-spacing', value),
+    ],
   })
 
   staticUtility('antialiased', [
@@ -4129,9 +4149,22 @@ export function createUtilities(theme: Theme) {
 
         return [
           decl('font-size', fontSize),
-          decl('line-height', options['--line-height']),
-          decl('letter-spacing', options['--letter-spacing']),
-          decl('font-weight', options['--font-weight']),
+          decl(
+            'line-height',
+            options['--line-height'] ? `var(--tw-leading, ${options['--line-height']})` : undefined,
+          ),
+          decl(
+            'letter-spacing',
+            options['--letter-spacing']
+              ? `var(--tw-tracking, ${options['--letter-spacing']})`
+              : undefined,
+          ),
+          decl(
+            'font-weight',
+            options['--font-weight']
+              ? `var(--tw-font-weight, ${options['--font-weight']})`
+              : undefined,
+          ),
         ]
       }
     }
diff --git a/packages/tailwindcss/tests/ui.spec.ts b/packages/tailwindcss/tests/ui.spec.ts
index 03555eebf..59546cf6d 100644
--- a/packages/tailwindcss/tests/ui.spec.ts
+++ b/packages/tailwindcss/tests/ui.spec.ts
@@ -287,12 +287,135 @@ test('content-none persists when conditionally styling a pseudo-element', async
   expect(await getPropertyValue(['#x', '::after'], 'content')).toEqual('none')
 })
 
+test('explicit leading utilities are respected when overriding font-size', async ({ page }) => {
+  let { getPropertyValue } = await render(
+    page,
+    html`
+      <div id="x" class="text-sm hover:text-xl">Hello world</div>
+      <div id="y" class="text-sm leading-tight hover:text-xl">Hello world</div>
+      <div id="z" class="text-sm leading-[10px] hover:text-xl">Hello world</div>
+    `,
+    css`
+      @theme {
+        --font-size-sm: 14px;
+        --font-size-sm--line-height: 16px;
+        --font-size-xl: 20px;
+        --font-size-xl--line-height: 24px;
+        --line-height-tight: 8px;
+      }
+    `,
+  )
+
+  expect(await getPropertyValue('#x', 'line-height')).toEqual('16px')
+  await page.locator('#x').hover()
+  expect(await getPropertyValue('#x', 'line-height')).toEqual('24px')
+
+  expect(await getPropertyValue('#y', 'line-height')).toEqual('8px')
+  await page.locator('#y').hover()
+  expect(await getPropertyValue('#y', 'line-height')).toEqual('8px')
+
+  expect(await getPropertyValue('#z', 'line-height')).toEqual('10px')
+  await page.locator('#z').hover()
+  expect(await getPropertyValue('#z', 'line-height')).toEqual('10px')
+})
+
+test('explicit leading utilities are overridden by line-height modifiers', async ({ page }) => {
+  let { getPropertyValue } = await render(
+    page,
+    html`
+      <div id="x" class="text-sm hover:text-xl/[100px]">Hello world</div>
+      <div id="y" class="text-sm leading-tight hover:text-xl/[100px]">Hello world</div>
+      <div id="z" class="text-sm leading-[10px] hover:text-xl/[100px]">Hello world</div>
+    `,
+    css`
+      @theme {
+        --font-size-sm: 14px;
+        --font-size-sm--line-height: 16px;
+        --font-size-xl: 20px;
+        --font-size-xl--line-height: 24px;
+        --line-height-tight: 8px;
+      }
+    `,
+  )
+
+  expect(await getPropertyValue('#x', 'line-height')).toEqual('16px')
+  await page.locator('#x').hover()
+  expect(await getPropertyValue('#x', 'line-height')).toEqual('100px')
+
+  expect(await getPropertyValue('#y', 'line-height')).toEqual('8px')
+  await page.locator('#y').hover()
+  expect(await getPropertyValue('#y', 'line-height')).toEqual('100px')
+
+  expect(await getPropertyValue('#z', 'line-height')).toEqual('10px')
+  await page.locator('#z').hover()
+  expect(await getPropertyValue('#z', 'line-height')).toEqual('100px')
+})
+
+test('explicit tracking utilities are respected when overriding font-size', async ({ page }) => {
+  let { getPropertyValue } = await render(
+    page,
+    html`
+      <div id="x" class="text-sm hover:text-xl">Hello world</div>
+      <div id="y" class="text-sm tracking-tight hover:text-xl">Hello world</div>
+      <div id="z" class="text-sm tracking-[3px] hover:text-xl">Hello world</div>
+    `,
+    css`
+      @theme {
+        --font-size-sm--letter-spacing: 5px;
+        --font-size-xl--letter-spacing: 10px;
+        --letter-spacing-tight: 1px;
+      }
+    `,
+  )
+
+  expect(await getPropertyValue('#x', 'letter-spacing')).toEqual('5px')
+  await page.locator('#x').hover()
+  expect(await getPropertyValue('#x', 'letter-spacing')).toEqual('10px')
+
+  expect(await getPropertyValue('#y', 'letter-spacing')).toEqual('1px')
+  await page.locator('#y').hover()
+  expect(await getPropertyValue('#y', 'letter-spacing')).toEqual('1px')
+
+  expect(await getPropertyValue('#z', 'letter-spacing')).toEqual('3px')
+  await page.locator('#z').hover()
+  expect(await getPropertyValue('#z', 'letter-spacing')).toEqual('3px')
+})
+
+test('explicit font-weight utilities are respected when overriding font-size', async ({ page }) => {
+  let { getPropertyValue } = await render(
+    page,
+    html`
+      <div id="x" class="text-sm hover:text-xl">Hello world</div>
+      <div id="y" class="text-sm font-bold hover:text-xl">Hello world</div>
+      <div id="z" class="text-sm font-[900] hover:text-xl">Hello world</div>
+    `,
+    css`
+      @theme {
+        --font-size-sm--font-weight: 100;
+        --font-size-xl--font-weight: 200;
+      }
+    `,
+  )
+
+  expect(await getPropertyValue('#x', 'font-weight')).toEqual('100')
+  await page.locator('#x').hover()
+  expect(await getPropertyValue('#x', 'font-weight')).toEqual('200')
+
+  expect(await getPropertyValue('#y', 'font-weight')).toEqual('700')
+  await page.locator('#y').hover()
+  expect(await getPropertyValue('#y', 'font-weight')).toEqual('700')
+
+  expect(await getPropertyValue('#z', 'font-weight')).toEqual('900')
+  await page.locator('#z').hover()
+  expect(await getPropertyValue('#z', 'font-weight')).toEqual('900')
+})
+
 // ---
 
 const preflight = fs.readFileSync(path.resolve(__dirname, '..', 'preflight.css'), 'utf-8')
 const defaultTheme = fs.readFileSync(path.resolve(__dirname, '..', 'theme.css'), 'utf-8')
 
-async function render(page: Page, content: string) {
+async function render(page: Page, content: string, extraCss: string = '') {
   let { build } = await compile(css`
     @layer theme, base, components, utilities;
     @layer theme {
@@ -304,6 +427,7 @@ async function render(page: Page, content: string) {
     @layer utilities {
       @tailwind utilities;
     }
+    ${extraCss}
   `)
 
   // We noticed that some of the tests depending on the `hover:` variant were