Migrate theme(…) calls to var(…) or the modern theme(…) syntax
#14664
Conversation
RobinMalfait
requested review from
philipp-spiess,
thecrypticace and
adamwathan
RobinMalfait
force-pushed
the
feat/migrate-theme-to-var
branch
2 times, most recently
from
ac4d760 to
cf037a8
Compare
packages/@tailwindcss-upgrade/src/template/codemods/theme-to-var.ts
Outdated
Show resolved
Hide resolved
RobinMalfait
force-pushed
the
feat/migrate-theme-to-var
branch
from
cf037a8 to
6c10608
Compare
CHANGELOG.md
Outdated
| @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
| - _Upgrade (experimental)_: Migrate `@media screen(…)` when running codemods ([#14603](https://github.com/tailwindlabs/tailwindcss/pull/14603)) | |||
| - _Upgrade (experimental)_: Inject `@config "…"` when a `tailwind.config.{js,ts,…}` is detected ([#14635](https://github.com/tailwindlabs/tailwindcss/pull/14635)) | |||
| - _Upgrade (experimental)_: Migrate `aria-*`, `data-*`, and `supports-*` variants from arbitrary values to bare values ([#14644](https://github.com/tailwindlabs/tailwindcss/pull/14644)) | |||
| - _Upgrade (experimental)_: Migrate `theme(…)` calls to `var(…)` or the modern `theme(…)` syntax ([#14664](https://github.com/tailwindlabs/tailwindcss/pull/14664)) | |||
There was a problem hiding this comment.
The change log entry is a bit misleading since I would assume all theme() functions are migrated when it's only theme() function inside candidates.
Might want to clarify this or (even better) follow up with the CSS theme() function implementation right away? hopefully should be able to reuse most of the implementation.
There was a problem hiding this comment.
The change log entry is a bit misleading since I would assume all
theme()functions are migrated when it's onlytheme()function inside candidates.
Do you mean that you expect all the theme() functions to be gone? Because in case you have max-[theme(spacing.4)]:flex then we can't replace the theme() with var because the browser doesn't support this. We could sort of resolve the theme(spacing.4) value, but then we end up with max-[1rem]:flex which isn't "dynamic" anymore if you change your spacing scale.
So in this case, we migrate max-[theme(spacing.4)] to the modern theme function max-[theme(--spacing.4)].
Is this what you mean?
There was a problem hiding this comment.
Got it!
It's only about the theme(…) in classes, not about the theme(…) in CSS.
RobinMalfait
force-pushed
the
feat/migrate-theme-to-var
branch
from
6c10608 to
1ab1a01
Compare
| // This test in itself doesn't make much sense. But we need to make sure | ||
| // that this doesn't end up as the modifier in the candidate itself. | ||
| ['max-[theme(spacing.4/50)]:flex', 'max-[theme(--spacing-4/50)]:flex'], | ||
| ])('%s => %s', async (candidate, result) => { |
There was a problem hiding this comment.
So there's a class of theme() keypaths that are valid in v3 (and are valid in our interop layer) but don't have a keypath equivalent in v4. Things like: theme(fontWeight.semibold) or theme(cursor.progress). Let's add some tests here to see they are still migrated.
Maybe what we can do is to use the dict in default-theme.ts and resolve these to the actual values?
There was a problem hiding this comment.
While on call, one thing we noticed is that theme(fontWeight.semibold) is hardcoded in the font-weight utility definition to be 600 and doesn't have a corresponding CSS variable. This means that we can't migrate theme(fontWeight.semibold) to a var(…) or even a theme(--font-weight-semibold) because there is no CSS variable. We could inline it to 600 directly since it's hardcoded.
However, there is a catch... if you define --font-weight-semibold: 100 as a CSS variable inside your @theme { … }, then the value is not hardcoded anymore, but the CSS variable takes precedence.
This means that we will migrate theme(fontWeight.semibold) to theme(--font-weight-semibold) or even var(--font-weight-semibold) but only if you have this CSS variable configured.
Long story short:
- If you migrate to
var(…)ortheme(--…), and remove the CSS variable from your@theme { … }later, then the defaultfontWeight.semiboldwon't apply. - If you don't migrate, but inline the value
theme(fontWeight.semibold)becomes600. Then adding the CSS variable to your@theme { … }later, then the variable won't take effect because we already hardcoded the value to600. - Keep it as-is which is the more correct thing to do I think, but then we have some
theme(…)calls with dot notation, some with--…notation and somevar(…)notation.
There was a problem hiding this comment.
I think the safer thing to do here is to:
- Migrate if we a variable was configured (that's what will happen by default)
- Keep the old syntax if we can't instead of inlining the value.
RobinMalfait
force-pushed
the
feat/migrate-theme-to-var
branch
from
1ab1a01 to
bb35ca8
Compare
RobinMalfait
force-pushed
the
feat/migrate-theme-to-var
branch
from
bb35ca8 to
e65676e
Compare
Because `Convert.None` is just the default, but feels incorrect naming wise.
This PR is a follow up from #14664 migrates all the `theme(…)` calls in your CSS to `var(…)` if we can. In at-rules, like `@media` we can't use `var(…)` so we have to use the modern version of `theme(…)`. In declarations, we can convert to `var(…)` unless there is a modifier used in the `theme(…)` function, then we can only convert to the new `theme(…)` syntax without dot notation. Input: ```css @media theme(spacing.4) { .foo { background-color: theme(colors.red.900); color: theme(colors.red.900 / 75%); /* With spaces around the `/` */ border-color: theme(colors.red.200/75%); /* Without spaces around the `/` */ } } ``` Output: ```css @media theme(--spacing-4) { /* ^^^^^^^^^^^^^^^^^^ Use `theme(…)` since `var(…)` is invalid in this position*/ .foo { background-color: var(--color-red-900); /* Converted to var(…) */ color: theme(--color-red-900 / 75%); /* Convert to modern theme(…) */ border-color: theme(--color-red-200 / 75%); /* Convert to modern theme(…) — pretty printed*/ } } ```
This PR adds a codemod to convert
theme(…)calls tovar(…)calls. If we can't safely do this, then we try to convert thetheme(…)syntax (dot notation) to the moderntheme(…)syntax (with CSS variable-like syntax).Let's look at some examples:
Simple example:
Input:
Output:
With fallback:
Input:
Output:
With modifiers:
Input:
Output:
We can special case this, because if you are using that modifier syntax we assume it's being used in a
theme(…)call referencing a color. This means that we can also convert it to a modifier on the actual candidate.With modifier, if a modifier is already present:
Input:
Output:
In this case we can't use the
var(…)syntax because that requires us to move the opacity modifier to the candidate itself. In this case we could use math to figure out the expected modifier, but that might be too confusing. Instead, we convert to the moderntheme(…)syntax.Multiple
theme(…)calls with modifiers:Input:
Output:
In this case we can't convert to
var(…)syntax because then we lose the opacity modifier. We also can't move the opacity modifier to the candidate itself e.g.:/50because we have 2 different variables to worry about.In this situation we convert to the modern
theme(…)syntax itself.Inside variants:
Input:
Output:
Unfortunately we can't convert to
var(…)syntax reliably because in some cases (like the one above) the value will be used inside of an@media (…)query and CSS doesn't support that at the time of writing this PR.So to be safe, we will not try to convert
theme(…)tovar(…)in variants, but we will only upgrade thetheme(…)call itself to modern syntax.