Fix duplicate -webkit-backdrop-filter output
#850
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: LeoniePhiline <22329650+LeoniePhiline@users.noreply.github.com>
If you have the following CSS:
```css
.foo {
transition-property: backdrop-filter;
}
```
Then the `backdrop-filter` will be prefixed such that it looks like
this:
```css
.foo {
transition-property: -webkit-backdrop-filter, backdrop-filter;
}
```
However, if you already have `-webkit-backdrop-filter` in the list:
```css
.foo {
transition-property: -webkit-backdrop-filter, backdrop-filter;
}
```
Then it compiles to:
```css
.foo {
transition-property: -webkit-backdrop-filter, -webkit-backdrop-filter, backdrop-filter;
}
```
This is not what we want. So in this case, whenever a non-prefixed
property is found in the list (`backdrop-filter`), then we can remove or
ignore all prefixed values. Once we are printing the CSS back, the
`backdrop-filter` will print the prefixed version as well.
```css
.foo {
transition-property: -webkit-backdrop-filter, backdrop-filter;
}
```
RobinMalfait
added a commit
to tailwindlabs/tailwindcss
that referenced
this pull request
Nov 6, 2024
This PR improves the generated CSS by running it through Lightning CSS twice.Right now Lightning CSS merges adjacent at-rules and at the end flattens the nesting. This means that after the nesting is flattened, the at-rules that are adjacent and could be merged together will not be merged. This PR improves our output by running Lightning CSS twice on the generated CSS which will make sure to merge adjacent at-rules after the nesting is flattened. Note: in the diff output you'll notice that some properties are duplicated. These need some fixes in Lightning CSS itself but they don't break anything for us right now. Related PR in Lightning CSS for the double `-webkit-backdrop-filter` can be found here: parcel-bundler/lightningcss#850 --------- Co-authored-by: Philipp Spiess <hello@philippspiess.com>
…inMalfait/lightningcss into RobinMalfait-fix/duplicate-webkit-backdrop-filter
|
Thanks, and sorry for the delay on review. I updated this to handle a couple more cases:
|
Very kind – thank you! ❤️ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes an issue where using CSS that looks like this:
Results in CSS that looks like this:
This PR solves it for the
transition-property(internallyTransitionProperty, so I think it's also solved fortransitionthat uses that) only.We currently do this by checking whether a
PropertyIdexists withVendorPrefix::None. If it does, then we will make sure to remove all otherPropertyId's (with the same name). The idea is that thePropertyIdwithVendorPrefix::Nonewill print the property with the correct values, including the vendor prefix.If however you only use CSS that looks like this:
Which is without the unprefixed version, then this is maintained in the output because no
VendorPrefix::Noneexists in this case.I feel like there might be a better spot to handle this (maybe even in parsing, or during printing) but wasn't to sure. I also explicitly scoped it to the
transition-propertyfor now but maybe this can be more generalized?I also started from the failing tests (and slightly adjusted with more test cases) from this PR: #551, I also made sure that the original author is marked as a co-author.
Fixes: #403
Closes: #551