Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking changes in plugins potentially caused by parser changes #13473

Closed
brandonmcconnell opened this issue Apr 8, 2024 · 12 comments
Closed
Assignees

Comments

@brandonmcconnell
Copy link
Contributor

brandonmcconnell commented Apr 8, 2024

What version of Tailwind CSS are you using?

v3.4.3

What browser are you using?

Chrome v123.0.6312.107 (Official Build) (arm64)

What operating system are you using?

macOS Sonoma v14.4.1 (23E224)

Reproduction URL

Describe your issue

I was auditing some of the plugins in my growing collection of plugins I've developed for the Tailwind CSS community this past week, and a couple of them appear to now break due to what I perceive to be changes in the way the parser handles arbitrary values in plugins.

Affected plugins:

1. MultiTool for Tailwind CSS (github | npm)

My MultiTool for Tailwind CSS plugin breaks when it contains a string including the : symbol, likely due to the parser terminating/splitting variants when it encounters the : symbol.

I've done some testing, and this doesn't appear to be something my plugin can account for internally. If you look at the reproduction URL linked above (also here), the first two lines receive the expected styles, but the last breaks entirely if it contains that character.

The problematic instances (those that contain :) don't appear to trigger the plugin and are likely seen as erroneous since, assuming my guesses here are correct, a usage like multi-[underline;hover:font-bold] would be split into :multi-[underline;hover and font-bold].

With some slight tweaking to the parser, values could whitelist any instances of : within brackets.

  • current: 'a:b:[c:d]:e'['a', 'b', '[c', 'd]', 'e']
  • desired: 'a:b:[c:d]:e'['a', 'b', '[c:d]', 'e']

It's also important to note here that this was not always the case, or at least I am convinced it is not, as I include this example in the plugin's README, which worked when I tested it before including it in the README:

<div class="sm:[&>div]:hover:active:multi-[font-bold;text-[red];[font-family:'Open_Sans',sans-serif]]">
  When hovered, this text will appear bold, red, and in Open Sans font.
</div>

Even a more :-riddled nested variant should be perfectly fine:

<div class="hover:multi-[[&:is(*)]:focus:underline;[--some-var:{whatever}]]"></div>

2. JS Tool for Tailwind CSS (github | npm)

Without rehashing many of the same points I did above, if you take a look at this Tailwind Play example linked above (also here), you'll see many examples I use to showcase how JS can expose specific values to Tailwind CSS using plugins. Admittedly, I seldom use this one, but it is nice sometimes when I need to break glass and expose JSON values to CSS while testing. I could expose them as CSS variables alternatively, but it's helpful to have some means to directly interface with raw data like this, even if only used on occasion.

Here are those same examples from the linked Tailwind Play example, all of which previously worked:

<div class="before:js-[content-['fontSize.2xl_===_#{theme('fontSize.2xl')}']]"></div>
<div class="before:js-[content-['the_registered_config_keys_are_#{Object.keys(config()).join(',_')}']]"></div>
<div class="before:js-[content-['A_random_digit_is_#{randomDigit()}']]"></div>
<div class="js-[[--random-color:#{randomColor()}]] js-[[--random-color-2:#{randomColor()}]] text-[--random-color] font-semibold [text-shadow:1px_2px_0_var(--random-color-2)]">Random_colors_ftw!</div>
<div class="js-[[--random-length:#{randomRange(16,22)}px]] text-[length:--random-length]">Random sizes too 🤯</div>

In both of these examples, I'm primarily addressing how these regressions limit plugin authors from achieving some of the powerful tasks plugins were formerly able to perform.

Both of these plugins may seem to operate unconventionally, but that's why they're plugins. This is part of what excites a community like this one—testing the limits of a technology like Tailwind CSS, finding various ways it can be extended to achieve different tasks, and ultimately building within those limits to make someone else's work easier.

I've heard from real users of my plugins how my tools have improved their experience building with Tailwind CSS, and I would hate to discontinue some of them due to breaking changes.

Even this past week, I've been prepping content for a blog post or video—perhaps both—on the powerful ways Tailwind CSS plugins supercharge the developer experience, demonstrating examples and providing a crash course on how to build effective plugins.

"The power of JavaScript in the simple and familiar form factor of Tailwind CSS utilities." (still bikeshedding the name)

Because many people primarily associate Tailwind CSS with CSS (for obvious reasons), they miss that with plugins, we can interject all sorts of sophisticated and useful functionality into our utilities. Plugins are essentially middleware that allow us to bridge the parser/compiler to our frontend in dynamic and performant ways, leveraging build-time processing.

Many things that are not yet possible in pure CSS are possible with Tailwind CSS because of this JS processing, so it acts more like mixins and functions than just variable interpolation.

That said, it would be helpful to use examples like these to demonstrate the flexibility of Tailwind CSS plugins. Thanks!

@brandonmcconnell
Copy link
Contributor Author

If there is any way to get a more granular version selection menu on Tailwind Play, I'd be happy to investigate further and find which version(s) triggered these regressions.

@brandonmcconnell brandonmcconnell changed the title Parser changes causing breaking changes Breaking changes in plugins potentially caused by parser changes Apr 8, 2024
@brandonmcconnell
Copy link
Contributor Author

Perhaps related to #13102 — different issue, but both issues occur when @apply'd

@brandonmcconnell
Copy link
Contributor Author

I've investigated this a bit further (Play CDN examples) and found another case where this breaks, demonstrated below. I'm going to backversion the Play CDN to see if I can pinpoint which version this bug first presented in.


@apply utilities with quotes (maybe related to #13465)

✅ multi-[font-bold;text-[red]]
✅ multi-[[font-family:monospace]]
❌ multi-[[font-family:'verdana']]
❌ multi-[[font-family:"verdana"]]

@apply nested variants

✅ multi-[!bg-green-500/30;text-green-800]
❌ multi-[!bg-green-500/30;text-green-800;hover:bg-purple-500/30;hover:text-purple-300]

@brandonmcconnell
Copy link
Contributor Author

brandonmcconnell commented Apr 9, 2024

Both groups of examples (@apply utilities with quotes, @apply nested variants) break in the current version, v3.4.3:
https://codepen.io/brandonmcconnell/pen/eYorvyr/f8ffd784de4d4a548023c34a7f815cd4

The last version the first group of examples (@apply utilities with quotes) worked in was v3.3.5.

v3.3.5 - ✅ 4/4 tests pass (first group) (CodePen) v3.3.6 - ❌ 2/4 tests pass (first group) (CodePen)

I've traversed back pretty far, and I'm struggling to find a version where multi-[!bg-green-500/30;text-green-800;hover:bg-purple-500/30;hover:text-purple-300] worked, so perhaps nested variants were never supported, though I find it hard to believe as I use this plugin often enough and have quite a few users who do as well.

If nested variants are not currently supported due to some issue parsing the : symbol in arbitrary values, that feels like a bug to me. Let me know if you want me to open a separate issue/feature request to address that.

@brandonmcconnell
Copy link
Contributor Author

brandonmcconnell commented Apr 9, 2024

That investigation above concerns my MultiTool plugin, though some of those fixes, such as the quotes issue, might also fix some of the issues with the JS Tool plugin.

It seems likely that some other symbols are being flagged as well, as JS Tool is breaking in some places without quotes at symbols like parentheses (e.g. js-[[--random-color:#{randomColor()}]]).

Interestingly, in the Tailwind Play example, the intellisense on Tailwind Play appears to be able to process this, though the actual values never make it to the CSSOM.

JS Tool also appears to have broken during the v3.3.6 release:

v3.3.5 - ✅ 5/5 tests pass (CodePen) v3.3.6 - ❌ 0/5 tests pass (CodePen)

@RobinMalfait RobinMalfait self-assigned this Apr 10, 2024
@brandonmcconnell
Copy link
Contributor Author

@RobinMalfait Could we have the fix for this pushed to both v3 and v4 once it's ready, as it introduced breaking changes to existing v3 users? Thanks

@RobinMalfait
Copy link
Member

Hey!

You are sharing a few bug reports at once, so I'll try to address them but sorry if I missed one.

In v3, we added arbitrary value support, the goal of arbitrary values is to provide an escape hatch for users that need one-off values that don't really fit in your design system.

The arbitrary value is a value that should be valid CSS because it will be used as-is in the generated CSS. This is a very important part and we will get back to this in a minute.

This means that we have to validate the value a bit to ensure that we don't generate completely broken CSS. One example is making sure that brackets are balanced. This is useful when scanning minified files (where you don't have control over the contents), and also in cases where you missed a bracket e.g.: w-[calc(100vw-10rem]. This way we can prevent a plethora of bugs.

Back to your issues.

One example you shared is this one:

<div class="multi-[underline]">Should be underlined</div>
<div class="multi-[underline;font-bold]">Should be underlined and bold</div>
<div class="multi-[underline;font-bold;hover:font-bold]">Should be underlined, bold, and red</div>

The last example here doesn't work. This example only ever worked in an alpha version (3.0.0-alpha.2) but never worked in a real release.

The reason is because we introduced arbitrary properties (e.g.: [color:red]) in which we validate the "value" part — which is, in this case, red.

This means that we have to ensure that we don't have a : in that value position because that would mean that [color:red:foo] would be valid which would generate something like .foo { color: red:foo; } which is invalid CSS.

This is why the : is invalid in some parts of the arbitrary value position.


The other bug you mentioned is this one:

<div class="multi-[[font-family:'verdana']]">abc</div>

This behavior changed in 3.3.6 as a bug fix (#12396).

The same idea applies here where we want to make sure that the arbitrary values are validated (especially around minified files) to prevent creating invalid CSS.

That brings me to the clue of the story. We want to ensure that arbitrary values are basically valid CSS, this means that some validation needs to happen.
Again, the feature was to allow users to punch in one-off values that don't fit in the design system, but can be used as-is.

There is an exception to the rule where you can almost do anything you want, the only requirement is that you wrap it in quotes. Later in your plugin, you can parse the value yourself.

The origin of this feature is for the content utility. E.g.: content-['write_any_prose_in_here'].

An analogy of what you are trying to do: you are trying to invent GraphQL as a new language inside JS syntax and that fails in various ways. This is because we expect valid CSS values in the arbitrary values position.

Don't get me wrong, it's awesome to see that people are pushing the boundaries of what Tailwind CSS can do.

Your multi plugin is a good example of a complex system that accepts a custom language (DSL) as an input and doesn't accept "CSS" which is what the arbitrary value feature in Tailwind CSS is built for.

I'm sure that we can fix the current bugs and figure out a way to not regress on previous bugs that happened before, but I'm pretty sure it will result in a whack-a-mole situation where you will discover more and more bugs that this feature in Tailwind CSS is not designed to solve in the first place.

So my proposal is this, if you want to write plugins with arbitrary values and those arbitrary values don't map to valid CSS values but rather a custom DSL, wrap the arbitrary value in quotes and adjust the plugin to parse it. This way it is just a plain string and you should be able to put anything in there.

To account for this in your plugin:

    matchUtilities({
      multi: (value) => {
        const escape = (str) => {
          return str.replace(/_/g, '\\_').replace(/ /g, '_')
        }
-       const utilities = value.split(';').map(escape).join(' ')
+       const utilities = value.slice(1, -1).split(';').map(escape).join(' ')
        console.log({ value, utilities })
        return {
          [`@apply ${utilities}`]: {},
        }
      },
    })

Usage once implemented:

- multi-[[font-family:'comic_sans_ms']]
+ multi-["[font-family:'comic_sans_ms']"]

- multi-[!bg-green-500/30;text-green-800;hover:bg-purple-500/30;hover:text-purple-300]
+ multi-['!bg-green-500/30;text-green-800;hover:bg-purple-500/30;hover:text-purple-300']

So while this doesn't solve the issue you are facing, I hope the reasoning behind the change makes sense and allows you to adjust your plugin.

Hope this helps!

@brandonmcconnell
Copy link
Contributor Author

brandonmcconnell commented Apr 16, 2024

@RobinMalfait Thanks for the thorough feedback!

For that last example, am I correct in assuming that even in quotes, multi-['!bg-green-500/30;text-green-800;hover:bg-purple-500/30;hover:text-purple-300'] would not work since hover:text-purple-300 contains a : and would essentially use @apply hover:text-purple-300 which might break due to the change you mentioned was implemented to support arbitrary properties (e.g. [color:red])?

A few ideas for ways we may be able to combat this:

  • Perform validation of plugin values based on the values returned by those plugins, rather than the values before being processed. Wrap the plugin executions in a try-catch, and if what comes back is invalid, ignore it then, rather than skipping the rule altogether.

  • Introduce a new plugin return type that overrides how the parser attempts to parse values from plugins that use it. Both of the meta-plugins I mentioned here could specify a type like 'unknown', 'arbitrary', or even 'invalid' to be more explicit and inform the parser that the value(s) being used should accept commonly invalid. I would then use this type in both of the plugins in question here, multi-[] and js-[], so the parser would return the values used as strings for the plugin to process further. This approach could yield a much better developer experience (DX) for my users, as requiring quotations to wrap the values feels a bit clunky.

    I totally understand the need, and I think this could prove a valuable way of telling the parser, "I understand the risk, but I would like to accept invalid values". In some cases, I throw errors in my plugins to completely reject unexpected values anyway, if any are encountered, to make it clear to the user that some values being used are not accepted.

    This could blend nicely with the first suggestion— the parser could ignore invalid values only for utilities/variants used by plugins associated with this new return type.

    I think another justification here could be that each of these values is, in fact, valid CSS when used as the value for a CSS custom property. For example, this is valid in CSS and does not get invalidated by any browser/engine that I have tested so far:

    * { --value: (!bg-green-500/30;text-green-800;hover:bg-purple-500/30;hover:text-purple-300) }

    I've tested and confirmed across all three major browser engines— Blink, Gecko, and WebKit

    Blink/Chrome Gecko/Firefox WebKit/Safari
    Chrome Firefox Safari
  • An alternative to prescribing a new type as mentioned in my second bullet, the type property could still list an expected "final return type" (what will be returned by the plugin), such as 'length', along with a new property like supportsInvalidValues (similar to supportsNegativeValues) that explicitly tells the parser to skip any validation warnings in values used by that plugin.

    I can see some value in this method, though even with this, I could still see it being considered useful to add a new type for that situation I described where you are stuffing arbitrary content into a CSS custom property value.

All three of these blended together might look like this:

/* e.g. for arbitrary custom property values */
plugin(({ matchUtilities, theme }) => { 
  matchUtilities(
    {
      custom: value => ({ '--custom': value })
    },
    {
      type: ['arbitrary'],
      supportsInvalidValues: true,
    }
  )
})

/* e.g. for arbitrary values that compute to length values */
plugin(({ matchUtilities, theme }) => { 
  matchUtilities(
    {
      custom: value => ({ 'width': customFunction(value) })
    },
    {
      type: ['length'],
      supportsInvalidValues: true,
    }
  )
})

I recognize this type property may have been temporary, as I don't see it in the docs, and I get TS errors when attempting to test with it.

@brandonmcconnell
Copy link
Contributor Author

I'm not sure if the same limitation mentioned above (support for [color:red]) is what prevents us from using apply statements like @apply hover:font-bold bur if it is, could that also be accounted for with bracket counting, where [color:red] and hover:font-bold could be differentiable based on their usage and depth of brackets?

@RobinMalfait
Copy link
Member

RobinMalfait commented Apr 16, 2024

Hey!

Just tried to answer some of your questions below:

For that last example, am I correct in assuming that even in quotes, multi-['!bg-green-500/30;text-green-800;hover:bg-purple-500/30;hover:text-purple-300'] would not work since hover:text-purple-300 contains a : and would essentially use @apply hover:text-purple-300 which might break due to the change you mentioned was implemented to support arbitrary properties (e.g. [color:red])?

No that's not correct, the validation only happens for validating the data inside the square brackets foo-[…]. The @apply feature just works with :, e.g.: @apply hover:underline
Here is an example: https://play.tailwindcss.com/8IzDkVLwmZ


Perform validation of plugin values based on the values returned by those plugins, rather than the values before being processed. Wrap the plugin executions in a try-catch, and if what comes back is invalid, ignore it then, rather than skipping the rule altogether.

Imo, that's a bit backwards, you typically always want to validate user input up front instead of validating the response you are going to generate. While we could do this, this would make everything way slower even if 99.999% of the generated CSS is going to be correct because some code has to run over the generated rules to validate them.

If we don't validate upfront and only at the end, then each plugin has to do its own validation. We can do that for core plugins, but we can't guarantee that for 3rd party plugins. This means that eventually each plugins has to do validation, and after the calling the plugin, we would have to do another pass to validate if it's actually correct.


Introduce a new plugin return type that overrides how the parser attempts to parse values from plugins that use it. Both of the meta-plugins I mentioned here could specify a type like 'unknown', 'arbitrary', or even 'invalid' to be more explicit and inform the parser that the value(s) being used should accept commonly invalid. I would then use this type in both of the plugins in question here, multi-[] and js-[], so the parser would return the values used as strings for the plugin to process further. This approach could yield a much better developer experience (DX) for my users, as requiring quotations to wrap the values feels a bit clunky.

The types on plugins are only used for arbitrary values (the values passed in brackets). The reason this exists is so that plugins with the same "root" (e.g.: bg) can handle their own sets of types.

For example:

  • bg-[#0088cc] — this will end up in the "background color" plugin, because one of its types is set to color and we can infer that from the value.
  • bg-[url(…)] — this will end up in the "background image" plugin, because one of its types is set to url and we can infer that from the value.

I think another justification here could be that each of these values is, in fact, valid CSS when used as the value for a CSS custom property. For example, this is valid in CSS and does not get invalidated by any browser/engine that I have tested so far:

* { --value: (!bg-green-500/30;text-green-800;hover:bg-purple-500/30;hover:text-purple-300) }

This unfortunately is a bit of a bad example because it's true that CSS custom properties can basically retrieve any value. However, this can only ever be used in CSS custom properties position.

Apart from that, again this would make the system way to complex for what it's worth. Not only for parsing the values but also picking out the values from your content files. If we allow anything, then there is no real grammar for what Tailwind CSS classes mean. This means that we have to handle way more potential classes that we have to throw away.


I'm not sure if the same limitation mentioned above (support for [color:red]) is what prevents us from using apply statements like @apply hover:font-bold bur if it is, could that also be accounted for with bracket counting, where [color:red] and hover:font-bold could be differentiable based on their usage and depth of brackets?

Yep, so this is not an issue as stated earlier.


So to summarise, wrapping the arbitrary value in quotes only requires you to add the .slice(1,-1) from a plugin perspective. Everything else will work as expected.

Allowing any value, and validating the "output" of a plugin will make everything more complex and way slower without much benefit. The only difference for your plugin is the need for quotes, once they are there everything should work as expected.

@brandonmcconnell
Copy link
Contributor Author

brandonmcconnell commented Apr 16, 2024

@RobinMalfait Thanks again

I'm not proposing that each plugin does its own validation, but rather that plugins be given the ability to opt into self-validation, with the accepted trade-off of being a bit slower.

This would be especially useful for plugins like these two, where users are already prioritizing DX over performance, as Multi can potentially bloat stylesheets except when used with truly unique utilities (e.g. long variant chains under keyed group, etc.)

We agree that quotations are a small trade-off and would be easy to account for in my plugin. My reason for surfacing this concern now is not any additional effort on my part but rather ensuring a better DX (and fewer gotchas) for my users.

The idea behind Multi is that any utilities that work on their own should be able to be added into a multi-[] and delimited by semicolons and just work. Adding quotes adds a few hurdles:

  • 3.3.6 introduced a breaking change affecting my plugin users, who are waiting to upgrade.This is more of an edge case, admittedly, but often, breaking changes are reserved for larger version changes, like v3 ➞ v4, in the heart of "Don't break the web".
  • If I do switch to requiring quoted values…
    • I would mention the syntax change in a release note and the README as well so that anyone experiencing the issue sees it and makes the change. Anyone who continues to upgrade hoping it will just eventually work without checking these notes would not see the change and may uninstall the dependency, considering it broken and likely unused, as they'd have been going some time without using it.

    • This new syntax would actually break the original idea of utilities being able to just be added into a multi and work.

      How would these be consolidated using Multi?

      <span class="group/cost:color-green-700 group/cost:font-bold group/cost:before:content-['$']">5.42</span>

      Previously, this would have worked:

      <span class="group/cost:multi-[color-green-700;font-bold;before:content-['$']]">5.42</span>

      but the new quotations would break the quotes used in content-['$']:

      <span class="group/cost:multi-[color-green-700;font-bold;before:content-['$']]">5.42</span>

      This would require some fashion of escaping the quoted value. This also builds in complexity if one quoted value ever needed to wrap another quoted value.

      Simply for the sake of example, this would have worked previously without any issues:

      <span class="group/cost:multi-[multi-[color-green-700];multi-[font-bold]]">5.42</span>

      But the quotes might prove problematic when nested:

      <span class="group/cost:multi-['multi-['color-green-700'];multi-['font-bold']']">5.42</span>

      This exact example isn't a realistic one, but something like this would be very common or even expected if most plugins need to resort to using quotes, preventing one quoted value from encapsulating another.
      Or would this actually work as expected, thanks to the parser's bracket/quotations/etc? balancing? In other words, would 'multi-['color-green-700'];multi-['font-bold']' be passed to the Multi plugin as the string multi-['color-green-700'];multi-['font-bold'], or would the nested single-quotes break the validity of the value?
      Even if there were a way to escape these values, even in a nested manner, this could get very cumbersome, possibly requiring double-escaping both the quotes and their respective backslashes used for escaping:

       <span class="group/cost:multi-['multi-[\'color-green-700\'];multi-[before:content-[\\\'$\\\']]']">5.42</span>

      Even for a single level of quote-nesting, like in the case of using content, this may be perceived to remove some of the beneficial trade-offs of using the plugin, as many would see this as overly complex:

      <span class="group/cost:multi-['color-green-700;font-bold;before:content-[\'$\']']">5.42</span>

@brandonmcconnell
Copy link
Contributor Author

My preference here would still be to allow plugins to opt into arbitrary content, which would open up what is possible with plugins even outside of the usual Tailwind grammars.

However, it would appear that nested quotes do work in most cases, though I am trying to figure out why these both work:

<div class="multi-['font-bold;text-green-700;before:content-['$']']">5.42</div>
<div class="multi-['multi-['multi-['font-bold']'];text-green-700;before:content-['$']']">5.42</div>

but this does not:

<div class="multi-['multi-['multi-['font-bold']'];text-green-700;multi-['before:content-['$']']']">5.42</div>

Inspect these examples on Tailwind Play:
https://play.tailwindcss.com/9TJ6ZBNdYJ

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

No branches or pull requests

2 participants