Skip to content

Evaluate theme functions in plugins#14326

Merged
adamwathan merged 4 commits intonextfrom
fix/evaluate-theme-functions-in-plugins
Sep 4, 2024
Merged

Evaluate theme functions in plugins#14326
adamwathan merged 4 commits intonextfrom
fix/evaluate-theme-functions-in-plugins

Conversation

@adamwathan
Copy link
Member

This PR fixes a bug where CSS theme() functions were not evaluated when present in rules added by plugins, using either @plugin or registering a plugin in a JS config file.

For example, prior to this PR the theme() functions in this plugin would make it into the final CSS without being evaluated:

// ./my-plugin.js
export default plugin(({ addBase }) => {
  addBase({
    '.my-rule': {
      background: 'theme(colors.primary)',
      color: 'theme(colors.secondary)',
    },
  })
})

// could register new rules that include functions, and JS config files could
// also contain functions or plugins that use functions so we need to evaluate
// functions if either of those are present.
if (css.includes(THEME_FUNCTION_INVOCATION) || plugins.length > 0 || configs.length > 0) {
Copy link
Contributor

@thecrypticace thecrypticace Sep 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The theme() calls from utilities are not processed by this call to substituteFunctions. The call in build() handles that because the generated nodes are the ones that have theme().

This is purely required for the work where we're taking the config stuff and backfilling some values into the theme (which ultimately makes it into the "static" AST).

So maybe the comment could use some clarification.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though theme calls inside @utility would be processed here but that happens to be more of an optimization.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is only for detecting wether addBase pushed stuff into the input CSS AST, right? The rest will exist only in the designSystem.

// Arbitrary values (`text-[theme(--color-red-500)]`) and arbitrary
// properties (`[--my-var:theme(--color-red-500)]`) can contain function
// calls so we need evaluate any functions we find there that weren't in
// the source CSS.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment above but this is where theme() calls output by utilities (doing like addUtilities or matchUtilities) are processed.

Comment on lines +640 to +650
addBase({
'.my-base-rule': {
color: 'theme(colors.red)',
'outline-color': 'theme(colors.orange / 15%)',
'background-color': 'theme(--color-blue)',
'border-color': 'theme(--color-pink / 10%)',
},
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably also add an addUtility to test values inside utilities (since these won't be added to the initial CSS AST so it will be replaced at a different layer.

Suggested change
addBase({
'.my-base-rule': {
color: 'theme(colors.red)',
'outline-color': 'theme(colors.orange / 15%)',
'background-color': 'theme(--color-blue)',
'border-color': 'theme(--color-pink / 10%)',
},
})
addBase({
'.my-base-rule': {
color: 'theme(colors.red)',
'outline-color': 'theme(colors.orange / 15%)',
'background-color': 'theme(--color-blue)',
'border-color': 'theme(--color-pink / 10%)',
},
})
addUtilities({
'.my-utility': {
color: 'theme(colors.red)',
},
})

// could register new rules that include functions, and JS config files could
// also contain functions or plugins that use functions so we need to evaluate
// functions if either of those are present.
if (css.includes(THEME_FUNCTION_INVOCATION) || plugins.length > 0 || configs.length > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is only for detecting wether addBase pushed stuff into the input CSS AST, right? The rest will exist only in the designSystem.

@adamwathan adamwathan force-pushed the fix/evaluate-theme-functions-in-plugins branch from a4f8a3c to 420a5ef Compare September 4, 2024 15:33
@adamwathan adamwathan merged commit 6d0371a into next Sep 4, 2024
@adamwathan adamwathan deleted the fix/evaluate-theme-functions-in-plugins branch September 4, 2024 15:49
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.

4 participants