Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a map.deep-merge() function #1077

Merged
merged 2 commits into from
Sep 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 23 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
50 changes: 49 additions & 1 deletion lib/src/functions/map.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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
Expand Down Expand Up @@ -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<Value> arguments)) =>
Expand Down
2 changes: 2 additions & 0 deletions lib/src/value.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
4 changes: 4 additions & 0 deletions lib/src/value/external/value.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions lib/src/value/list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down
2 changes: 2 additions & 0 deletions lib/src/value/map.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 2 additions & 0 deletions test/dart_api/value/boolean_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand All @@ -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);
});
Expand Down
1 change: 1 addition & 0 deletions test/dart_api/value/color_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
1 change: 1 addition & 0 deletions test/dart_api/value/function_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
3 changes: 3 additions & 0 deletions test/dart_api/value/list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down Expand Up @@ -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);
});
Expand Down Expand Up @@ -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", () {
Expand Down
1 change: 1 addition & 0 deletions test/dart_api/value/map_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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", () {
Expand Down
1 change: 1 addition & 0 deletions test/dart_api/value/null_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
1 change: 1 addition & 0 deletions test/dart_api/value/number_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
Expand Down
1 change: 1 addition & 0 deletions test/dart_api/value/string_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});

Expand Down