DeepCollectionEquality: throws on mismatched collection shapes, and never compares non-List iterables as equal
- Dominant language
- Dart
- Stars
- 800
- Forks
- 49
- Avg merge
- 3h 39m
- Merged PRs (30d)
- 22
Description
Two issues in `DeepCollectionEquality` (`packages/mix/lib/src/core/internal/deep_collection_equality.dart` on `main`). This class backs `Equatable.==`/`hashCode`, so it is on the equality path for every `Spec` and `Mix` value.
### 1. `equals` throws instead of returning `false` for mismatched shapes
Only the first operand is type-tested; the second is cast blind:
```dart
bool equals(Object? obj1, Object? obj2) {
if (identical(obj1, obj2)) return true;
if (obj1 == null || obj2 == null) return false;
if (obj1 is Map) {
return _mapsEqual(obj1, obj2 as Map); // <-- unguarded
} else if (obj1 is Set) {
return _setsEqual(obj1, obj2 as Set); // <-- unguarded
} else if (obj1 is Iterable) {
return _iterablesEqual(obj1, obj2 as Iterable); // <-- unguarded
}
return obj1 == obj2;
}
```
Repro:
```dart
const DeepCollectionEquality().equals({1}, [1]);
// TypeError: List is not a subtype of Set
```
An equality predicate should answer `false` for mismatched shapes rather than throw. It is reachable from `Equatable.==` via `equatable.dart`, which likewise passes `unit2` unchecked once `unit1` is found to be an `Iterable`/`Map`. In practice `Equatable.==` guards on `runtimeType` first, so hitting this needs a prop whose static type admits differing shapes (`Object?`, or a bare `Iterable`) — but the guard is incidental, not by design.
Note the same file already gets this right one method up, in `_iterablesEqual`, which tests `iter1 is List && iter2 is List`.
### 2. `_iterablesEqual` returns `false` for two equal non-`List` iterables
```dart
bool _iterablesEqual(Iterable iter1, Iterable iter2) {
if (iter1.length != iter2.length) return false;
if (iter1 is List && iter2 is List) {
for (int i = 0; i < iter1.length; i++) {
if (!equals(iter1[i], iter2[i])) return false;
}
return true;
}
return false; // <-- any non-List iterable pair
}
```
This contradicts the method's own doc comment ("The iterables are equal if they have the same length and contain the same
elements in the same order"). `Set` and `Map` are dispatched earlier in `equals`, so the fallthrough is reached by lazy iterables and other non-`List` collections:
```dart
final a = [1, 2].map((e) => e);
final b = [1, 2].map((e) => e);
const DeepCollectionEquality().equals(a, b); // false
```
Consequence for a spec or `Mix` with an `Iterable`-typed prop: it never compares equal to itself, so `==` is always `false` and dependent rebuilds/lerps never short-circuit.
I checked and found no prop or spec field in `mix`, `mix_winds`, or `mix_chart` currently typed as a non-`List` `Iterable`, so this looks latent rather than actively biting.
### Secondary observation (not a correctness bug)
`hash` XORs element hashes for `Iterable`, making it order-insensitive, while `equals` is order-sensitive for `List`. So `[1, 2]` and `[2, 1]` are unequal but hash identically. That is a legal hash (collisions are permitted) and only costs bucket distribution, but it is worth a look if this code is revisited.
### Suggested handling
Issue 1 is a safe fix — guarding both operands only converts a crash into the correct `false`, with no behavior change for any input that works today. **This one is already fixed** in the `mix_core` extraction work (the file moved to `mix_core` verbatim and was corrected there).
Issue 2 is deliberately left alone in that branch: making it compare element-wise changes `==` — and therefore rebuild behavior — for any `Iterable`-typed prop library-wide, and the current tests do not cover it. It seems worth deciding explicitly rather than as a drive-by.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with packages/mix/lib/src/core/internal/deep_collection_equality.dart and trace equals and _iterablesEqual, then compare the related path in equatable.dart. Check whether the mix_core extraction is present before changing anything. Done means mismatched shapes no longer throw, and the non-List iterable behavior has an explicit decision supported by appropriate equality tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- dart, flutter
- Domain
- design, mobile-dev
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100