Fix a bug with @media query bubbling (#1792)

Closes #1791

CHANGELOG.md

@@ -1,3 +1,9 @@
+## 1.54.6
+
+* Fix a bug where a `@media` query could be incorrectly omitted from a
+  stylesheet if it had multiple levels of nested `@media` queries within it
+  *and* the inner queries were mergeable but the outer query was not.
+
 ## 1.54.5
 
 * Properly consider `a ~ c` to be a superselector of `a ~ b ~ c` and `a + b +

lib/src/ast/css/media_query.dart

@@ -221,4 +221,6 @@ class MediaQuerySuccessfulMergeResult implements MediaQueryMergeResult {
   final CssMediaQuery query;
 
   MediaQuerySuccessfulMergeResult._(this.query);
+
+  String toString() => query.toString();
 }

lib/src/visitor/async_evaluate.dart

@@ -179,6 +179,13 @@ class _EvaluateVisitor
   /// The current media queries, if any.
   List<CssMediaQuery>? _mediaQueries;
 
+  /// The set of media queries that were merged together to create
+  /// [_mediaQueries].
+  ///
+  /// This will be non-null if and only if [_mediaQueries] is non-null, but it
+  /// will be empty if [_mediaQueries] isn't the result of a merge.
+  Set<CssMediaQuery>? _mediaQuerySources;
+
   /// The current parent node in the output CSS tree.
   ModifiableCssParentNode get _parent => _assertInModule(__parent, "__parent");
   set _parent(ModifiableCssParentNode value) => __parent = value;
@@ -1059,7 +1066,8 @@ class _EvaluateVisitor
 
     if (_mediaQueries != null && query.excludesName('media')) {
       var innerScope = scope;
-      scope = (callback) => _withMediaQueries(null, () => innerScope(callback));
+      scope = (callback) =>
+          _withMediaQueries(null, null, () => innerScope(callback));
     }
 
     if (_inKeyframes && query.excludesName('keyframes')) {
@@ -1778,9 +1786,14 @@ class _EvaluateVisitor
         .andThen((mediaQueries) => _mergeMediaQueries(mediaQueries, queries));
     if (mergedQueries != null && mergedQueries.isEmpty) return null;
 
+    var mergedSources = mergedQueries == null
+        ? const <CssMediaQuery>{}
+        : {..._mediaQuerySources!, ..._mediaQueries!, ...queries};
+
     await _withParent(
         ModifiableCssMediaRule(mergedQueries ?? queries, node.span), () async {
-      await _withMediaQueries(mergedQueries ?? queries, () async {
+      await _withMediaQueries(mergedQueries ?? queries, mergedSources,
+          () async {
         var styleRule = _styleRule;
         if (styleRule == null) {
           for (var child in node.children) {
@@ -1802,7 +1815,9 @@ class _EvaluateVisitor
     },
         through: (node) =>
             node is CssStyleRule ||
-            (mergedQueries != null && node is CssMediaRule),
+            (mergedSources.isNotEmpty &&
+                node is CssMediaRule &&
+                node.queries.every(mergedSources.contains)),
         scopeWhen: node.hasDeclarations);
 
     return null;
@@ -3018,10 +3033,15 @@ class _EvaluateVisitor
         (mediaQueries) => _mergeMediaQueries(mediaQueries, node.queries));
     if (mergedQueries != null && mergedQueries.isEmpty) return;
 
+    var mergedSources = mergedQueries == null
+        ? const <CssMediaQuery>{}
+        : {..._mediaQuerySources!, ..._mediaQueries!, ...node.queries};
+
     await _withParent(
         ModifiableCssMediaRule(mergedQueries ?? node.queries, node.span),
         () async {
-      await _withMediaQueries(mergedQueries ?? node.queries, () async {
+      await _withMediaQueries(mergedQueries ?? node.queries, mergedSources,
+          () async {
         var styleRule = _styleRule;
         if (styleRule == null) {
           for (var child in node.children) {
@@ -3043,7 +3063,9 @@ class _EvaluateVisitor
     },
         through: (node) =>
             node is CssStyleRule ||
-            (mergedQueries != null && node is CssMediaRule),
+            (mergedSources.isNotEmpty &&
+                node is CssMediaRule &&
+                node.queries.every(mergedSources.contains)),
         scopeWhen: false);
   }
 
@@ -3301,12 +3323,19 @@ class _EvaluateVisitor
   }
 
   /// Runs [callback] with [queries] as the current media queries.
-  Future<T> _withMediaQueries<T>(
-      List<CssMediaQuery>? queries, Future<T> callback()) async {
+  ///
+  /// This also sets [sources] as the current set of media queries that were
+  /// merged together to create [queries]. This is used to determine when it's
+  /// safe to bubble one query through another.
+  Future<T> _withMediaQueries<T>(List<CssMediaQuery>? queries,
+      Set<CssMediaQuery>? sources, Future<T> callback()) async {
     var oldMediaQueries = _mediaQueries;
+    var oldSources = _mediaQuerySources;
     _mediaQueries = queries;
+    _mediaQuerySources = sources;
     var result = await callback();
     _mediaQueries = oldMediaQueries;
+    _mediaQuerySources = oldSources;
     return result;
   }
 

lib/src/visitor/evaluate.dart

@@ -5,7 +5,7 @@
 // DO NOT EDIT. This file was generated from async_evaluate.dart.
 // See tool/grind/synchronize.dart for details.
 //
-// Checksum: ad5691d0c80b924308f6ee03863fa8c7f5e25ca0
+// Checksum: 1481489206d9df595860ec2e1c44729bd8928b90
 //
 // ignore_for_file: unused_import
 
@@ -187,6 +187,13 @@ class _EvaluateVisitor
   /// The current media queries, if any.
   List<CssMediaQuery>? _mediaQueries;
 
+  /// The set of media queries that were merged together to create
+  /// [_mediaQueries].
+  ///
+  /// This will be non-null if and only if [_mediaQueries] is non-null, but it
+  /// will be empty if [_mediaQueries] isn't the result of a merge.
+  Set<CssMediaQuery>? _mediaQuerySources;
+
   /// The current parent node in the output CSS tree.
   ModifiableCssParentNode get _parent => _assertInModule(__parent, "__parent");
   set _parent(ModifiableCssParentNode value) => __parent = value;
@@ -1063,7 +1070,8 @@ class _EvaluateVisitor
 
     if (_mediaQueries != null && query.excludesName('media')) {
       var innerScope = scope;
-      scope = (callback) => _withMediaQueries(null, () => innerScope(callback));
+      scope = (callback) =>
+          _withMediaQueries(null, null, () => innerScope(callback));
     }
 
     if (_inKeyframes && query.excludesName('keyframes')) {
@@ -1775,9 +1783,13 @@ class _EvaluateVisitor
         .andThen((mediaQueries) => _mergeMediaQueries(mediaQueries, queries));
     if (mergedQueries != null && mergedQueries.isEmpty) return null;
 
+    var mergedSources = mergedQueries == null
+        ? const <CssMediaQuery>{}
+        : {..._mediaQuerySources!, ..._mediaQueries!, ...queries};
+
     _withParent(ModifiableCssMediaRule(mergedQueries ?? queries, node.span),
         () {
-      _withMediaQueries(mergedQueries ?? queries, () {
+      _withMediaQueries(mergedQueries ?? queries, mergedSources, () {
         var styleRule = _styleRule;
         if (styleRule == null) {
           for (var child in node.children) {
@@ -1799,7 +1811,9 @@ class _EvaluateVisitor
     },
         through: (node) =>
             node is CssStyleRule ||
-            (mergedQueries != null && node is CssMediaRule),
+            (mergedSources.isNotEmpty &&
+                node is CssMediaRule &&
+                node.queries.every(mergedSources.contains)),
         scopeWhen: node.hasDeclarations);
 
     return null;
@@ -2997,9 +3011,13 @@ class _EvaluateVisitor
         (mediaQueries) => _mergeMediaQueries(mediaQueries, node.queries));
     if (mergedQueries != null && mergedQueries.isEmpty) return;
 
+    var mergedSources = mergedQueries == null
+        ? const <CssMediaQuery>{}
+        : {..._mediaQuerySources!, ..._mediaQueries!, ...node.queries};
+
     _withParent(
         ModifiableCssMediaRule(mergedQueries ?? node.queries, node.span), () {
-      _withMediaQueries(mergedQueries ?? node.queries, () {
+      _withMediaQueries(mergedQueries ?? node.queries, mergedSources, () {
         var styleRule = _styleRule;
         if (styleRule == null) {
           for (var child in node.children) {
@@ -3021,7 +3039,9 @@ class _EvaluateVisitor
     },
         through: (node) =>
             node is CssStyleRule ||
-            (mergedQueries != null && node is CssMediaRule),
+            (mergedSources.isNotEmpty &&
+                node is CssMediaRule &&
+                node.queries.every(mergedSources.contains)),
         scopeWhen: false);
   }
 
@@ -3272,11 +3292,19 @@ class _EvaluateVisitor
   }
 
   /// Runs [callback] with [queries] as the current media queries.
-  T _withMediaQueries<T>(List<CssMediaQuery>? queries, T callback()) {
+  ///
+  /// This also sets [sources] as the current set of media queries that were
+  /// merged together to create [queries]. This is used to determine when it's
+  /// safe to bubble one query through another.
+  T _withMediaQueries<T>(
+      List<CssMediaQuery>? queries, Set<CssMediaQuery>? sources, T callback()) {
     var oldMediaQueries = _mediaQueries;
+    var oldSources = _mediaQuerySources;
     _mediaQueries = queries;
+    _mediaQuerySources = sources;
     var result = callback();
     _mediaQueries = oldMediaQueries;
+    _mediaQuerySources = oldSources;
     return result;
   }
 

pkg/sass_api/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 3.0.1
+
+* No user-visible chances.
+
 ## 3.0.0
 
 * **Breaking change:** Convert all visitor superclasses into mixins. This

pkg/sass_api/pubspec.yaml

@@ -2,15 +2,15 @@ name: sass_api
 # Note: Every time we add a new Sass AST node, we need to bump the *major*
 # version because it's a breaking change for anyone who's implementing the
 # visitor interface(s).
-version: 3.0.0
+version: 3.0.1
 description: Additional APIs for Dart Sass.
 homepage: https://github.com/sass/dart-sass
 
 environment:
   sdk: ">=2.17.0 <3.0.0"
 
 dependencies:
-  sass: 1.54.5
+  sass: 1.54.6
 
 dev_dependencies:
   dartdoc: ^5.0.0

pubspec.yaml

@@ -1,5 +1,5 @@
 name: sass
-version: 1.54.5
+version: 1.54.6
 description: A Sass implementation in Dart.
 homepage: https://github.com/sass/dart-sass