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

var(--tw-empty,/*!*/ /*!*/) could be wrongly processed by externals preprocessors #2889

Closed
hanoii opened this issue Nov 24, 2020 · 3 comments

Comments

@hanoii
Copy link

hanoii commented Nov 24, 2020

While not a bug per se of tailwind, I did want to flag this and I wonder if there's not a better way of doing this.

I am using Drupal that has an option to preprocess css files. It seems that as part of that process comments are removed.

This leaves something like

--tw-ring-inset: var(--tw-empty,/*!*/ /*!*/);

as

--tw-ring-inset: var(--tw-empty,);

Which shows as invalid to chrome and breaks the ring functionality (no rings shown).

I am sure there's a reason for that choice of empty default value but I wonder if something less prone to something like this can be used.

For now I flagged the css of my theme (which is already preprocessed anyway) from being further processed by Drupal and this made it work. But wanted to raise this up for consideration.

@RobinMalfait
Copy link
Member

Hey! Thank you for your bug report!
Much appreciated! 🙏

The sad part however is that a lot of minifiers don't consider the valid case that an empty space ( ) is a valid value for custom property. var(--tw-empty, ) is valid and so is --tw-empty: ;.

We use this technique to make composing multiple classes possible. In case of the ring offset, we use this so that you can define rings and ring offsets, but the offset is not required and we default it to an empty space.
Initially you could think that we should use var(--tw-empty, " ") but this results in the literal (" ") not ( ). If we don't have a fallback than the final box-shadow value would be invalid.

We also tried a bunch of other solutions, but with this one we got the best success so far. Some other solutions we tried:

--tw-empty: ;
var(--tw-empty, );

--tw-empty: /**/;
var(--tw-empty, /**/);

--tw-empty: /*!*/;
var(--tw-empty, /*!*/);

--tw-empty: /**/ /**/;
var(--tw-empty, /**/ /**/);

--tw-empty: !important;
var(--tw-empty);

One way we can solve this for the --tw-ring-inset is that we use a default 0px for example, but there are other places in tailwind that uses the same technique (e.g.: font-variant numeric https://tailwindcss.com/docs/font-variant-numeric) where we can't default to a valid value.

That said, we got it fixed in postcss itself already, so tools that are built on top of that can benefit from it postcss/postcss#1404

So I don't think we can fix this anytime soon.

@MartijnCuppens
Copy link
Contributor

For those in need for a temporary fix, this 'll do the trick in Drupal:

function YOUR_THEME_preprocess_html(&$variables) {
  $filters = [
    'blur',
    'brightness',
    'contrast',
    'grayscale',
    'hue-rotate',
    'invert',
    'saturate',
    'sepia',
    'drop-shadow',
  ];
  $css = '*, ::before, ::after {';
  foreach ($filters as $value) {
    $css .= "--tw-{$value}: var(--tw-empty,/*!*/ /*!*/);";
  }
  $css .= '}';

  $variables['#attached']['html_head'][] = [
    [
      '#type' => 'html_tag',
      '#tag' => 'style',
      '#value' => $css,
    ],
    'tailwind-custom-properties',
  ];
}

Please note this is a workaround which just adds additional CSS.

@MichaelAllenWarner
Copy link
Contributor

Filed a Drupal issue about this here: https://www.drupal.org/project/drupal/issues/3238341

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

4 participants