From bc7216a441074f6a35b62dccb7fdc17e08ae442c Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Tue, 15 Sep 2020 13:03:38 -0700 Subject: [PATCH] Add a map.deep-merge() function (#1077) This also adds a Value.tryMap() function, which was useful for implementing this and may be more generally useful to users as well. See sass/sass#2836 See sass/sass-spec#1560 --- CHANGELOG.md | 23 ++++++++++++ lib/src/functions/map.dart | 50 +++++++++++++++++++++++++- lib/src/value.dart | 2 ++ lib/src/value/external/value.dart | 4 +++ lib/src/value/list.dart | 2 ++ lib/src/value/map.dart | 2 ++ pubspec.yaml | 2 +- test/dart_api/value/boolean_test.dart | 2 ++ test/dart_api/value/color_test.dart | 1 + test/dart_api/value/function_test.dart | 1 + test/dart_api/value/list_test.dart | 3 ++ test/dart_api/value/map_test.dart | 1 + test/dart_api/value/null_test.dart | 1 + test/dart_api/value/number_test.dart | 1 + test/dart_api/value/string_test.dart | 1 + 15 files changed, 94 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b8b82dd90..86b0168d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,26 @@ +## 1.27.0 + +* Add a `map.deep-merge()` function. This works like `map.merge()`, except that + nested map values are *also* recursively merged. For example: + + ``` + map.deep-merge( + (color: (primary: red, secondary: blue), + (color: (secondary: teal) + ) // => (color: (primary: red, secondary: teal)) + ``` + + See [the Sass documentation][map-deep-merge] for more details. + + [map-deep-merge]: https://sass-lang.com/documentation/modules/map#deep-merge + +### Dart API + +* Add a `Value.tryMap()` function which returns the `Value` as a `SassMap` if + it's a valid map, or `null` otherwise. This allows function authors to safely + retrieve maps even if they're internally stored as empty lists, without having + to catch exceptions from `Value.assertMap()`. + ## 1.26.11 * **Potentially breaking bug fix:** `selector.nest()` now throws an error diff --git a/lib/src/functions/map.dart b/lib/src/functions/map.dart index 9e522411f..fd7b8ff32 100644 --- a/lib/src/functions/map.dart +++ b/lib/src/functions/map.dart @@ -22,7 +22,7 @@ final global = UnmodifiableListView([ /// The Sass map module. final module = BuiltInModule("map", - functions: [_get, _merge, _remove, _keys, _values, _hasKey]); + functions: [_get, _merge, _remove, _keys, _values, _hasKey, _deepMerge]); final _get = _function("get", r"$map, $key", (arguments) { var map = arguments[0].assertMap("map"); @@ -36,6 +36,12 @@ final _merge = _function("merge", r"$map1, $map2", (arguments) { return SassMap({...map1.contents, ...map2.contents}); }); +final _deepMerge = _function("deep-merge", r"$map1, $map2", (arguments) { + var map1 = arguments[0].assertMap("map1"); + var map2 = arguments[1].assertMap("map2"); + return _deepMergeImpl(map1, map2); +}); + final _remove = BuiltInCallable.overloadedFunction("remove", { // Because the signature below has an explicit `$key` argument, it doesn't // allow zero keys to be passed. We want to allow that case, so we add an @@ -73,6 +79,48 @@ final _hasKey = _function("has-key", r"$map, $key", (arguments) { return SassBoolean(map.contents.containsKey(key)); }); +/// Merges [map1] and [map2], with values in [map2] taking precedence. +/// +/// If both [map1] and [map2] have a map value associated with the same key, +/// this recursively merges those maps as well. +SassMap _deepMergeImpl(SassMap map1, SassMap map2) { + if (map2.contents.isEmpty) return map1; + + // Avoid making a mutable copy of `map2` if it would totally overwrite `map1` + // anyway. + var mutable = false; + var result = map2.contents; + void _ensureMutable() { + if (mutable) return; + mutable = true; + result = Map.of(result); + } + + // Because values in `map2` take precedence over `map1`, we just check if any + // entires in `map1` don't have corresponding keys in `map2`, or if they're + // maps that need to be merged in their own right. + map1.contents.forEach((key, value) { + var resultValue = result[key]; + if (resultValue == null) { + _ensureMutable(); + result[key] = value; + } else { + var resultMap = resultValue.tryMap(); + var valueMap = value.tryMap(); + + if (resultMap != null && valueMap != null) { + var merged = _deepMergeImpl(valueMap, resultMap); + if (identical(merged, resultMap)) return; + + _ensureMutable(); + result[key] = merged; + } + } + }); + + return mutable ? SassMap(result) : map2; +} + /// Like [new BuiltInCallable.function], but always sets the URL to `sass:map`. BuiltInCallable _function( String name, String arguments, Value callback(List arguments)) => diff --git a/lib/src/value.dart b/lib/src/value.dart index 320fe8297..1bd532caf 100644 --- a/lib/src/value.dart +++ b/lib/src/value.dart @@ -98,6 +98,8 @@ abstract class Value implements ext.Value { SassMap assertMap([String name]) => throw _exception("$this is not a map.", name); + SassMap tryMap() => null; + SassNumber assertNumber([String name]) => throw _exception("$this is not a number.", name); diff --git a/lib/src/value/external/value.dart b/lib/src/value/external/value.dart index 50a0ea7c3..2089726c5 100644 --- a/lib/src/value/external/value.dart +++ b/lib/src/value/external/value.dart @@ -104,6 +104,10 @@ abstract class Value { /// (without the `$`). It's used for error reporting. SassMap assertMap([String name]); + /// Returns [this] as a [SassMap] if it is one (including empty lists, which + /// count as empty maps) or returns `null` if it's not. + SassMap tryMap(); + /// Throws a [SassScriptException] if [this] isn't a number. /// /// If this came from a function argument, [name] is the argument name diff --git a/lib/src/value/list.dart b/lib/src/value/list.dart index fc09ecebc..1960e0c5e 100644 --- a/lib/src/value/list.dart +++ b/lib/src/value/list.dart @@ -46,6 +46,8 @@ class SassList extends Value implements ext.SassList { SassMap assertMap([String name]) => asList.isEmpty ? const SassMap.empty() : super.assertMap(name); + SassMap tryMap() => asList.isEmpty ? const SassMap.empty() : null; + bool operator ==(Object other) => (other is SassList && other.separator == separator && diff --git a/lib/src/value/map.dart b/lib/src/value/map.dart index 9e1877683..971b858c9 100644 --- a/lib/src/value/map.dart +++ b/lib/src/value/map.dart @@ -32,6 +32,8 @@ class SassMap extends Value implements ext.SassMap { SassMap assertMap([String name]) => this; + SassMap tryMap() => this; + bool operator ==(Object other) => (other is SassMap && mapEquals(other.contents, contents)) || (contents.isEmpty && other is SassList && other.asList.isEmpty); diff --git a/pubspec.yaml b/pubspec.yaml index 03d660839..aa86787d7 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.26.11-dev +version: 1.27.0-dev description: A Sass implementation in Dart. author: Sass Team homepage: https://github.com/sass/dart-sass diff --git a/test/dart_api/value/boolean_test.dart b/test/dart_api/value/boolean_test.dart index abdcea4b8..875431485 100644 --- a/test/dart_api/value/boolean_test.dart +++ b/test/dart_api/value/boolean_test.dart @@ -31,6 +31,7 @@ void main() { expect(value.assertColor, throwsSassScriptException); expect(value.assertFunction, throwsSassScriptException); expect(value.assertMap, throwsSassScriptException); + expect(value.tryMap(), isNull); expect(value.assertNumber, throwsSassScriptException); expect(value.assertString, throwsSassScriptException); }); @@ -56,6 +57,7 @@ void main() { expect(value.assertColor, throwsSassScriptException); expect(value.assertFunction, throwsSassScriptException); expect(value.assertMap, throwsSassScriptException); + expect(value.tryMap(), isNull); expect(value.assertNumber, throwsSassScriptException); expect(value.assertString, throwsSassScriptException); }); diff --git a/test/dart_api/value/color_test.dart b/test/dart_api/value/color_test.dart index 6b296c016..bef97a7ef 100644 --- a/test/dart_api/value/color_test.dart +++ b/test/dart_api/value/color_test.dart @@ -139,6 +139,7 @@ void main() { expect(value.assertBoolean, throwsSassScriptException); expect(value.assertFunction, throwsSassScriptException); expect(value.assertMap, throwsSassScriptException); + expect(value.tryMap(), isNull); expect(value.assertNumber, throwsSassScriptException); expect(value.assertString, throwsSassScriptException); }); diff --git a/test/dart_api/value/function_test.dart b/test/dart_api/value/function_test.dart index 5a0f5547b..bca96635a 100644 --- a/test/dart_api/value/function_test.dart +++ b/test/dart_api/value/function_test.dart @@ -31,6 +31,7 @@ void main() { expect(value.assertBoolean, throwsSassScriptException); expect(value.assertColor, throwsSassScriptException); expect(value.assertMap, throwsSassScriptException); + expect(value.tryMap(), isNull); expect(value.assertNumber, throwsSassScriptException); expect(value.assertString, throwsSassScriptException); }); diff --git a/test/dart_api/value/list_test.dart b/test/dart_api/value/list_test.dart index 927344981..9db1dce88 100644 --- a/test/dart_api/value/list_test.dart +++ b/test/dart_api/value/list_test.dart @@ -110,6 +110,7 @@ void main() { expect(value.assertColor, throwsSassScriptException); expect(value.assertFunction, throwsSassScriptException); expect(value.assertMap, throwsSassScriptException); + expect(value.tryMap(), isNull); expect(value.assertNumber, throwsSassScriptException); expect(value.assertString, throwsSassScriptException); }); @@ -140,6 +141,7 @@ void main() { expect(value.assertColor, throwsSassScriptException); expect(value.assertFunction, throwsSassScriptException); expect(value.assertMap, throwsSassScriptException); + expect(value.tryMap(), isNull); expect(value.assertNumber, throwsSassScriptException); expect(value.assertString, throwsSassScriptException); }); @@ -167,6 +169,7 @@ void main() { test("counts as an empty map", () { expect(value.assertMap().contents, isEmpty); + expect(value.tryMap().contents, isEmpty); }); test("isn't any other type", () { diff --git a/test/dart_api/value/map_test.dart b/test/dart_api/value/map_test.dart index 2a8b19f3f..23e241cb8 100644 --- a/test/dart_api/value/map_test.dart +++ b/test/dart_api/value/map_test.dart @@ -128,6 +128,7 @@ void main() { test("is a map", () { expect(value.assertMap(), equals(value)); + expect(value.tryMap(), equals(value)); }); test("isn't any other type", () { diff --git a/test/dart_api/value/null_test.dart b/test/dart_api/value/null_test.dart index b95a53775..6e3e9295d 100644 --- a/test/dart_api/value/null_test.dart +++ b/test/dart_api/value/null_test.dart @@ -27,6 +27,7 @@ void main() { expect(value.assertColor, throwsSassScriptException); expect(value.assertFunction, throwsSassScriptException); expect(value.assertMap, throwsSassScriptException); + expect(value.tryMap(), isNull); expect(value.assertNumber, throwsSassScriptException); expect(value.assertString, throwsSassScriptException); }); diff --git a/test/dart_api/value/number_test.dart b/test/dart_api/value/number_test.dart index 6e85ca46f..f06402450 100644 --- a/test/dart_api/value/number_test.dart +++ b/test/dart_api/value/number_test.dart @@ -102,6 +102,7 @@ void main() { expect(value.assertColor, throwsSassScriptException); expect(value.assertFunction, throwsSassScriptException); expect(value.assertMap, throwsSassScriptException); + expect(value.tryMap(), isNull); expect(value.assertString, throwsSassScriptException); }); }); diff --git a/test/dart_api/value/string_test.dart b/test/dart_api/value/string_test.dart index dfdae8b43..3e8c3642e 100644 --- a/test/dart_api/value/string_test.dart +++ b/test/dart_api/value/string_test.dart @@ -37,6 +37,7 @@ void main() { expect(value.assertColor, throwsSassScriptException); expect(value.assertFunction, throwsSassScriptException); expect(value.assertMap, throwsSassScriptException); + expect(value.tryMap(), isNull); expect(value.assertNumber, throwsSassScriptException); });