From b7bae5287375c2165e8fbb46afae7470bf72cd37 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 14:19:54 +0000 Subject: [PATCH 1/3] css: keep declarations written after a nested rule in their cascade position Declarations that followed a nested rule were parsed into the enclosing style rule's own declaration block, which hoisted them above the nested rules they were written after. `a { color: red; & { color: blue } color: green }` therefore printed the `green` block before the `blue` one and rendered blue. Declarations directly inside a nested conditional at-rule were wrapped in a `& { }` rule, which also changed their specificity for selector lists. Parse both into a new CssRule::NestedDeclarations rule that keeps its position among the nested rules (https://drafts.csswg.org/css-nesting/#nested-declarations-rule). When nesting is compiled away it prints as a rule under the enclosing rule's selectors (repeated as written, not wrapped in :is()); when nesting is preserved it prints its declarations in place, keeping the trailing `;` whenever more output follows it in the block, including through an unwrapped `@media all`. Minifying it runs the declarations through the property handlers and inserts the logical property, @supports and dark color scheme fallback rules they generate right after it, nested under `&`, so those fallbacks keep their position too. The declarations still count towards the CSS modules `composes` property usage of the rule. --- src/css/context.rs | 9 + src/css/css_modules.rs | 6 +- src/css/css_parser.rs | 94 +++++-- src/css/printer.rs | 6 + src/css/rules/media.rs | 3 +- src/css/rules/mod.rs | 120 ++++++++- src/css/rules/nesting.rs | 103 +++++++- src/css/rules/style.rs | 160 ++++++----- test/bundler/css/css-modules.test.ts | 53 ++++ test/js/bun/css/nested-declarations.test.ts | 278 ++++++++++++++++++++ 10 files changed, 702 insertions(+), 130 deletions(-) create mode 100644 test/js/bun/css/nested-declarations.test.ts diff --git a/src/css/context.rs b/src/css/context.rs index e8d17e5f72a8..904f0da479b0 100644 --- a/src/css/context.rs +++ b/src/css/context.rs @@ -80,6 +80,15 @@ impl<'a> PropertyHandlerContext<'a> { self.rtl.push(rtl); } + /// Whether the handlers staged anything that `get_supports_rules` / + /// `get_additional_rules` would turn into rules. + pub(crate) fn has_fallback_rules(&self) -> bool { + !(self.supports.is_empty() + && self.ltr.is_empty() + && self.rtl.is_empty() + && self.dark.is_empty()) + } + pub(crate) fn should_compile_logical(&self, feature: css::compat::Feature) -> bool { // Don't convert logical properties in style attributes because // our fallbacks rely on extra rules to define --ltr and --rtl. diff --git a/src/css/css_modules.rs b/src/css/css_modules.rs index c342a548d35e..9e68cb1e9885 100644 --- a/src/css/css_modules.rs +++ b/src/css/css_modules.rs @@ -172,13 +172,9 @@ impl<'a> CssModule<'a> { } pub(crate) fn handle_composes( - &mut self, - _dest: &mut css::Printer, + &self, selectors: &css::selector::parser::SelectorList, - _composes: &css::css_properties::css_modules::Composes, - _source_index: u32, ) -> css::Maybe<(), css::PrinterErrorKind> { - // let bump = dest.arena; for sel in selectors.v.slice() { if sel.len() == 1 && matches!( diff --git a/src/css/css_parser.rs b/src/css/css_parser.rs index 3e1c9d02e635..7d2d3081f286 100644 --- a/src/css/css_parser.rs +++ b/src/css/css_parser.rs @@ -66,6 +66,7 @@ pub use crate::rules::{ import::{ImportConditions, ImportRule}, layer::{LayerBlockRule, LayerName, LayerStatementRule}, namespace::NamespaceRule, + nesting::NestedDeclarationsRule, style::StyleRule, supports::{SupportsCondition, SupportsRule}, unknown::UnknownAtRule, @@ -1071,7 +1072,14 @@ pub struct NestedRuleParser<'a, T: CustomAtRuleParser> { impl<'a, T: CustomAtRuleParser> NestedRuleParser<'a, T> { pub(crate) fn get_loc(&self, start: &ParserState) -> Location { - let loc = start.source_location(); + self.loc_at(start.source_location()) + } + + pub(crate) fn current_loc(&self, input: &Parser) -> Location { + self.loc_at(input.current_source_location()) + } + + fn loc_at(&self, loc: SourceLocation) -> Location { Location { source_index: self.options.source_index, line: loc.line, @@ -1520,32 +1528,18 @@ mod rule_parsers { &mut self, input: &mut Parser, ) -> CssResult> { - let srcloc = input.current_source_location(); - let loc = Location { - source_index: self.options.source_index, - line: srcloc.line, - column: srcloc.column, - }; + let loc = self.current_loc(input); // Declarations can be immediately within @media and @supports blocks - // that are nested within a parent style rule. These act the same way - // as if they were nested within a `& { ... }` block. + // that are nested within a parent style rule. They apply to the parent + // rule's elements, from that position: + // https://drafts.csswg.org/css-nesting/#nested-declarations-rule let (declarations, mut rules) = self.parse_nested(input, false)?; if declarations.len() > 0 { rules.v.insert( 0, - CssRule::Style(StyleRule { - // Arena-backed: this StyleRule lands in arena AST; bulk-free won't run Drop. - selectors: SelectorList::from_selector(Selector::from_component_in( - Component::Nesting, - bun_alloc::ArenaPtr::new(input.arena()), - )), - declarations, - vendor_prefix: VendorPrefix::default(), - rules: CssRuleList::default(), - loc, - }), + CssRule::NestedDeclarations(NestedDeclarationsRule { declarations, loc }), ); } @@ -2081,6 +2075,17 @@ mod rule_parsers { let mut usage = PropertyBitset::init_empty(); let mut custom_properties: Vec<&'static [u8]> = Vec::new(); fill_property_bit_set(&mut usage, &declarations, &mut custom_properties); + // Declarations written after a nested rule still belong to this rule's + // elements. + for rule in rules.v.iter() { + if let CssRule::NestedDeclarations(nested) = rule { + fill_property_bit_set( + &mut usage, + &nested.declarations, + &mut custom_properties, + ); + } + } let custom_properties_slice = custom_properties.slice(); @@ -2126,25 +2131,60 @@ mod rule_parsers { type Declaration = (); fn parse_value(this: &mut Self, name: &[u8], input: &mut Parser) -> CssResult<()> { - // Note: split-borrow — see `NestedComposesCtx` above. // SAFETY: `input.arena()` re-borrows the parser arena through `&self`; // detach that borrow so `input` can be re-borrowed mutably below. The - // arena outlives the parser (it owns all parsed allocations). - let arena: &Bump = unsafe { bun_ptr::detach_lifetime_ref(input.arena()) }; + // arena outlives the parser (it owns all parsed allocations); `'static` + // is the crate-wide erasure of its lifetime (see `parse_nested`). + let arena: &'static Bump = unsafe { bun_ptr::detach_lifetime_ref(input.arena()) }; + + // A declaration after a nested rule has to stay behind that rule in the + // cascade, so rather than joining the block's own declarations it goes + // into a nested declarations rule at the end of the block's rules, + // continuing the one the previous declaration started if there is one: + // https://drafts.csswg.org/css-nesting/#nested-declarations-rule + let mut nested = match this.rules.v.pop() { + None => None, + Some(CssRule::NestedDeclarations(nested)) => Some(nested), + Some(rule) => { + this.rules.v.push(rule); + Some(NestedDeclarationsRule { + declarations: DeclarationBlock::new_in(arena), + loc: this.current_loc(input), + }) + } + }; + let (declarations, important_declarations) = match &mut nested { + Some(nested) => ( + &mut nested.declarations.declarations, + &mut nested.declarations.important_declarations, + ), + None => (&mut this.declarations, &mut this.important_declarations), + }; + + // Note: split-borrow — see `NestedComposesCtx` above. let mut ctx = NestedComposesCtx { state: this.composes_state, arena, composes: &mut *this.composes, composes_refs: &mut *this.composes_refs, }; - declaration::parse_declaration_impl( + let result = declaration::parse_declaration_impl( name, input, - &mut this.declarations, - &mut this.important_declarations, + declarations, + important_declarations, this.options, &mut ctx, - ) + ); + + // `RuleBodyParser` retries a declaration that failed to parse as a nested + // style rule; a rule that received nothing must not end up in front of it. + if let Some(nested) = nested + && !nested.declarations.is_empty() + { + this.rules.v.push(CssRule::NestedDeclarations(nested)); + } + result } } diff --git a/src/css/printer.rs b/src/css/printer.rs index c37427280041..b6f28fc9e434 100644 --- a/src/css/printer.rs +++ b/src/css/printer.rs @@ -129,6 +129,11 @@ pub struct Printer<'a> { /// they are skipped while this is set and emitted once in the final pass, /// keeping the output linear in nesting depth instead of exponential. pub(crate) skip_prefixed_nested_rules: bool, + /// Set by `CssRuleList::to_css` right before printing each rule: whether + /// more output follows that rule inside the block it is printed in. A + /// nested declarations rule printed in place (nesting preserved) may only + /// drop its final `;` when nothing does. + pub(crate) more_rules_follow: bool, pub(crate) in_calc: bool, pub(crate) css_module: Option>, pub(crate) dependencies: Option>, @@ -306,6 +311,7 @@ impl<'a> Printer<'a> { col: 0, vendor_prefix: css::VendorPrefix::default(), skip_prefixed_nested_rules: false, + more_rules_follow: false, in_calc: false, css_module: None, ctx: None, diff --git a/src/css/rules/media.rs b/src/css/rules/media.rs index 691e393e676a..38ac8b7d185e 100644 --- a/src/css/rules/media.rs +++ b/src/css/rules/media.rs @@ -18,8 +18,7 @@ pub struct MediaRule { impl MediaRule { pub fn to_css(&self, dest: &mut Printer) -> Result<(), PrintErr> { if dest.minify && self.query.always_matches() { - self.rules.to_css(dest)?; - return Ok(()); + return self.rules.to_css_unwrapped(dest); } // #[cfg(feature = "sourcemap")] // dest.add_mapping(self.loc); diff --git a/src/css/rules/mod.rs b/src/css/rules/mod.rs index 78ac8c937465..bccebed9abdf 100644 --- a/src/css/rules/mod.rs +++ b/src/css/rules/mod.rs @@ -36,7 +36,7 @@ pub mod viewport; // lifetime-free here. // ─── CssRule variant table ──────────────────────────────────────────────── -// Single source of truth for the 20 typed at-rule payloads. Adding a new +// Single source of truth for the 21 typed rule payloads. Adding a new // at-rule = one line here; the enum variant + `to_css` arm + `deep_clone` // arm are generated. `Unknown`/`Custom`/`Ignored` stay a fixed tail because // their `to_css` arms are special-cased (see the note on `Custom`). @@ -118,6 +118,8 @@ css_rule_variants! { MozDocument(document::MozDocumentRule), /// A `@nest` rule. Nesting(nesting::NestingRule), + /// Declarations nested after a rule; see `NestedDeclarationsRule`. + NestedDeclarations(nesting::NestedDeclarationsRule), /// A `@viewport` rule. Viewport(viewport::ViewportRule), /// A `@custom-media` rule. @@ -421,10 +423,33 @@ impl CssRule { impl CssRuleList { pub fn to_css(&self, dest: &mut Printer) -> Result<(), PrintErr> { + self.to_css_impl(dest, false) + } + + /// Prints the rules straight into the block the list's owner sits in, for an owner whose + /// own block is elided (`@media all` when minifying). Whatever follows the owner in that + /// block, which `to_css_impl` recorded in `dest.more_rules_follow` just before printing + /// the owner, therefore also follows this list's last rule. + pub(crate) fn to_css_unwrapped(&self, dest: &mut Printer) -> Result<(), PrintErr> { + let more_rules_follow = dest.more_rules_follow; + self.to_css_impl(dest, more_rules_follow) + } + + fn to_css_impl( + &self, + dest: &mut Printer, + more_rules_follow_list: bool, + ) -> Result<(), PrintErr> { let mut first = true; let mut last_without_block = false; - - for rule in self.v.iter() { + // The other rules the loop skips still count as following their predecessors; that + // only costs a nested declarations rule ahead of them a `;` it could have dropped. + let last = self + .v + .iter() + .rposition(|rule| !matches!(rule, CssRule::Ignored)); + + for (i, rule) in self.v.iter().enumerate() { if matches!(rule, CssRule::Ignored) { continue; } @@ -476,6 +501,7 @@ impl CssRuleList { } dest.newline()?; } + dest.more_rules_follow = more_rules_follow_list || Some(i) != last; rule.to_css(dest)?; last_without_block = matches!( rule, @@ -572,6 +598,17 @@ impl CssRuleList { )?; break 'arm; } + CssRule::NestedDeclarations(decls) => { + minify_nested_declarations_arm( + decls, + &mut rules, + &mut style_rules, + &mut merge_state, + context, + parent_is_unused, + )?; + break 'arm; + } CssRule::CounterStyle(_) => {} CssRule::Scope(scpe) => { // See `Container` above: recurse so nested style rules @@ -762,12 +799,7 @@ fn minify_style_arm css::generics::DeepClone<'b>>( // re-minify re-runs the staging declarations and stages their rules // again, and the per-merge re-minify this replaces also ran before // collection, so the re-staged entries belong in this rule's extras. - if merge_state.pending_minify - && !(context.handler_context.supports.is_empty() - && context.handler_context.ltr.is_empty() - && context.handler_context.rtl.is_empty() - && context.handler_context.dark.is_empty()) - { + if merge_state.pending_minify && context.handler_context.has_fallback_rules() { flush_pending_style_merge(rules, merge_state, context); } @@ -898,6 +930,44 @@ fn minify_style_arm css::generics::DeepClone<'b>>( Ok(()) } +fn minify_nested_declarations_arm css::generics::DeepClone<'b>>( + decls: &mut nesting::NestedDeclarationsRule, + rules: &mut Vec>, + style_rules: &mut StyleRuleKeyMap, + merge_state: &mut StyleRuleMergeState, + context: &mut MinifyContext<'_, '_>, + parent_is_unused: bool, +) -> Result<(), MinifyErr> { + // These declarations are as dead as the enclosing rule's own ones, which + // `StyleRule::minify` dropped. + if parent_is_unused { + return Ok(()); + } + + let fallbacks = decls.minify(context)?; + if decls.declarations.is_empty() && fallbacks.is_empty() { + return Ok(()); + } + + // Same bookkeeping as for the other non-style rules in `CssRuleList::minify`. + flush_pending_style_merge(rules, merge_state, context); + merge_state.last_compat = None; + if !decls.declarations.is_empty() { + rules.push(CssRule::NestedDeclarations( + nesting::NestedDeclarationsRule { + declarations: core::mem::replace( + &mut decls.declarations, + dc::decl_block_empty_static(context.arena), + ), + loc: decls.loc, + }, + )); + } + rules.extend(fallbacks); + style_rules.clear(); + Ok(()) +} + // ─── StyleRuleKey ────────────────────────────────────────────────────────── /// A key to a `StyleRule` meant for use in a hash map for quickly detecting /// duplicates. It stores an index into the live `rules` Vec plus a @@ -1248,3 +1318,35 @@ pub struct MinifyContext<'a, 'bump> { /// will expand to, checked against [`MAX_SELECTOR_EXPANSION`]. pub(crate) selector_expansion_total: u32, } + +impl MinifyContext<'_, '_> { + /// Charge a rule with `selectors` selectors against the selector-expansion + /// budget. + /// + /// Compiling the enclosing nesting away for the targets repeats the rule's + /// selectors once per combination of the enclosing style rules' selectors. + /// That expansion is multiplicative across nesting levels, so bound it; + /// otherwise a few hundred bytes of deeply nested multi-selector rules + /// expand into gigabytes of cloned rules and output. See + /// [`MAX_SELECTOR_EXPANSION`]. `loc` is reported if the budget is exceeded. + pub(crate) fn charge_selector_expansion( + &mut self, + selectors: u32, + loc: Location, + ) -> Result<(), MinifyErr> { + if self.selector_expansion_multiplier > 1 { + self.selector_expansion_total = self.selector_expansion_total.saturating_add( + self.selector_expansion_multiplier + .saturating_mul(selectors.max(1)), + ); + if self.selector_expansion_total > MAX_SELECTOR_EXPANSION { + self.err = Some(crate::error::MinifyError { + kind: crate::error::MinifyErrorKind::selector_expansion_limit_exceeded, + loc, + }); + return Err(MinifyErr::minify_err); + } + } + Ok(()) + } +} diff --git a/src/css/rules/nesting.rs b/src/css/rules/nesting.rs index 4aab335f5abe..cfc3b64f3147 100644 --- a/src/css/rules/nesting.rs +++ b/src/css/rules/nesting.rs @@ -1,6 +1,11 @@ -use crate::css_rules::Location; -use crate::css_rules::style::StyleRule; -use crate::{PrintErr, Printer}; +use crate::context::DeclarationContext; +use crate::css_rules::style::{StyleRule, write_declarations}; +use crate::css_rules::{CssRule, CssRuleList, Location, MinifyContext}; +use crate::error::MinifyErr; +use crate::selectors::selector::serialize::serialize_selector_list; +use crate::selectors::{Component, Selector, SelectorList}; +use crate::{DeclarationBlock, PrintErr, Printer, VendorPrefix}; +use bun_alloc::ArenaPtr; /// A [@nest](https://www.w3.org/TR/css-nesting-1/#at-nest) rule. pub struct NestingRule { @@ -32,3 +37,95 @@ impl NestingRule { } } } + +/// A [nested declarations](https://drafts.csswg.org/css-nesting-1/#nested-declarations-rule) +/// rule: declarations that follow a nested rule in a style rule's body, or sit directly in a +/// conditional at-rule nested in one. They apply to the enclosing style rule's elements but +/// cascade from their own position, after the nested rules before them. +pub struct NestedDeclarationsRule { + /// The declarations. + // `DeclarationBlock<'bump>` borrows the parser arena; the lifetime is + // erased to `'static` here, matching `CssRule` in rules/mod.rs. + pub(crate) declarations: DeclarationBlock<'static>, + /// The location of the rule in the source file. + pub(crate) loc: Location, +} + +impl NestedDeclarationsRule { + pub(crate) fn to_css(&self, dest: &mut Printer) -> Result<(), PrintErr> { + // Read the `Copy` field directly: `Printer::context()` ties the borrow to `dest`. + let Some(ctx) = dest.ctx else { + // Nesting is preserved: the declarations are printed in place, between the + // enclosing rule's nested rules. + let more_rules_follow = dest.more_rules_follow; + return write_declarations(&self.declarations, None, dest, false, more_rules_follow); + }; + + // Nesting is being compiled away, so the enclosing rule's block is already closed: + // these declarations get a rule of their own under the enclosing rule's selectors. + // The list is repeated as is rather than wrapped in `:is()`, which would give every + // selector the specificity of the most specific one. + dest.nesting_expansions = 0; + serialize_selector_list(ctx.selectors.v.slice(), dest, ctx.parent, false)?; + dest.block(|d| write_declarations(&self.declarations, None, d, true, false)) + } + + /// Minifies the declarations and returns the already minified rules the property handlers + /// derived from them (logical property, `@supports` and dark color scheme fallbacks), for + /// the caller to insert right after this rule. Like the declarations, the fallbacks apply + /// to the enclosing style rule's elements, so they are built under `&` where + /// `minify_style_arm` builds a style rule's fallbacks under its own selectors. + pub(crate) fn minify( + &mut self, + context: &mut MinifyContext<'_, '_>, + ) -> Result>, MinifyErr> + where + R: for<'b> crate::generics::DeepClone<'b>, + { + // Compiling the enclosing nesting away prints the enclosing rule's selector list once + // more for this rule, and splitting that list for the targets clones this rule per part. + context.charge_selector_expansion(1, self.loc)?; + + context.handler_context.context = DeclarationContext::StyleRule; + self.declarations.minify( + super::dc::decl_handler_static(&mut *context.handler), + super::dc::decl_handler_static(&mut *context.important_handler), + &mut context.handler_context, + ); + context.handler_context.context = DeclarationContext::None; + + if !context.handler_context.has_fallback_rules() { + return Ok(Vec::new()); + } + + let enclosing = StyleRule:: { + selectors: SelectorList::from_selector(Selector::from_component_in( + Component::Nesting, + ArenaPtr::new(context.arena), + )), + vendor_prefix: VendorPrefix::default(), + declarations: super::dc::decl_block_empty_static(context.arena), + rules: CssRuleList::default(), + loc: self.loc, + }; + let supports = context.handler_context.get_supports_rules::(&enclosing); + let mut fallbacks = CssRuleList { + v: context + .handler_context + .get_additional_rules::(&enclosing), + }; + context.handler_context.reset(); + + // Downlevels the `:dir()` selectors of the logical property fallbacks for the targets. + fallbacks.minify(context, false)?; + fallbacks.v.extend(supports); + Ok(fallbacks.v) + } + + pub(crate) fn deep_clone(&self, bump: &bun_alloc::Arena) -> Self { + Self { + declarations: super::dc::decl_block_static(&self.declarations, bump), + loc: self.loc, + } + } +} diff --git a/src/css/rules/style.rs b/src/css/rules/style.rs index bfc3551a87da..db1a99f53db5 100644 --- a/src/css/rules/style.rs +++ b/src/css/rules/style.rs @@ -158,15 +158,11 @@ impl StyleRule { } fn to_css_base(&self, dest: &mut Printer, is_final_prefix_pass: bool) -> Result<(), PrintErr> { - use css::error::PrinterErrorKind; - use css::properties::Property; - // If supported, or there are no targets, preserve nesting. Otherwise, write nested rules after parent. let supports_nesting = self.rules.v.len() == 0 || !css::targets::Targets::should_compile_same(&dest.targets, css::Feature::Nesting); - let len = - self.declarations.declarations.len() + self.declarations.important_declarations.len(); + let len = self.declarations.len(); let has_declarations = supports_nesting || len > 0 || self.rules.v.len() == 0; if has_declarations { @@ -188,62 +184,13 @@ impl StyleRule { dest.whitespace()?; dest.write_char(b'{')?; dest.indent(); - - let mut i: usize = 0; - // A pair of (slice, important) tuples; declarations first, then - // important declarations. - let decls_groups: [(&[Property], bool); 2] = [ - (self.declarations.declarations.as_slice(), false), - (self.declarations.important_declarations.as_slice(), true), - ]; - for (decls, important) in decls_groups { - for decl in decls { - // The CSS modules `composes` property is handled specially, and omitted during printing. - // We need to add the classes it references to the list for the selectors in this rule. - if let Property::Composes(composes) = decl { - if dest.is_nested() && dest.css_module.is_some() { - return dest.new_error( - PrinterErrorKind::invalid_composes_nesting, - Some(composes.cssparser_loc), - ); - } - - if dest.css_module.is_some() { - // `handle_composes` needs `&mut dest` while the - // module also lives in `dest.css_module`. Move the - // module out for the duration of the call, then put - // it back before any `dest.new_error` early return. - let mut cm = dest.css_module.take(); - let err = if let Some(css_module) = &mut cm { - css_module - .handle_composes( - dest, - &self.selectors, - composes, - self.loc.source_index, - ) - .err() - } else { - None - }; - dest.css_module = cm; - if let Some(error_kind) = err { - return dest.new_error(error_kind, Some(composes.cssparser_loc)); - } - continue; - } - } - - dest.newline()?; - decl.to_css(dest, important)?; - if i != len - 1 || !dest.minify || (supports_nesting && self.rules.v.len() > 0) - { - dest.write_char(b';')?; - } - - i += 1; - } - } + write_declarations( + &self.declarations, + Some(&self.selectors), + dest, + true, + supports_nesting && self.rules.v.len() > 0, + )?; } fn helpers_newline( @@ -309,6 +256,71 @@ impl StyleRule { } } +/// Writes the declarations of a style rule's body (or of a nested declarations +/// rule, which prints like one), one per line. +/// +/// The CSS modules `composes` property is consumed here instead of being +/// printed, after being checked against the rule's `selectors`; a nested +/// declarations rule has no selectors of its own and passes `None`, relying on +/// the check the parser already made. With `own_block`, the caller has just +/// opened a block, so every declaration starts on a new line; otherwise the +/// caller has already positioned the first one. `more_follows` says more output +/// follows the last declaration within the same block, so it keeps its `;` even +/// when minifying. +pub(crate) fn write_declarations( + declarations: &DeclarationBlock<'_>, + selectors: Option<&selector::parser::SelectorList>, + dest: &mut Printer, + own_block: bool, + more_follows: bool, +) -> Result<(), PrintErr> { + use css::error::PrinterErrorKind; + use css::properties::Property; + + let len = declarations.len(); + let mut i: usize = 0; + // A pair of (slice, important) tuples; declarations first, then + // important declarations. + let decls_groups: [(&[Property], bool); 2] = [ + (declarations.declarations.as_slice(), false), + (declarations.important_declarations.as_slice(), true), + ]; + for (decls, important) in decls_groups { + for decl in decls { + // The CSS modules `composes` property is handled specially, and omitted during printing. + // We need to add the classes it references to the list for the selectors in this rule. + if let Property::Composes(composes) = decl { + if dest.is_nested() && dest.css_module.is_some() { + return dest.new_error( + PrinterErrorKind::invalid_composes_nesting, + Some(composes.cssparser_loc), + ); + } + + if let Some(css_module) = &dest.css_module { + let err = + selectors.and_then(|selectors| css_module.handle_composes(selectors).err()); + if let Some(error_kind) = err { + return dest.new_error(error_kind, Some(composes.cssparser_loc)); + } + continue; + } + } + + if own_block || i > 0 { + dest.newline()?; + } + decl.to_css(dest, important)?; + if i != len - 1 || !dest.minify || more_follows { + dest.write_char(b';')?; + } + + i += 1; + } + } + Ok(()) +} + impl StyleRule { pub(crate) fn minify( &mut self, @@ -377,33 +389,13 @@ impl StyleRule { Ok(false) } - /// Charge this rule's selectors against the selector-expansion budget. - /// - /// Compiling the enclosing nesting away for the targets repeats this rule's - /// selectors once per combination of the enclosing style rules' selectors. - /// That expansion is multiplicative across nesting levels, so bound it — - /// otherwise a few hundred bytes of deeply nested multi-selector rules - /// expand into gigabytes of cloned rules and output. See - /// [`css_rules::MAX_SELECTOR_EXPANSION`](super::MAX_SELECTOR_EXPANSION). + /// Charge this rule's selectors against the selector-expansion budget; see + /// [`MinifyContext::charge_selector_expansion`]. pub(crate) fn charge_selector_expansion( &self, context: &mut MinifyContext<'_, '_>, ) -> Result<(), MinifyErr> { - if context.selector_expansion_multiplier > 1 { - context.selector_expansion_total = context.selector_expansion_total.saturating_add( - context - .selector_expansion_multiplier - .saturating_mul(self.selectors.v.len().max(1)), - ); - if context.selector_expansion_total > super::MAX_SELECTOR_EXPANSION { - context.err = Some(crate::error::MinifyError { - kind: crate::error::MinifyErrorKind::selector_expansion_limit_exceeded, - loc: self.loc, - }); - return Err(MinifyErr::minify_err); - } - } - Ok(()) + context.charge_selector_expansion(self.selectors.v.len(), self.loc) } /// Minify this rule's nested rules, bumping the selector-expansion diff --git a/test/bundler/css/css-modules.test.ts b/test/bundler/css/css-modules.test.ts index ed43a6157a8f..ab2642742b52 100644 --- a/test/bundler/css/css-modules.test.ts +++ b/test/bundler/css/css-modules.test.ts @@ -212,4 +212,57 @@ describe("css", () => { expect(css).toContain(`.${betaOwn}`); }, }); + + // Declarations written after a nested rule stay behind it in the output + // (they used to be hoisted into the rule's leading declaration block), while + // `composes` in the leading block is still consumed instead of printed. + itBundled("css-module/DeclarationsAfterNestedRule", { + files: { + "/entry.js": ` + import styles from './styles.module.css'; + console.log(styles); + `, + "/styles.module.css": ` + .base { color: red; } + .button { + composes: base; + &:hover { color: blue; } + color: green; + } + `, + }, + entryPoints: ["/entry.js"], + outdir: "/out", + onAfterBundle(api) { + api.expectFile("/out/entry.js").toMatchInlineSnapshot(` + "// styles.module.css + var styles_module_default = { + base: "base_-MSaAA", + button: "base_-MSaAA button_-MSaAA" + }; + + // entry.js + console.log(styles_module_default); + " + `); + api.expectFile("/out/entry.css").toMatchInlineSnapshot(` + "/* styles.module.css */ + .base_-MSaAA { + color: red; + } + + .button_-MSaAA { + } + + .button_-MSaAA:hover { + color: #00f; + } + + .button_-MSaAA { + color: green; + } + " + `); + }, + }); }); diff --git a/test/js/bun/css/nested-declarations.test.ts b/test/js/bun/css/nested-declarations.test.ts new file mode 100644 index 000000000000..9bd6b05b5da3 --- /dev/null +++ b/test/js/bun/css/nested-declarations.test.ts @@ -0,0 +1,278 @@ +import { cssInternals } from "bun:internal-for-testing"; +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; +import path from "node:path"; + +// Declarations written after a nested rule (or directly inside a conditional +// at-rule nested in a style rule) form a nested declarations rule that keeps +// its position in the cascade: https://drafts.csswg.org/css-nesting/#nested-declarations-rule +// +// They used to be merged into the enclosing rule's own declaration block, which +// hoisted them above the nested rules they were written after, so e.g. +// `a { &{color:blue} color:green }` rendered blue instead of green. + +const { minifyTest, prefixTest, _test } = cssInternals; + +// CSS nesting is not supported, so it gets compiled away; everything else +// used below is supported, so the output stays small. +const chrome87 = { chrome: 87 << 16 }; +// Additionally lacks logical inset properties and light-dark(), but supports +// `:dir()`, so those properties compile to compact fallback rules. +const firefox60 = { firefox: 60 << 16 }; + +const minifyPreserved = (source: string) => minifyTest(source, ""); +const minifyLowered = (source: string, targets: object = chrome87) => minifyTest(source, "", targets); + +describe("declarations after a nested rule keep their position", () => { + test.each([ + [ + "a { color: red; & { color: blue } color: green }", + "a{color:red;&{color:#00f}color:green}", + "a{color:red}a{color:#00f}a{color:green}", + ], + [".a { .b { color: blue } color: green }", ".a{& .b{color:#00f}color:green}", ".a .b{color:#00f}.a{color:green}"], + [ + ".a { &:hover { color: blue } color: green; &:focus { color: red } }", + ".a{&:hover{color:#00f}color:green;&:focus{color:red}}", + ".a:hover{color:#00f}.a{color:green}.a:focus{color:red}", + ], + // Each run of declarations becomes its own rule; consecutive declarations share one. + [ + ".a { color: red; .b { color: blue } color: green; .c { color: yellow } color: purple; background: white }", + ".a{color:red;& .b{color:#00f}color:green;& .c{color:#ff0}color:purple;background:#fff}", + ".a{color:red}.a .b{color:#00f}.a{color:green}.a .c{color:#ff0}.a{color:purple;background:#fff}", + ], + // The run is minified like a declaration block: shorthands, overrides and `!important` ordering. + [ + ".a { .b { color: blue } margin-left: 1px; margin-right: 2px; margin-top: 3px; margin-bottom: 4px }", + ".a{& .b{color:#00f}margin:3px 2px 4px 1px}", + ".a .b{color:#00f}.a{margin:3px 2px 4px 1px}", + ], + [ + ".a { .b { color: blue } color: green !important; background: red }", + ".a{& .b{color:#00f}background:red;color:green!important}", + ".a .b{color:#00f}.a{background:red;color:green!important}", + ], + // A selector list is repeated as written, not wrapped in `:is()`, so each + // selector keeps its own specificity. + [ + ".a, .d { .b { color: blue } color: green }", + ".a,.d{& .b{color:#00f}color:green}", + ":is(.a,.d) .b{color:#00f}.a,.d{color:green}", + ], + // At every nesting level, resolved against the levels above. + [ + ".a { .b { .c { color: blue } color: green } color: red }", + ".a{& .b{& .c{color:#00f}color:green}color:red}", + ".a .b .c{color:#00f}.a .b{color:green}.a{color:red}", + ], + [ + ".a { @nest .p & { .b { color: blue } color: red } color: green }", + ".a{@nest .p &{& .b{color:#00f}color:red}color:green}", + ".p .a .b{color:#00f}.p .a{color:red}.a{color:green}", + ], + // `div:hover {` is first tried as a declaration named `div`; that attempt + // must not leave an empty rule behind. + [ + ".a { .b { color: blue } div:hover { color: red } color: green }", + ".a{& .b{color:#00f}& div:hover{color:red}color:green}", + ".a .b{color:#00f}.a div:hover{color:red}.a{color:green}", + ], + // A following rule with the same selector is not merged across the nested rules. + [ + ".a { .b { color: blue } color: green } .a { color: red }", + ".a{& .b{color:#00f}color:green}.a{color:red}", + ".a .b{color:#00f}.a{color:green}.a{color:red}", + ], + // The nested rule is removed as empty; the declarations still apply. + [".a { .b {} color: green }", ".a{color:green}", ".a{color:green}"], + ])("%s", (source, preserved, lowered) => { + expect(minifyPreserved(source)).toBe(preserved); + expect(minifyLowered(source)).toBe(lowered); + }); +}); + +describe("declarations directly inside a nested conditional rule", () => { + test.each([ + [ + ".c { @media (min-width: 600px) { color: pink } color: black }", + ".c{@media (width>=600px){color:pink}color:#000}", + "@media (min-width:600px){.c{color:pink}}.c{color:#000}", + ], + [ + ".a { @supports (display: grid) { color: pink } color: black }", + ".a{@supports (display: grid){color:pink}color:#000}", + "@supports (display: grid){.a{color:pink}}.a{color:#000}", + ], + [ + ".a { @container (min-width: 100px) { color: pink } color: black }", + ".a{@container (width>=100px){color:pink}color:#000}", + "@container (width>=100px){.a{color:pink}}.a{color:#000}", + ], + // Declarations before and after a rule nested in the conditional. + [ + ".a { @media (min-width: 100px) { color: red; .b { color: blue } color: green } }", + ".a{@media (width>=100px){color:red;& .b{color:#00f}color:green}}", + "@media (min-width:100px){.a{color:red}.a .b{color:#00f}.a{color:green}}", + ], + [ + ".a, .d { @media (min-width: 100px) { color: pink } }", + ".a,.d{@media (width>=100px){color:pink}}", + "@media (min-width:100px){.a,.d{color:pink}}", + ], + ])("%s", (source, preserved, lowered) => { + expect(minifyPreserved(source)).toBe(preserved); + expect(minifyLowered(source)).toBe(lowered); + }); +}); + +describe("trailing semicolon when nesting is preserved", () => { + // `@media all` is dropped when minifying and its contents printed in place, + // so the declarations at its end are not the last thing in the block. + test.each([ + [".a { @media all { .b { color: green } color: red } color: blue }", ".a{& .b{color:green}color:red;color:#00f}"], + [ + ".a { @media all { .b { color: green } color: red } .c { color: blue } }", + ".a{& .b{color:green}color:red;& .c{color:#00f}}", + ], + [".a { @media all { .b { color: green } color: red } }", ".a{& .b{color:green}color:red}"], + ])("%s", (source, expected) => { + expect(minifyPreserved(source)).toBe(expected); + }); +}); + +describe("fallback rules generated for the declarations follow them", () => { + test.each([ + [ + ".a { .b { color: blue } inset-inline-end: 20px; .c { color: red } }", + ".a .b{color:#00f}.a:dir(ltr){right:20px}.a:dir(rtl){left:20px}.a .c{color:red}", + ], + [ + ".a, .d { .b { color: blue } inset-inline-end: 20px }", + ":is(.a,.d) .b{color:#00f}:is(.a,.d):dir(ltr){right:20px}:is(.a,.d):dir(rtl){left:20px}", + ], + [ + ".a { .b { color: blue } color-scheme: light dark; .c { color: red } }", + ".a .b{color:#00f}" + + ".a{--buncss-light:initial;--buncss-dark: ;color-scheme:light dark}" + + "@media (prefers-color-scheme:dark){.a{--buncss-light: ;--buncss-dark:initial}}" + + ".a .c{color:red}", + ], + [ + ".a { @media (min-width: 100px) { inset-inline-end: 20px } }", + "@media (min-width:100px){.a:dir(ltr){right:20px}.a:dir(rtl){left:20px}}", + ], + // The enclosing rule's own fallbacks go between its declarations and its + // nested rules, which keep their order. + [ + ".a { color: red; inset-inline-end: 1px; .b { color: blue } color: green }", + ".a{color:red}.a:dir(ltr){right:1px}.a:dir(rtl){left:1px}.a .b{color:#00f}.a{color:green}", + ], + ])("%s", (source, expected) => { + expect(minifyLowered(source, firefox60)).toBe(expected); + }); +}); + +test("printed once per vendor prefix pass of the enclosing rule", () => { + // Safari 8 needs `:-webkit-full-screen`, so the rule is printed once per prefix. + expect(minifyLowered(":fullscreen { div { color: red } color: green }", { safari: 8 << 16 })).toBe( + ":-webkit-full-screen div{color:red}:-webkit-full-screen{color:green}:fullscreen div{color:red}:fullscreen{color:green}", + ); +}); + +const prettySource = + "a { color: red; & { color: blue } color: green }\n" + + ".c { @media (min-width: 600px) { color: pink; .d { color: white } color: gray; background: black } color: black }"; + +test("pretty printing with nesting preserved", () => { + expect(_test(prettySource, "")).toBe( + [ + "a {", + " color: red;", + "", + " & {", + " color: #00f;", + " }", + "", + " color: green;", + "}", + "", + ".c {", + " @media (width >= 600px) {", + " color: pink;", + "", + " & .d {", + " color: #fff;", + " }", + "", + " color: gray;", + " background: #000;", + " }", + "", + " color: #000;", + "}", + "", + ].join("\n"), + ); +}); + +test("pretty printing with nesting compiled away", () => { + expect(prefixTest(prettySource, "", chrome87)).toBe( + [ + "a {", + " color: red;", + "}", + "", + "a {", + " color: #00f;", + "}", + "", + "a {", + " color: green;", + "}", + "", + "@media (min-width: 600px) {", + " .c {", + " color: pink;", + " }", + "", + " .c .d {", + " color: #fff;", + " }", + "", + " .c {", + " color: gray;", + " background: #000;", + " }", + "}", + "", + ".c {", + " color: #000;", + "}", + "", + ].join("\n"), + ); +}); + +describe("bun build", () => { + const source = + "a { color: red; & { color: blue } color: green }\n.c { @media (min-width: 600px) { color: pink } color: black }\n"; + + test.concurrent.each([ + ["browser", "a{color:red}a{color:#00f}a{color:green}@media (min-width:600px){.c{color:pink}}.c{color:#000}"], + ["bun", "a{color:red;&{color:#00f}color:green}.c{@media (width>=600px){color:pink}color:#000}"], + ])("--target=%s --minify", async (target, expected) => { + using dir = tempDir("css-nested-declarations", { "in.css": source }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "build", "in.css", `--target=${target}`, "--minify", "--outdir", "out"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect({ exitCode, stderr: stderr.includes("error") ? stderr : "" }).toEqual({ exitCode: 0, stderr: "" }); + const output = await Bun.file(path.join(String(dir), "out", "in.css")).text(); + expect(output.replace(/^\/\*.*\*\/\n/, "")).toBe(expected + "\n"); + }); +}); From 2c66fffc94951df8420e8cf4426cbf6c807a6f99 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 14:55:58 +0000 Subject: [PATCH 2/3] test: assert the bundled stylesheet on stdout and drain every pipe --- test/js/bun/css/nested-declarations.test.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/test/js/bun/css/nested-declarations.test.ts b/test/js/bun/css/nested-declarations.test.ts index 9bd6b05b5da3..751dd38a55c2 100644 --- a/test/js/bun/css/nested-declarations.test.ts +++ b/test/js/bun/css/nested-declarations.test.ts @@ -1,7 +1,6 @@ import { cssInternals } from "bun:internal-for-testing"; import { describe, expect, test } from "bun:test"; import { bunEnv, bunExe, tempDir } from "harness"; -import path from "node:path"; // Declarations written after a nested rule (or directly inside a conditional // at-rule nested in a style rule) form a nested declarations rule that keeps @@ -263,16 +262,15 @@ describe("bun build", () => { ["bun", "a{color:red;&{color:#00f}color:green}.c{@media (width>=600px){color:pink}color:#000}"], ])("--target=%s --minify", async (target, expected) => { using dir = tempDir("css-nested-declarations", { "in.css": source }); + // Without --outdir the bundled stylesheet is written to stdout. await using proc = Bun.spawn({ - cmd: [bunExe(), "build", "in.css", `--target=${target}`, "--minify", "--outdir", "out"], + cmd: [bunExe(), "build", "in.css", `--target=${target}`, "--minify"], env: bunEnv, cwd: String(dir), stdout: "pipe", stderr: "pipe", }); - const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); - expect({ exitCode, stderr: stderr.includes("error") ? stderr : "" }).toEqual({ exitCode: 0, stderr: "" }); - const output = await Bun.file(path.join(String(dir), "out", "in.css")).text(); - expect(output.replace(/^\/\*.*\*\/\n/, "")).toBe(expected + "\n"); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: expected + "\n", stderr: "", exitCode: 0 }); }); }); From e120c583195817f3f201dae779890d51e434c3b4 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 15:18:08 +0000 Subject: [PATCH 3/3] css: trim comments to one line, name write_declarations' layout flags --- src/css/context.rs | 3 +-- src/css/css_parser.rs | 20 +++++--------------- src/css/printer.rs | 5 +---- src/css/rules/mod.rs | 20 +++----------------- src/css/rules/nesting.rs | 33 ++++++++++----------------------- src/css/rules/style.rs | 31 +++++++++---------------------- 6 files changed, 29 insertions(+), 83 deletions(-) diff --git a/src/css/context.rs b/src/css/context.rs index 904f0da479b0..091472567936 100644 --- a/src/css/context.rs +++ b/src/css/context.rs @@ -80,8 +80,7 @@ impl<'a> PropertyHandlerContext<'a> { self.rtl.push(rtl); } - /// Whether the handlers staged anything that `get_supports_rules` / - /// `get_additional_rules` would turn into rules. + /// Whether `get_supports_rules` or `get_additional_rules` would return anything. pub(crate) fn has_fallback_rules(&self) -> bool { !(self.supports.is_empty() && self.ltr.is_empty() diff --git a/src/css/css_parser.rs b/src/css/css_parser.rs index 7d2d3081f286..9f89961f682a 100644 --- a/src/css/css_parser.rs +++ b/src/css/css_parser.rs @@ -1530,10 +1530,7 @@ mod rule_parsers { ) -> CssResult> { let loc = self.current_loc(input); - // Declarations can be immediately within @media and @supports blocks - // that are nested within a parent style rule. They apply to the parent - // rule's elements, from that position: - // https://drafts.csswg.org/css-nesting/#nested-declarations-rule + // Declarations directly inside a @media, @supports, ... block nested in a style rule. let (declarations, mut rules) = self.parse_nested(input, false)?; if declarations.len() > 0 { @@ -2075,8 +2072,7 @@ mod rule_parsers { let mut usage = PropertyBitset::init_empty(); let mut custom_properties: Vec<&'static [u8]> = Vec::new(); fill_property_bit_set(&mut usage, &declarations, &mut custom_properties); - // Declarations written after a nested rule still belong to this rule's - // elements. + // Declarations written after a nested rule are still this rule's own. for rule in rules.v.iter() { if let CssRule::NestedDeclarations(nested) = rule { fill_property_bit_set( @@ -2133,15 +2129,10 @@ mod rule_parsers { fn parse_value(this: &mut Self, name: &[u8], input: &mut Parser) -> CssResult<()> { // SAFETY: `input.arena()` re-borrows the parser arena through `&self`; // detach that borrow so `input` can be re-borrowed mutably below. The - // arena outlives the parser (it owns all parsed allocations); `'static` - // is the crate-wide erasure of its lifetime (see `parse_nested`). + // arena outlives the parser (it owns all parsed allocations). let arena: &'static Bump = unsafe { bun_ptr::detach_lifetime_ref(input.arena()) }; - // A declaration after a nested rule has to stay behind that rule in the - // cascade, so rather than joining the block's own declarations it goes - // into a nested declarations rule at the end of the block's rules, - // continuing the one the previous declaration started if there is one: - // https://drafts.csswg.org/css-nesting/#nested-declarations-rule + // Past a nested rule, declarations go into a `NestedDeclarationsRule` ending the list. let mut nested = match this.rules.v.pop() { None => None, Some(CssRule::NestedDeclarations(nested)) => Some(nested), @@ -2177,8 +2168,7 @@ mod rule_parsers { &mut ctx, ); - // `RuleBodyParser` retries a declaration that failed to parse as a nested - // style rule; a rule that received nothing must not end up in front of it. + // A failed declaration is retried as a nested rule; leave no empty rule ahead of it. if let Some(nested) = nested && !nested.declarations.is_empty() { diff --git a/src/css/printer.rs b/src/css/printer.rs index b6f28fc9e434..385e860ab92d 100644 --- a/src/css/printer.rs +++ b/src/css/printer.rs @@ -129,10 +129,7 @@ pub struct Printer<'a> { /// they are skipped while this is set and emitted once in the final pass, /// keeping the output linear in nesting depth instead of exponential. pub(crate) skip_prefixed_nested_rules: bool, - /// Set by `CssRuleList::to_css` right before printing each rule: whether - /// more output follows that rule inside the block it is printed in. A - /// nested declarations rule printed in place (nesting preserved) may only - /// drop its final `;` when nothing does. + /// Set by `CssRuleList::to_css` per rule: whether more output follows it within its block. pub(crate) more_rules_follow: bool, pub(crate) in_calc: bool, pub(crate) css_module: Option>, diff --git a/src/css/rules/mod.rs b/src/css/rules/mod.rs index bccebed9abdf..17e7d58f576d 100644 --- a/src/css/rules/mod.rs +++ b/src/css/rules/mod.rs @@ -426,10 +426,7 @@ impl CssRuleList { self.to_css_impl(dest, false) } - /// Prints the rules straight into the block the list's owner sits in, for an owner whose - /// own block is elided (`@media all` when minifying). Whatever follows the owner in that - /// block, which `to_css_impl` recorded in `dest.more_rules_follow` just before printing - /// the owner, therefore also follows this list's last rule. + /// For an owner printed without a block of its own: whatever follows it also follows this list. pub(crate) fn to_css_unwrapped(&self, dest: &mut Printer) -> Result<(), PrintErr> { let more_rules_follow = dest.more_rules_follow; self.to_css_impl(dest, more_rules_follow) @@ -442,8 +439,7 @@ impl CssRuleList { ) -> Result<(), PrintErr> { let mut first = true; let mut last_without_block = false; - // The other rules the loop skips still count as following their predecessors; that - // only costs a nested declarations rule ahead of them a `;` it could have dropped. + // Counting rules skipped below as followers only costs a nested declarations rule a `;`. let last = self .v .iter() @@ -938,8 +934,6 @@ fn minify_nested_declarations_arm css::generics::DeepClone<'b>>( context: &mut MinifyContext<'_, '_>, parent_is_unused: bool, ) -> Result<(), MinifyErr> { - // These declarations are as dead as the enclosing rule's own ones, which - // `StyleRule::minify` dropped. if parent_is_unused { return Ok(()); } @@ -1320,15 +1314,7 @@ pub struct MinifyContext<'a, 'bump> { } impl MinifyContext<'_, '_> { - /// Charge a rule with `selectors` selectors against the selector-expansion - /// budget. - /// - /// Compiling the enclosing nesting away for the targets repeats the rule's - /// selectors once per combination of the enclosing style rules' selectors. - /// That expansion is multiplicative across nesting levels, so bound it; - /// otherwise a few hundred bytes of deeply nested multi-selector rules - /// expand into gigabytes of cloned rules and output. See - /// [`MAX_SELECTOR_EXPANSION`]. `loc` is reported if the budget is exceeded. + /// Charges `selectors` selectors against [`MAX_SELECTOR_EXPANSION`], failing at `loc`. pub(crate) fn charge_selector_expansion( &mut self, selectors: u32, diff --git a/src/css/rules/nesting.rs b/src/css/rules/nesting.rs index cfc3b64f3147..1bf02205a330 100644 --- a/src/css/rules/nesting.rs +++ b/src/css/rules/nesting.rs @@ -38,14 +38,8 @@ impl NestingRule { } } -/// A [nested declarations](https://drafts.csswg.org/css-nesting-1/#nested-declarations-rule) -/// rule: declarations that follow a nested rule in a style rule's body, or sit directly in a -/// conditional at-rule nested in one. They apply to the enclosing style rule's elements but -/// cascade from their own position, after the nested rules before them. +/// Declarations after a nested rule (drafts.csswg.org/css-nesting-1/#nested-declarations-rule). pub struct NestedDeclarationsRule { - /// The declarations. - // `DeclarationBlock<'bump>` borrows the parser arena; the lifetime is - // erased to `'static` here, matching `CssRule` in rules/mod.rs. pub(crate) declarations: DeclarationBlock<'static>, /// The location of the rule in the source file. pub(crate) loc: Location, @@ -55,26 +49,19 @@ impl NestedDeclarationsRule { pub(crate) fn to_css(&self, dest: &mut Printer) -> Result<(), PrintErr> { // Read the `Copy` field directly: `Printer::context()` ties the borrow to `dest`. let Some(ctx) = dest.ctx else { - // Nesting is preserved: the declarations are printed in place, between the - // enclosing rule's nested rules. - let more_rules_follow = dest.more_rules_follow; - return write_declarations(&self.declarations, None, dest, false, more_rules_follow); + // Nesting is preserved: printed in place among the enclosing rule's nested rules. + let trailing_semicolon = !dest.minify || dest.more_rules_follow; + return write_declarations(&self.declarations, None, dest, false, trailing_semicolon); }; - // Nesting is being compiled away, so the enclosing rule's block is already closed: - // these declarations get a rule of their own under the enclosing rule's selectors. - // The list is repeated as is rather than wrapped in `:is()`, which would give every - // selector the specificity of the most specific one. + // Compiled nesting: enclosing selectors as written, since `:is()` would change specificity. dest.nesting_expansions = 0; serialize_selector_list(ctx.selectors.v.slice(), dest, ctx.parent, false)?; - dest.block(|d| write_declarations(&self.declarations, None, d, true, false)) + let trailing_semicolon = !dest.minify; + dest.block(|d| write_declarations(&self.declarations, None, d, true, trailing_semicolon)) } - /// Minifies the declarations and returns the already minified rules the property handlers - /// derived from them (logical property, `@supports` and dark color scheme fallbacks), for - /// the caller to insert right after this rule. Like the declarations, the fallbacks apply - /// to the enclosing style rule's elements, so they are built under `&` where - /// `minify_style_arm` builds a style rule's fallbacks under its own selectors. + /// Minifies the declarations; returns their fallback rules, which belong right after this rule. pub(crate) fn minify( &mut self, context: &mut MinifyContext<'_, '_>, @@ -82,8 +69,7 @@ impl NestedDeclarationsRule { where R: for<'b> crate::generics::DeepClone<'b>, { - // Compiling the enclosing nesting away prints the enclosing rule's selector list once - // more for this rule, and splitting that list for the targets clones this rule per part. + // Compiling nesting away prints the enclosing selector list once more for this rule. context.charge_selector_expansion(1, self.loc)?; context.handler_context.context = DeclarationContext::StyleRule; @@ -98,6 +84,7 @@ impl NestedDeclarationsRule { return Ok(Vec::new()); } + // The fallbacks apply to the enclosing rule's elements, so they are built under `&`. let enclosing = StyleRule:: { selectors: SelectorList::from_selector(Selector::from_component_in( Component::Nesting, diff --git a/src/css/rules/style.rs b/src/css/rules/style.rs index db1a99f53db5..fdb34bf23e0e 100644 --- a/src/css/rules/style.rs +++ b/src/css/rules/style.rs @@ -184,12 +184,13 @@ impl StyleRule { dest.whitespace()?; dest.write_char(b'{')?; dest.indent(); + let trailing_semicolon = !dest.minify || (supports_nesting && self.rules.v.len() > 0); write_declarations( &self.declarations, Some(&self.selectors), dest, true, - supports_nesting && self.rules.v.len() > 0, + trailing_semicolon, )?; } @@ -256,39 +257,26 @@ impl StyleRule { } } -/// Writes the declarations of a style rule's body (or of a nested declarations -/// rule, which prints like one), one per line. -/// -/// The CSS modules `composes` property is consumed here instead of being -/// printed, after being checked against the rule's `selectors`; a nested -/// declarations rule has no selectors of its own and passes `None`, relying on -/// the check the parser already made. With `own_block`, the caller has just -/// opened a block, so every declaration starts on a new line; otherwise the -/// caller has already positioned the first one. `more_follows` says more output -/// follows the last declaration within the same block, so it keeps its `;` even -/// when minifying. +/// Writes a rule body's declarations, one per line; `composes` is validated against `selectors`. pub(crate) fn write_declarations( declarations: &DeclarationBlock<'_>, selectors: Option<&selector::parser::SelectorList>, dest: &mut Printer, - own_block: bool, - more_follows: bool, + newline_before_first: bool, + trailing_semicolon: bool, ) -> Result<(), PrintErr> { use css::error::PrinterErrorKind; use css::properties::Property; let len = declarations.len(); let mut i: usize = 0; - // A pair of (slice, important) tuples; declarations first, then - // important declarations. let decls_groups: [(&[Property], bool); 2] = [ (declarations.declarations.as_slice(), false), (declarations.important_declarations.as_slice(), true), ]; for (decls, important) in decls_groups { for decl in decls { - // The CSS modules `composes` property is handled specially, and omitted during printing. - // We need to add the classes it references to the list for the selectors in this rule. + // CSS modules consume `composes` instead of printing it. if let Property::Composes(composes) = decl { if dest.is_nested() && dest.css_module.is_some() { return dest.new_error( @@ -307,11 +295,11 @@ pub(crate) fn write_declarations( } } - if own_block || i > 0 { + if newline_before_first || i > 0 { dest.newline()?; } decl.to_css(dest, important)?; - if i != len - 1 || !dest.minify || more_follows { + if i != len - 1 || trailing_semicolon { dest.write_char(b';')?; } @@ -389,8 +377,7 @@ impl StyleRule { Ok(false) } - /// Charge this rule's selectors against the selector-expansion budget; see - /// [`MinifyContext::charge_selector_expansion`]. + /// See [`MinifyContext::charge_selector_expansion`]. pub(crate) fn charge_selector_expansion( &self, context: &mut MinifyContext<'_, '_>,