Git Product home page Git Product logo

Comments (12)

RobinMalfait avatar RobinMalfait commented on May 29, 2024 1

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!

from tailwindcss.

brandonmcconnell avatar brandonmcconnell commented on May 29, 2024

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.

from tailwindcss.

brandonmcconnell avatar brandonmcconnell commented on May 29, 2024

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

from tailwindcss.

brandonmcconnell avatar brandonmcconnell commented on May 29, 2024

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]

from tailwindcss.

brandonmcconnell avatar brandonmcconnell commented on May 29, 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.

from tailwindcss.

brandonmcconnell avatar brandonmcconnell commented on May 29, 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)

from tailwindcss.

brandonmcconnell avatar brandonmcconnell commented on May 29, 2024

@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

from tailwindcss.

brandonmcconnell avatar brandonmcconnell commented on May 29, 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.

from tailwindcss.

brandonmcconnell avatar brandonmcconnell commented on May 29, 2024

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?

from tailwindcss.

RobinMalfait avatar RobinMalfait commented on May 29, 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.

from tailwindcss.

brandonmcconnell avatar brandonmcconnell commented on May 29, 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>

from tailwindcss.

brandonmcconnell avatar brandonmcconnell commented on May 29, 2024

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

from tailwindcss.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.