Skip to content

Commit

Permalink
Revert "Add a map.deep-merge() function (#1077)"
Browse files Browse the repository at this point in the history
This reverts commit bc7216a.

This was intended to land on a feature branch, not on master.
  • Loading branch information
nex3 committed Sep 15, 2020
1 parent bc7216a commit 9503b57
Show file tree
Hide file tree
Showing 15 changed files with 2 additions and 94 deletions.
23 changes: 0 additions & 23 deletions CHANGELOG.md
@@ -1,26 +1,3 @@
## 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: 1 addition & 49 deletions lib/src/functions/map.dart
Expand Up @@ -22,7 +22,7 @@ final global = UnmodifiableListView([

/// The Sass map module.
final module = BuiltInModule("map",
functions: [_get, _merge, _remove, _keys, _values, _hasKey, _deepMerge]);
functions: [_get, _merge, _remove, _keys, _values, _hasKey]);

final _get = _function("get", r"$map, $key", (arguments) {
var map = arguments[0].assertMap("map");
Expand All @@ -36,12 +36,6 @@ 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 @@ -79,48 +73,6 @@ 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: 0 additions & 2 deletions lib/src/value.dart
Expand Up @@ -98,8 +98,6 @@ 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: 0 additions & 4 deletions lib/src/value/external/value.dart
Expand Up @@ -104,10 +104,6 @@ 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: 0 additions & 2 deletions lib/src/value/list.dart
Expand Up @@ -46,8 +46,6 @@ 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: 0 additions & 2 deletions lib/src/value/map.dart
Expand Up @@ -32,8 +32,6 @@ 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
@@ -1,5 +1,5 @@
name: sass
version: 1.27.0-dev
version: 1.26.11-dev
description: A Sass implementation in Dart.
author: Sass Team
homepage: https://github.com/sass/dart-sass
Expand Down
2 changes: 0 additions & 2 deletions test/dart_api/value/boolean_test.dart
Expand Up @@ -31,7 +31,6 @@ 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 @@ -57,7 +56,6 @@ 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: 0 additions & 1 deletion test/dart_api/value/color_test.dart
Expand Up @@ -139,7 +139,6 @@ 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: 0 additions & 1 deletion test/dart_api/value/function_test.dart
Expand Up @@ -31,7 +31,6 @@ 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: 0 additions & 3 deletions test/dart_api/value/list_test.dart
Expand Up @@ -110,7 +110,6 @@ 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 @@ -141,7 +140,6 @@ 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 @@ -169,7 +167,6 @@ 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: 0 additions & 1 deletion test/dart_api/value/map_test.dart
Expand Up @@ -128,7 +128,6 @@ 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: 0 additions & 1 deletion test/dart_api/value/null_test.dart
Expand Up @@ -27,7 +27,6 @@ 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: 0 additions & 1 deletion test/dart_api/value/number_test.dart
Expand Up @@ -102,7 +102,6 @@ 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: 0 additions & 1 deletion test/dart_api/value/string_test.dart
Expand Up @@ -37,7 +37,6 @@ void main() {
expect(value.assertColor, throwsSassScriptException);
expect(value.assertFunction, throwsSassScriptException);
expect(value.assertMap, throwsSassScriptException);
expect(value.tryMap(), isNull);
expect(value.assertNumber, throwsSassScriptException);
});

Expand Down

0 comments on commit 9503b57

Please sign in to comment.