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

Lists even if elements are Equatable don't cause changes #85

Closed
JohnGalt1717 opened this issue Sep 14, 2020 · 9 comments
Closed

Lists even if elements are Equatable don't cause changes #85

JohnGalt1717 opened this issue Sep 14, 2020 · 9 comments
Assignees
Labels
question Further information is requested
Projects

Comments

@JohnGalt1717
Copy link

Describe the bug
If a list is one of the properties on an object, then add/remove and per item comparisons fail which causes things like FlutterBloc etc. that rely on equitable to determine if something has changed, to also fail.

To Reproduce

  1. Create an object, add a List property and add it to props
  2. Add an item to the list, note that nothing changes and equatable still doesn't equal and as a result FlutterBloc etc. won't update.
  3. Remove an item from the list. Note that nothing changes in the status of equatable.

Expected behavior
Equatable should by default inspect all elements for equatableness of a list and flag the property dirty based on an mismatch in size or specific elements changing. Alternatively this should be a setting that you can configure, or there should be a specific type of list (EquatableList<>) that will explicitly cause the same.

Version
Dart SDK version: 2.10.0-7.3.beta (beta) (Wed Aug 26 09:46:50 2020 +0200) on "windows_x64"

@felangel felangel self-assigned this Sep 14, 2020
@felangel felangel added question Further information is requested waiting for response Waiting for follow up labels Sep 14, 2020
@felangel
Copy link
Owner

Hi @JohnGalt1717 👋
Thanks for opening an issue!

Can you please provide a link to a reproduction sample? Thanks! 🙏

@felangel felangel added this to To do in equatable Sep 14, 2020
@JohnGalt1717
Copy link
Author

@felangel Don't know what there is to reproduce. Lists don't cause equatable to change status. It's that simple. It ignores add/deletes of items from the list.

@felangel
Copy link
Owner

@JohnGalt1717 having a concrete reproduction sample would help because we can be sure we are looking at the exact same scenario.

For example, are you mutating the list (sounds like it) or are you creating a new instance of the list with the added item? I'm happy to take the time to investigate this but I would really appreciate if you could take the time to create a simple reproduction sample, thanks!

@JohnGalt1717
Copy link
Author

JohnGalt1717 commented Sep 14, 2020

Standard process of immutable classses.

Create one with a list.
Create a copy and update the list with a new element.
compare the original class with the updated class. Still shows that it's equal even though the lists are different.

@felangel
Copy link
Owner

felangel commented Sep 14, 2020

There are currently tests for this

group('List Equatable', () {

Is there a case that is missing? I'm not sure why you are so resistant to sharing a small chunk of code which reproduces the issue you're facing -- it's making it harder to reach a solution.

@JohnGalt1717
Copy link
Author

I'm trying to.

create a class, put a property in the class that is a list, set props = [listprop]
Create an instance of said class with something in the list.
Create a copy of said class
Add another item the list of the copy

Compare the original and the copy of the class.

Equatable says that the 2 are the same. They are not.

The tests you just linked to compare 2 lists, not to objects with lists in them.

@felangel
Copy link
Owner

Rather than describing in words can you share the dart code? There are tests for classes with lists in them as well:

test('should return false when values only differ in list', () {

@JohnGalt1717
Copy link
Author

The following:

import 'package:equatable/equatable.dart';

class ListObject extends Equatable {
  final bool isValue;
  ListObject(this.isValue);

  @override
  // TODO: implement props
  List<Object> get props => [isValue];
}

class A extends Equatable {
  final List<ListObject> values;
  final String someString;

  A(this.values, this.someString);

  @override
  // TODO: implement props
  List<Object> get props => [values, someString];
}

void main() {
  final objA = new A(List<ListObject>(), "something");

  final objB = new A(objA.values, objA.someString);
  objB.values.add(ListObject(true));

  print(objA == objB); //Should be false

  final objC = new A(objB.values, objB.someString);
  objC.values.add(ListObject(true));

  print(objB == objC); //Should be false
}

Results in equatable = true on line 29 and 34.

They're clearly not because objB doesn't have the same number of elements that objC has in values as an example.

It should have returned false in both cases because values has a different length.

@felangel
Copy link
Owner

felangel commented Sep 14, 2020

@JohnGalt1717 this is because you are mutating values. If instead you created a new instance of objB with a copyWith then things should work as expected:

final objC = objB.copyWith(values: List.of(objB.values)..add(ListObject(true)));
print(objB == objC); // false

This is documented in the README:

Note: Equatable is designed to only work with immutable objects so all member variables must be final (This is not just a feature of Equatable - overriding a hashCode with a mutable value can break hash-based collections).

Hope that helps 👍

@felangel felangel closed this as completed Oct 8, 2020
@felangel felangel removed the waiting for response Waiting for follow up label Oct 8, 2020
@felangel felangel moved this from To do to Done in equatable Oct 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
equatable
  
Done
Development

No branches or pull requests

2 participants