Skip to content

Commit

Permalink
Avoid splitting and vendor prefixing :is() when we can safely unwrap it
Browse files Browse the repository at this point in the history
Fixes #509
  • Loading branch information
devongovett committed Jun 17, 2023
1 parent fbf0d24 commit 8d94ea1
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 16 deletions.
2 changes: 1 addition & 1 deletion src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ impl<'i, 'o> PropertyHandlerContext<'i, 'o> {
ltr: Vec::new(),
rtl: Vec::new(),
context,
unused_symbols: self.unused_symbols
unused_symbols: self.unused_symbols,
}
}

Expand Down
55 changes: 55 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9576,6 +9576,61 @@ mod tests {
..Browsers::default()
},
);

prefix_test(
r#"
.foo :is(.bar) {
color: red;
}
"#,
indoc! {r#"
.foo .bar {
color: red;
}
"#},
Browsers {
chrome: Some(87 << 16),
..Browsers::default()
},
);

prefix_test(
r#"
.foo :is(.bar), .bar :is(.baz) {
color: red;
}
"#,
indoc! {r#"
.foo .bar, .bar .baz {
color: red;
}
"#},
Browsers {
chrome: Some(87 << 16),
..Browsers::default()
},
);

prefix_test(
r#"
.foo :is(.bar:focus-visible), .bar :is(.baz:hover) {
color: red;
}
"#,
indoc! {r#"
.bar .baz:hover {
color: red;
}

.foo .bar:focus-visible {
color: red;
}
"#},
Browsers {
chrome: Some(85 << 16),
..Browsers::default()
},
);
}

#[test]
Expand Down
6 changes: 4 additions & 2 deletions src/properties/transition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,8 @@ fn expand_properties<'i>(

// Expand mask properties, which use different vendor-prefixed names.
if let Some(property_id) = get_webkit_mask_property(&properties[i]) {
if context.targets
if context
.targets
.prefixes(VendorPrefix::None, Feature::MaskBorder)
.contains(VendorPrefix::WebKit)
{
Expand All @@ -387,7 +388,8 @@ fn expand_properties<'i>(
rtl_properties[i].set_prefixes_for_targets(context.targets);

if let Some(property_id) = get_webkit_mask_property(&rtl_properties[i]) {
if context.targets
if context
.targets
.prefixes(VendorPrefix::None, Feature::MaskBorder)
.contains(VendorPrefix::WebKit)
{
Expand Down
10 changes: 6 additions & 4 deletions src/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ use self::font_palette_values::FontPaletteValuesRule;
use self::layer::{LayerBlockRule, LayerStatementRule};
use self::property::PropertyRule;
use crate::context::PropertyHandlerContext;
use crate::declaration::{DeclarationHandler, DeclarationBlock};
use crate::declaration::{DeclarationBlock, DeclarationHandler};
use crate::dependencies::{Dependency, ImportDependency};
use crate::error::{MinifyError, ParserError, PrinterError, PrinterErrorKind};
use crate::parser::{
Expand Down Expand Up @@ -672,15 +672,17 @@ impl<'i, T: Clone> CssRuleList<'i, T> {
// If the rule has nested rules, and we have extra rules to insert such as for logical properties,
// we need to split the rule in two so we can insert the extra rules in between the declarations from
// the main rule and the nested rules.
let nested_rule = if !style.rules.0.is_empty() && (!logical.is_empty() || !supports.is_empty() || !incompatible_rules.is_empty()) {
let mut rules = CssRuleList(vec![]);
let nested_rule = if !style.rules.0.is_empty()
&& (!logical.is_empty() || !supports.is_empty() || !incompatible_rules.is_empty())
{
let mut rules = CssRuleList(vec![]);
std::mem::swap(&mut style.rules, &mut rules);
Some(StyleRule {
selectors: style.selectors.clone(),
declarations: DeclarationBlock::default(),
rules,
vendor_prefix: style.vendor_prefix,
loc: style.loc
loc: style.loc,
})
} else {
None
Expand Down
2 changes: 1 addition & 1 deletion src/rules/style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ impl<'i, T: Clone> StyleRule<'i, T> {
}

self.vendor_prefix = get_prefix(&self.selectors);
if self.vendor_prefix.contains(VendorPrefix::None) & context.targets.should_compile_selectors() {
if self.vendor_prefix.contains(VendorPrefix::None) && context.targets.should_compile_selectors() {
self.vendor_prefix = downlevel_selectors(self.selectors.0.as_mut_slice(), *context.targets);
}

Expand Down
35 changes: 27 additions & 8 deletions src/selector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1479,12 +1479,9 @@ where
Component::Where(..) => dest.write_str(":where(")?,
Component::Is(ref selectors) => {
// If there's only one simple selector, serialize it directly.
if selectors.len() == 1 {
let first = selectors.first().unwrap();
if !has_type_selector(first) && is_simple(first) {
serialize_selector(first, dest, context, false)?;
return Ok(());
}
if should_unwrap_is(selectors) {
serialize_selector(selectors.first().unwrap(), dest, context, false)?;
return Ok(());
}

let vp = dest.vendor_prefix;
Expand Down Expand Up @@ -1544,6 +1541,17 @@ where
}
}

fn should_unwrap_is<'i>(selectors: &Box<[Selector<'i>]>) -> bool {
if selectors.len() == 1 {
let first = selectors.first().unwrap();
if !has_type_selector(first) && is_simple(first) {
return true;
}
}

false
}

fn serialize_nesting<W>(
dest: &mut Printer<W>,
context: Option<&StyleContext>,
Expand Down Expand Up @@ -1735,7 +1743,15 @@ pub(crate) fn is_compatible(selectors: &[Selector], targets: Targets) -> bool {
}

// These support forgiving selector lists, so no need to check nested selectors.
Component::Is(_) | Component::Where(_) | Component::Nesting => Feature::IsSelector,
Component::Is(selectors) => {
// ... except if we are going to unwrap them.
if should_unwrap_is(selectors) && is_compatible(selectors, targets) {
continue;
}

Feature::IsSelector
}
Component::Where(_) | Component::Nesting => Feature::IsSelector,
Component::Any(..) => Feature::AnyPseudo,
Component::Has(selectors) => {
if !targets.is_compatible(Feature::HasSelector) || !is_compatible(&*selectors, targets) {
Expand Down Expand Up @@ -1946,7 +1962,10 @@ fn downlevel_component<'i>(component: &mut Component<'i>, targets: Targets) -> V

// Convert :is to :-webkit-any/:-moz-any if needed.
// All selectors must be simple, no combinators are supported.
if should_compile!(targets, IsSelector) && selectors.iter().all(|selector| !selector.has_combinator()) {
if should_compile!(targets, IsSelector)
&& !should_unwrap_is(selectors)
&& selectors.iter().all(|selector| !selector.has_combinator())
{
necessary_prefixes |= targets.prefixes(VendorPrefix::None, crate::prefixes::Feature::AnyPseudo)
} else {
necessary_prefixes |= VendorPrefix::empty()
Expand Down

0 comments on commit 8d94ea1

Please sign in to comment.