felangel / equatable

A Dart package that helps to implement value based equality without needing to explicitly override == and hashCode.
https://pub.dev/packages/equatable
MIT License
901 stars 100 forks source link

`runtimeType` prevents from using generic type #157

Closed ValentinVignal closed 1 year ago

ValentinVignal commented 1 year ago

Describe the bug The == method uses runtimeType which can be false in some scenarios with generic types:

If we consider the data class A:

class A<T> {
  const A(this.value);

  final T value;
} 

then

A<int>(2) == A<num>(2); // false

while I would expect it to be true.

Expected behavior

I would expect A<int>(2) == A<num>(2) or A<int>(2) == A<dynamic>(2) to be true. Because in the case of data classes, what matter are the field values.

Version Paste the output of running dart --version here.

Dart SDK version: 2.18.3 (stable) (Mon Oct 17 13:23:20 2022 +0000) on "macos_x64"
Yczar commented 1 year ago

hey @ValentinVignal does this still occur... I'd like to jump on this?

ValentinVignal commented 1 year ago

@Yczar Yes the error still occurs.

It is because of the check on runtimeType in:

https://github.com/felangel/equatable/blob/6de8b97d72bd3bef52af1a369632635784866fe3/lib/src/equatable_mixin.dart#L22-L27

felangel commented 1 year ago

Hi @ValentinVignal 👋 Thanks for opening an issue!

If you remove the runtimeType check then the following would also evaluate to true:

class Cat extends Equatable {
  const Cat(this.name):
  final String name;

  @override
  List<Object> get props => [name];
}

class Dog extends Equatable {
  const Dog(this.name):
  final String name;

  @override
  List<Object> get props => [name];
}

void main() {
  final cat = Cat('Fluffy');
  final dog = Dog('Fluffy');

  print(cat == dog); // true
}
ValentinVignal commented 1 year ago

Hello @felangel , yes it is true. I'm not saying that completely removing the check on runtimeType is the correct solution. I was just saying that the behavior described in the issue was because of this check.

felangel commented 1 year ago

@ValentinVignal after discussing this a bit further I'd prefer to close this as won't fix because in order to solve this we'd need to have access to the generics and do something like:

bool operator ==(Object other) { 
  return identical(this, other) || 
    other is EquatableMixin && 
-      runtimeType == other.runtimeType && 
+     typeOf(other) is typeOf(this) 
      equals(props, other.props);
} 

This isn't possible afaik (see https://github.com/dart-lang/language/issues/945) without forcing developers to explicitly specify the generics similar to props.

I'd recommend overriding == manually for your specific use-case and hopefully one day equatable can be deprecated in favor of built-in support for data classes.

Closing for now but feel free to comment with additional thoughts/comments and I'm happy to continue the conversation 👍