Skip to content

Commit 45da11d

Browse files
nex3nex3
authored
Preserve nested media queries when they can't be merged (#410)
See sass/sass#1831
1 parent f740e97 commit 45da11d

File tree

5 files changed

+141
-39
lines changed

5 files changed

+141
-39
lines changed

‎CHANGELOG.md‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
## 1.9.3
1+
## 1.10.0
2+
3+
* When two `@media` rules' queries can't be merged, leave nested rules in place
4+
for browsers that support them.
25

36
* Fix a typo in an error message.
47

‎lib/src/ast/css/media_query.dart‎

Lines changed: 86 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -45,55 +45,94 @@ class CssMediaQuery {
4545

4646
/// Merges this with [other] to return a query that matches the intersection
4747
/// of both inputs.
48-
CssMediaQuery merge(CssMediaQuery other) {
48+
MediaQueryMergeResult merge(CssMediaQuery other) {
4949
var ourModifier = this.modifier?.toLowerCase();
5050
var ourType = this.type?.toLowerCase();
5151
var theirModifier = other.modifier?.toLowerCase();
5252
var theirType = other.type?.toLowerCase();
5353

5454
if (ourType == null && theirType == null) {
55-
return new CssMediaQuery.condition(
56-
features.toList()..addAll(other.features));
55+
return new MediaQuerySuccessfulMergeResult._(new CssMediaQuery.condition(
56+
this.features.toList()..addAll(other.features)));
5757
}
5858

5959
String modifier;
6060
String type;
61+
List<String> features;
6162
if ((ourModifier == 'not') != (theirModifier == 'not')) {
62-
if (ourType == theirType) return null;
63+
if (ourType == theirType) {
64+
var negativeFeatures =
65+
ourModifier == 'not' ? this.features : other.features;
66+
var positiveFeatures =
67+
ourModifier == 'not' ? other.features : this.features;
68+
69+
// If the negative features are a subset of the positive features, the
70+
// query is empty. For example, `not screen and (color)` has no
71+
// intersection with `screen and (color) and (grid)`.
72+
//
73+
// However, `not screen and (color)` *does* intersect with `screen and
74+
// (grid)`, because it means `not (screen and (color))` and so it allows
75+
// a screen with no color but with a grid.
76+
if (negativeFeatures.every(positiveFeatures.contains)) {
77+
return MediaQueryMergeResult.empty;
78+
} else {
79+
return MediaQueryMergeResult.unrepresentable;
80+
}
81+
} else if (ourType == null || theirType == null) {
82+
return MediaQueryMergeResult.unrepresentable;
83+
}
6384

6485
if (ourModifier == 'not') {
65-
// The "not" would apply to the other query's features, which is not
66-
// what we want.
67-
if (other.features.isNotEmpty) return null;
6886
modifier = theirModifier;
6987
type = theirType;
88+
features = other.features;
7089
} else {
71-
if (this.features.isNotEmpty) return null;
7290
modifier = ourModifier;
7391
type = ourType;
92+
features = this.features;
7493
}
7594
} else if (ourModifier == 'not') {
7695
assert(theirModifier == 'not');
7796
// CSS has no way of representing "neither screen nor print".
78-
if (ourType == theirType) return null;
79-
modifier = ourModifier; // "not"
80-
type = ourType;
97+
if (ourType != theirType) return MediaQueryMergeResult.unrepresentable;
98+
99+
var moreFeatures = this.features.length > other.features.length
100+
? this.features
101+
: other.features;
102+
var fewerFeatures = this.features.length > other.features.length
103+
? other.features
104+
: this.features;
105+
106+
// If one set of features is a superset of the other, use those features
107+
// because they're strictly narrower.
108+
if (fewerFeatures.every(moreFeatures.contains)) {
109+
modifier = ourModifier; // "not"
110+
type = ourType;
111+
features = moreFeatures;
112+
} else {
113+
// Otherwise, there's no way to represent the intersection.
114+
return MediaQueryMergeResult.unrepresentable;
115+
}
81116
} else if (ourType == null) {
82117
modifier = theirModifier;
83118
type = theirType;
119+
features = this.features.toList()..addAll(other.features);
84120
} else if (theirType == null) {
85121
modifier = ourModifier;
86122
type = ourType;
123+
features = this.features.toList()..addAll(other.features);
87124
} else if (ourType != theirType) {
88-
return null;
125+
return MediaQueryMergeResult.empty;
89126
} else {
90127
modifier = ourModifier ?? theirModifier;
91128
type = ourType;
129+
features = this.features.toList()..addAll(other.features);
92130
}
93131

94-
return new CssMediaQuery(type == ourType ? this.type : other.type,
132+
return new MediaQuerySuccessfulMergeResult._(new CssMediaQuery(
133+
type == ourType ? this.type : other.type,
95134
modifier: modifier == ourModifier ? this.modifier : other.modifier,
96-
features: features.toList()..addAll(other.features));
135+
features: features));
97136
}
98137

99138
bool operator ==(other) =>
@@ -115,3 +154,36 @@ class CssMediaQuery {
115154
return buffer.toString();
116155
}
117156
}
157+
158+
/// The interface of possible return values of [CssMediaQuery.merge].
159+
///
160+
/// This is either the singleton values [empty] or [unrepresentable], or an
161+
/// instance of [MediaQuerySuccessfulMergeResult].
162+
abstract class MediaQueryMergeResult {
163+
/// A singleton value indicating that there are no contexts that match both
164+
/// input queries.
165+
static const empty = const _SingletonCssMediaQueryMergeResult("empty");
166+
167+
/// A singleton value indicating that the contexts that match both input
168+
/// queries can't be represented by a Level 3 media query.
169+
static const unrepresentable =
170+
const _SingletonCssMediaQueryMergeResult("unrepresentable");
171+
}
172+
173+
/// The subclass [MediaQueryMergeResult] that represents singleton enum values.
174+
class _SingletonCssMediaQueryMergeResult implements MediaQueryMergeResult {
175+
/// The name of the result type.
176+
final String _name;
177+
178+
const _SingletonCssMediaQueryMergeResult(this._name);
179+
180+
String toString() => _name;
181+
}
182+
183+
/// A successful result of [CssMediaQuery.merge].
184+
class MediaQuerySuccessfulMergeResult implements MediaQueryMergeResult {
185+
/// The merged media query.
186+
final CssMediaQuery query;
187+
188+
MediaQuerySuccessfulMergeResult._(this.query);
189+
}

‎lib/src/visitor/async_evaluate.dart‎

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -885,13 +885,14 @@ class _EvaluateVisitor
885885
}
886886

887887
var queries = await _visitMediaQueries(node.query);
888-
if (_mediaQueries != null) {
889-
queries = _mergeMediaQueries(_mediaQueries, queries);
890-
if (queries.isEmpty) return null;
891-
}
888+
var mergedQueries = _mediaQueries == null
889+
? null
890+
: _mergeMediaQueries(_mediaQueries, queries);
891+
if (mergedQueries != null && mergedQueries.isEmpty) return null;
892892

893-
await _withParent(new CssMediaRule(queries, node.span), () async {
894-
await _withMediaQueries(queries, () async {
893+
await _withParent(new CssMediaRule(mergedQueries ?? queries, node.span),
894+
() async {
895+
await _withMediaQueries(mergedQueries ?? queries, () async {
895896
if (!_inStyleRule) {
896897
for (var child in node.children) {
897898
await child.accept(this);
@@ -910,7 +911,9 @@ class _EvaluateVisitor
910911
}
911912
});
912913
},
913-
through: (node) => node is CssStyleRule || node is CssMediaRule,
914+
through: (node) =>
915+
node is CssStyleRule ||
916+
(mergedQueries != null && node is CssMediaRule),
914917
scopeWhen: node.hasDeclarations);
915918

916919
return null;
@@ -928,13 +931,24 @@ class _EvaluateVisitor
928931
() => CssMediaQuery.parseList(resolved, logger: _logger));
929932
}
930933

931-
/// Returns a list of queries that selects for platforms that match both
934+
/// Returns a list of queries that selects for contexts that match both
932935
/// [queries1] and [queries2].
936+
///
937+
/// Returns the empty list if there are no contexts that match both [queries1]
938+
/// and [queries2], or `null` if there are contexts that can't be represented
939+
/// by media queries.
933940
List<CssMediaQuery> _mergeMediaQueries(
934941
Iterable<CssMediaQuery> queries1, Iterable<CssMediaQuery> queries2) {
935-
return new List.unmodifiable(queries1.expand((query1) {
936-
return queries2.map((query2) => query1.merge(query2));
937-
}).where((query) => query != null));
942+
var queries = <CssMediaQuery>[];
943+
for (var query1 in queries1) {
944+
for (var query2 in queries2) {
945+
var result = query1.merge(query2);
946+
if (result == MediaQueryMergeResult.empty) continue;
947+
if (result == MediaQueryMergeResult.unrepresentable) return null;
948+
queries.add((result as MediaQuerySuccessfulMergeResult).query);
949+
}
950+
}
951+
return queries;
938952
}
939953

940954
Future<Value> visitReturnRule(ReturnRule node) =>

‎lib/src/visitor/evaluate.dart‎

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
// DO NOT EDIT. This file was generated from async_evaluate.dart.
66
// See tool/synchronize.dart for details.
77
//
8-
// Checksum: e9558d77111c693319f39af55da8ce3797fe1807
8+
// Checksum: b774b5c88da1ce98a48ce71a245357a11b047ec3
99

1010
import 'async_evaluate.dart' show EvaluateResult;
1111
export 'async_evaluate.dart' show EvaluateResult;
@@ -883,13 +883,13 @@ class _EvaluateVisitor
883883
}
884884

885885
var queries = _visitMediaQueries(node.query);
886-
if (_mediaQueries != null) {
887-
queries = _mergeMediaQueries(_mediaQueries, queries);
888-
if (queries.isEmpty) return null;
889-
}
886+
var mergedQueries = _mediaQueries == null
887+
? null
888+
: _mergeMediaQueries(_mediaQueries, queries);
889+
if (mergedQueries != null && mergedQueries.isEmpty) return null;
890890

891-
_withParent(new CssMediaRule(queries, node.span), () {
892-
_withMediaQueries(queries, () {
891+
_withParent(new CssMediaRule(mergedQueries ?? queries, node.span), () {
892+
_withMediaQueries(mergedQueries ?? queries, () {
893893
if (!_inStyleRule) {
894894
for (var child in node.children) {
895895
child.accept(this);
@@ -908,7 +908,9 @@ class _EvaluateVisitor
908908
}
909909
});
910910
},
911-
through: (node) => node is CssStyleRule || node is CssMediaRule,
911+
through: (node) =>
912+
node is CssStyleRule ||
913+
(mergedQueries != null && node is CssMediaRule),
912914
scopeWhen: node.hasDeclarations);
913915

914916
return null;
@@ -924,13 +926,24 @@ class _EvaluateVisitor
924926
() => CssMediaQuery.parseList(resolved, logger: _logger));
925927
}
926928

927-
/// Returns a list of queries that selects for platforms that match both
929+
/// Returns a list of queries that selects for contexts that match both
928930
/// [queries1] and [queries2].
931+
///
932+
/// Returns the empty list if there are no contexts that match both [queries1]
933+
/// and [queries2], or `null` if there are contexts that can't be represented
934+
/// by media queries.
929935
List<CssMediaQuery> _mergeMediaQueries(
930936
Iterable<CssMediaQuery> queries1, Iterable<CssMediaQuery> queries2) {
931-
return new List.unmodifiable(queries1.expand((query1) {
932-
return queries2.map((query2) => query1.merge(query2));
933-
}).where((query) => query != null));
937+
var queries = <CssMediaQuery>[];
938+
for (var query1 in queries1) {
939+
for (var query2 in queries2) {
940+
var result = query1.merge(query2);
941+
if (result == MediaQueryMergeResult.empty) continue;
942+
if (result == MediaQueryMergeResult.unrepresentable) return null;
943+
queries.add((result as MediaQuerySuccessfulMergeResult).query);
944+
}
945+
}
946+
return queries;
934947
}
935948

936949
Value visitReturnRule(ReturnRule node) => node.expression.accept(this);

‎pubspec.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
name: sass
2-
version: 1.9.3-dev
2+
version: 1.10.0
33
description: A Sass implementation in Dart.
44
author: Dart Team <[email protected]>
55
homepage: https://github.com/sass/dart-sass

0 commit comments

Comments
 (0)