-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Ensure @apply
of a rule inside an AtRule works
#6594
Conversation
If that atrule happens to contain another rule that is technically unrelated. Co-authored-by: Jordan Pittman <[email protected]>
9f4be07
to
f36b6c9
Compare
Not sure that this is related, but with the latest
Seems like this PR is the only thing that might be related to reading parent. Is that correct? |
@driskull hmm can you create a minimal reproduction repo and open a new issue? |
// therefore it removed the selector altogether. In this specific | ||
// case it would result in `{}` instead of `.something-unrelated {}` | ||
if (!extractClasses(rule).some((thing) => thing === applyCandidate)) { | ||
rule.remove() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RobinMalfait I'm not sure what exactly the issue is. I'll keep investigating.
My best guess is that something else is missing besides just removing the rule. Maybe the parent still has a reference to the rule which would cause it to be undefined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I do the following, it no longer errors but I'm just shooting in the dark here...
rule.before(rule.nodes)
rule.remove();
return;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please open an issue with a reproduction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR will ensure that if you are using
@apply something
and thatsomething
happens to be in an AtRule and that AtRule contains an unrelated node that we only apply the related node... 🥵Let's imagine you have the following structure:
In this case we want to apply
.bar
but it happens to be in an AtRule node. We clone that node instead of the nested one because we still want that@supports
rule to be there once we applied everything.However it happens to be that the
.something-unrelated
is also in that same shared@supports
atrule. This is not good, and this should not be there. The good part is that this is a clone already and it can be safely removed. The question is how do we know we can remove it. Basically what we can do is match it against the applyCandidate that you want to apply. If it doesn't match the we can safely delete it.If we didn't do this, then the
replaceSelector
function would have replaced this with something that didn't exist and therefore it removed the selector altogether. In this specific case it would result in{}
instead of.something-unrelated {}
Fixes: #6246