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

Not recognizing selector:not(.class-name) unless class-name exists in content #7750

Closed
adamwathan opened this issue Mar 5, 2022 Discussed in #7747 · 4 comments · Fixed by #7815
Closed

Not recognizing selector:not(.class-name) unless class-name exists in content #7750

adamwathan opened this issue Mar 5, 2022 Discussed in #7747 · 4 comments · Fixed by #7815
Assignees

Comments

@adamwathan
Copy link
Member

Discussed in #7747

Originally posted by rjschie March 4, 2022
I'm not sure that this is a bug per-se, so I wanted to bring it up in a discussion first. If you have a rule in a layer, or plugin, that includes a :not(.class-name), it will not add it to the output if class-name is not found in the content. Once you add class-name it will recognize it, add it, and continue keeping it throughout that build (but once you stop the watcher and re-start it will revert back).

@tailwind base;
@tailwind components;
@tailwind utilities;

@layer base {
  div:not(.class-name) {
    background-color: magenta;
  }
}
<div class="p-4"> <!-- does not have magenta bg -->
  Some content
</div>

You can see an example here: https://play.tailwindcss.com/fSaAQLAJ2i

In that playground, you'll see that the div's background is not magenta as you might expect, but if you add class-name to the class attribute, and then remove it, it will turn magenta.

Adding safelist: ['class-name'] to the config will work around this issue, but is this unintuitive? Is it a bug?

@mrinx
Copy link
Contributor

mrinx commented Mar 10, 2022

I wonder how much this is a bug though. It seems to be working for elements, but not for classes.

@layer base {
  :not(p) {
    background-color: magenta;
  }
}

I'd argue that if the class-name is never ever used in the code, the selector used should be just div and not div:not(.class-name), because no div could ever have that class-name, therefore div and div:not(.class-name) would be almost equivalent (except their specificity). Unless the class-name comes from some outside source, but then it should be manually whitelisted as would be normally done for classes like that now.

So even though in principle there's nothing wrong with using selectors like div:not(.class-name-that-will-never-exist), I see it more like a syntactic sugar, or rather syntactic dessert and I wonder if the JIT compiler should cater for that.

Am I missing something?

@RobinMalfait RobinMalfait self-assigned this Mar 11, 2022
@RobinMalfait
Copy link
Member

That's a great point @mrinx

Currently, if you are using classes that don't exist in the actual code base, then you have to safelist them. If they do exist, then everything will work as expected.

If you have code like:

@layer base {
  div:not(.some-class) {}
}

And we follow the same rules, then some-class should exist somewhere otherwise it won't get generated.

The reason it works for just this:

@layer base {
  div:not(p) {}
}

... is because it doesn't include any classes. Internally we call everything that has a class "on-demandable" and the others, well, are not and will always be generated.

We could have a special case where we ignore everything inside a :not and then check whether we should generate these rules or not.

My initial gut says that this is not a bug and works as expected. But as @adamwathan's demo shows, this can lead to super annoying results because right now the div doesn't have a background color, but if you all of a sudden use .class-name in an unrelated file it becomes magenta.


I'll provide a PR to fix this, and then we can evaluate whether we want this behaviour or not.

@meanspa
Copy link

meanspa commented Mar 11, 2022

For my part, I'd like to add another vote for this being considered a bug. I actually found this issue because it's already been a problem in the project I'm working on. Here's my perspective (apologies for the redundancy with what's already been shared, just spelling it all out to make sure I understand the behavior correctly):

Example Code:

@layer base {
  div:not(.fancy) {
    background-color: #999999;
  }
}
  • If we're using the fancy class throughout our HTML, this works great, any div without that class has the styles I expect it to have. But if at some point the last HTML element that has the fancy class is removed, the styles will be changed for all of the divs that never had that class to begin with.
  • When I'm working with CSS, I know that when I remove a class from an HTML element I need to consider how it will effect the styling of that element. With the current behavior, I need to consider how removing a class might effect elements that never had that class to begin with.
  • Likewise, when I'm working with CSS, I know that if I remove a class from my stylesheets, I need to anticipate and test how that will effect everything that uses that class globally, throughout the project. With the current behavior, I need to have this same kind of global awareness whenever I remove a class from an html element. Because removing a class from one HTML element can have a global impact on the project.
  • In a broad sense, within a @layer, the current behavior deviates from standard CSS behavior, which I see as a bad thing (when it can be avoided)
  • This is one of those "gotcha"'s that, once you know it, might not seem like a huge deal. However especially when considering development over the long term, with multiple devs, etc., I think minimizing those gotchas is pretty important. Especially because this particular issue is something that the developer will probably be totally unaware of when they're initially writing the code (I wouldn't have written that rule if I wasn't already using the fancy class), so there's a good chance I'm not going to use the work around, and then it becomes a "gotcha" that's sort of lying in wait, until it comes up years down the road and has a very broad impact on the project in a way that's hard to google, and, in my opinion, not intuitive.

Edit: After re-reading the earlier comments and the docs, this seems like it's somewhat of a philosophical question so I'm just going to leave one more example/analogy to try and explain what I mean in case it's helpful.

Let's pretend we're building a bridge, and the rules we make are perfectly enforced:

  • If we have a rule that says "no vehicles can drive on the bridge if they are more than X pounds" and there are no vehicles that heavy in the world, then that rule can go away, it's meaningless.
  • BUT - if the rule is "as long as you're not more than X pounds, you can drive on this bridge" then it doesn't matter whether those vehicles exist or not, if that rule goes away nobody can drive on the bridge.
  • Tailwind's current behavior removes both rules, which seems like a bad thing to me.
  • In essence, I'm trying to say a CSS rule that uses:not() isn't about the thing that's in the (), it's about everything else. So I don't think pruning the rule based on what's in the () makes sense, because everything else is still there...

@RobinMalfait
Copy link
Member

This should be fixed, and will be available in the next release.

You can already try it by using the insiders build npm install tailwindcss@insiders or yarn add tailwindcss@insiders.

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 a pull request may close this issue.

4 participants