镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Re-enable: Only expose used CSS variables - #16676

Merged
philipp-spiess merged 8 commits into
mainfrom
feat/cleanup-unused-vars-v2
Feb 21, 2025
Merged

philipp-spiess merged 8 commits into
mainfrom
feat/cleanup-unused-vars-v2

Conversation

@philipp-spiess

@philipp-spiess philipp-spiess commented Feb 19, 2025 •

Copy link
Copy Markdown
Contributor

This PR re-enables the changes necessary to remove unused theme variables and keyframes form your CSS.

This change was initially landed as #16211 and then later reverted in #16403 because we found some unexpected interactions with using @apply and CSS variables in multi-root setups like CSS modules or Vue inline <style> blocks that were no longer seeing their required variables defined.

This issue is fixed by now ensuring that theme variables that are defined within an @reference "…" boundary will still be emitted in the generated CSS when used (as this would otherwise not generate a valid stylesheet).

So given the following input CSS:

@reference "tailwindcss";
.text-red {
  @apply text-red-500;
}

We will now compile this to:

@layer theme {
  :root, :host {
    --text-red-500: oklch(0.637 0.237 25.331);
  }
}
.text-red {
  color: var(--text-red-500);
}

This PR also improves the initial implementation to not mark theme variables as used if they are only used to define other theme variables. For example:

@theme {
  --font-sans:
    ui-sans-serif, system-ui, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol',
    'Noto Color Emoji';
  --font-mono:
    ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, 'Liberation Mono', 'Courier New',
    monospace;

  --default-font-family: var(--font-sans);
  --default-mono-font-family: var(--font-mono);
}

.default-font-family {
  font-family: var(--default-font-family);
}

This would be reduced to the following now as --font-mono is only used to define another variable and never used outside the theme block:

:root, :host {
  --font-sans:
    ui-sans-serif, system-ui, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol',
    'Noto Color Emoji';
  --default-font-family: var(--font-sans);
}

.default-font-family {
  font-family: var(--default-font-family);
}

Test plan

  • See updated unit and integration tests
  • Validated it works end-to-end by using a SvelteKit example

@philipp-spiess
philipp-spiess force-pushed the feat/cleanup-unused-vars-v2 branch 2 times, most recently from c772a81 to b3cb5c3 Compare February 19, 2025 23:03
@philipp-spiess
philipp-spiess marked this pull request as ready for review February 20, 2025 14:23
@philipp-spiess
philipp-spiess requested a review from a team as a code owner February 20, 2025 14:23
@philipp-spiess
philipp-spiess force-pushed the feat/cleanup-unused-vars-v2 branch from b3cb5c3 to 0dce342 Compare February 20, 2025 14:28
@philipp-spiess
philipp-spiess merged commit 7bece4d into main Feb 21, 2025
@philipp-spiess
philipp-spiess deleted the feat/cleanup-unused-vars-v2 branch February 21, 2025 09:45
@moatorres

moatorres commented Feb 21, 2025 •

Copy link
Copy Markdown

Great work, @philipp-spiess! Am I right to assume that this will be included on v4.0.8?

@philipp-spiess

Copy link
Copy Markdown
Contributor Author

@moatorres Yep. We're preparing the release right now so hopefully it's out in a couple of hours 👍

@dpi

dpi commented Feb 21, 2025

Copy link
Copy Markdown

Thanks @philipp-spiess !

@doits

doits commented Feb 21, 2025

Copy link
Copy Markdown

Sorry for hijacking this, but how can I force to output some of the default theme variables even if they are not detected automatically? E.g. how can I always emit all the--color-* variables (because I use them in other places dynamically where the values come from the database and cannot be inferred just by scanning the code)?

@wenbei

wenbei commented Feb 21, 2025

Copy link
Copy Markdown

@doits You can use the new static keyword, see the original #16211

@gene-git

Copy link
Copy Markdown

Should 4.0.8 have fixed #16725 ?

@wongjn

wongjn commented Feb 21, 2025

Copy link
Copy Markdown
Collaborator

No, I think this PR is what is causing #16725.

@gene-git

Copy link
Copy Markdown

Thanks @wongjn - makes more sense :)

I downgraded to 4.0.7 but problem remains. I will try going back to 4.0.6

@gene-git

Copy link
Copy Markdown

4.0.6 has same problem. 4.0.6 with postcss rolled back to 8.5.2 same problem as well.

Now I am perplexed. This should be same as production server is running.

Suggestions?

@wongjn

wongjn commented Feb 21, 2025

Copy link
Copy Markdown
Collaborator

Try checking you have successfully downgraded, via npm ls tailwind - you may have different versions installed as deeper dependencies.

@gene-git

gene-git commented Feb 21, 2025 •

Copy link
Copy Markdown

you're correct - 4.0.8 is pulled in and visible in package-lock.json.

I deleted node_modules and .next and package-lock.json and package.json has 4.0.6

grep tailwindcss package.json
    "@tailwindcss/typography": "^0.5.16",
    "@tailwindcss/postcss": "^4.0.6",
    "tailwindcss": "^4.0.6"

Yet after doing npm install it shows:

$ npm ls tailwindcss

├─┬ @tailwindcss/postcss@4.0.8
│ ├─┬ @tailwindcss/node@4.0.8
│ │ └── tailwindcss@4.0.8 deduped
│ └── tailwindcss@4.0.8 deduped
├─┬ @tailwindcss/typography@0.5.16
│ └── tailwindcss@4.0.8 deduped
└── tailwindcss@4.0.8

Hmm. My ignorance clearly showing - what is the correct way to get 4.0.6 ?

Okay this seems to work:

npm install @tailwindcss/postcss@4.0.6 @tailwindcss/node@4.0.6 tailwindcss@4.0.6

Now its back to 4.0.6 and builds again.

Thank you for your help @wongjn.

@wongjn

wongjn commented Feb 21, 2025

Copy link
Copy Markdown
Collaborator

If you're using git, you could revert package.json and package-lock.json to a commit before 4.0.8 was installed. Otherwise, you could force 4.0.7 or 4.0.6 like:

npm install @tailwindcss/postcss@4.0.6 tailwindcss@4.0.6

@gene-git

Copy link
Copy Markdown

Thanks @wongjn - yep I'm using git so for sure I can go back to the production tag.

@gene-git

Copy link
Copy Markdown

Does anyone have thoughts on how to fix #16725 ?

@doits

doits commented Feb 21, 2025

Copy link
Copy Markdown

@doits You can use the new static keyword, see the original #16211

@wenbei thanks for the tip, I tried it but got stuck at how to name the variable to include all colors, e.g. tried this:

@theme static {
  --color-*: <XXX>;
}

with <XXX> as static, default, initial and inherit (my four best guesses) – it failed to compile with each, for example:

Error: Invalid theme value static for namespace --color-*

Or must I copy all the variables from the docs and put them there explicitly? I guess this works in the meantime but then I won't get any update if the theme is changed 🤔

@doits

doits commented Feb 21, 2025

Copy link
Copy Markdown

... just found out in #16514 how to include all variables:

@import "tailwindcss" theme(static);

Works for my use case.

Though I'd still be interested if you can make only some of the variables static (e.g. only all --color-* ones).

philipp-spiess added a commit that referenced this pull request Feb 25, 2025
…larations (#16774)

Fixes #16725

When using `@reference "tailwindcss";` inside a separate CSS root (e.g.
Svelte `<style>` components, CSS modules, etc.), we have no guarantee
that the CSS variables will be defined in the main stylesheet (or if
there even is one). To work around potential issues with this we decided
in #16676 that we would emit all used CSS variables from the `@theme`
inside the `@reference` block.

However, this is not only a bit surprising but also unexpected in CSS
modules and Next.js that **requires CSS module files to only create
scope-able declarations**. To fix this issue, we decided to not emit CSS
variables but instead ensure all `var(…)` calls we create for theme
values in reference mode will simply have their fallback value added.

This ensures styles work as-expected even if the root Tailwind file does
not pick up the variable as being used or _if you don't add a root at
all_. Furthermore we do not duplicate any variable declarations across
your stylesheets and you still have the ability to change variables at
runtime.

## Test plan

- Updated snapshots everywhere (see diff)
- New Next.js CSS modules integration test
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.

8 participants