ListChange is not informative for ObservableList changes

[darkstarx]

Let's take the method remove and setter length of the ObservableList class:

    final list = ObservableList<int>.of(<int>[null, 2, 3]);
    list.observe((ListChange<int> listChange) {
        // (1): index == 0, added == null (!), removed == null, newValue == null, oldValue == null
        //     list: [null, 2, 3, null, null]
        // (2): index == 0, added == null, removed == null (!), newValue == null, oldValue == null
        //     list: [2, 3, null, null]
    });
    list.length = 5;      // (1) append 2 elements with value null
    list.remove(null);  // (2) remove first element with value null

Both of them leads to observe identical wrong ListChange values, this is bug.

So, fix setter length:

  @override
  set length(int value) {
    /// There is no need to enforceWritePolicy since we are conditionally wrapping in an Action.
    _context.conditionallyRunInAction(() {
      if (value < _list.length) {
        final removed = _list.sublist(value);
        _list.length = value;
        _notifyListUpdate(value, null, removed);
      } else if (value > _list.length) {
        final index = _list.length;
        _list.length = value;
        _notifyListUpdate(index, _list.sublist(value), null);
      }
    }, _atom);
  }

Note: do not notify if the length not changes.

And fix remove method:

  @override
  bool remove(Object element) {
    var didRemove = false;

    _context.conditionallyRunInAction(() {
      final index = _list.indexOf(element);
      didRemove = _list.remove(element);

      if (didRemove) {
        _notifyListUpdate(index, null, [element]);  // Here show the count of deleted values even if element is null!
      }
    }, _atom);

    return didRemove;
  }

Note: there is the same bug in the removeAt method: _notifyListUpdate(index, null, value == null ? null : [value]);! Why you hides information about the number of removed items? This is bug!
Note: removeLast contains the same bug.

And what about removeWhere method?

      final removedItems = _list.where(test).toList(growable: false);
      _list.removeWhere(test);
      _notifyListUpdate(0, null, removedItems);

Are you seriously? How can I define the indexes of removed items? It does matter for flutter AnimatedList for instance.

And what about replaceRange method?
Why you send me ListChange with OperationType.add instead of OperationType.update? Were you lazy to extend _notifyListUpdate arguments list with optional OperationType type?

Also there is bug in setRange. Why you send me update with index and without a number of modified elements? How should I interpret the value of ListChange with nulls in added, removed and oldValue, newValue properties?

[pavanpodila]

How about a PR with the fix?

[darkstarx]

How about a PR with the fix?

Shure but Permission to mobxjs/mobx.dart.git denied. I can prepare the patch for you.

0001-fix-and-improve-ListChange-details-for-ObservableLis.zip

[MaikuB]

You can fork the repository, apply the changes in your fork and submit a PR from your fork back to the original repository

[darkstarx]

Ok, thanks, I did it

[darkstarx]

Fixed in master