onDidChange not working as expected

[pdegand]

Hi,

After trying to compare the old VM and the new VM in onDidChange to trigger some animations, I realized that the code of onDidChange is probably wrong.

Here is the code of handleChange annotated to illustrate where I think the implementation is wrong :

  void _handleChange(ViewModel vm, EventSink<ViewModel> sink) { // vm is the new VM
    _latestError = null;
    widget.onWillChange?.call(_latestValue, vm); // Here it's ok, _latestValue is the old VM, and vm is the new VM
    final previousValue = vm; // Here, previousValue is now the new VM ! This is where I think it's wrong, it should be previousValue = _latestValue
    _latestValue = vm;

    if (widget.onDidChange != null) {
      WidgetsBinding.instance?.addPostFrameCallback((_) {
        if (mounted) {
          widget.onDidChange!(previousValue, _requireLatestValue); // And finally, as previousValue has been wrongly set, previousValue and _requireLatestValue are always the same, the new, VM
        }
      });
    }

    sink.add(vm);
  }

To easily reproduce this bug, you can edit the counter sample, add a onDidChange callback to the StoreConnector that simply logs the old and the new VM :

                StoreConnector<int, String>(
                  converter: (store) => store.state.toString(),
                  // THIS IS NEW
                  onDidChange: (oldVm, newVm) {
                    print('old: $oldVm - new: $newVm');
                  },
                  // END NEW
                  builder: (context, count) {
                    return Text(
                      'The button has been pushed this many times: $count',
                      style: Theme.of(context).textTheme.headline4,
                    );
                  },
                )

And here is the log result with this modification :

I/flutter (31953): old: 1 - new: 1
I/flutter (31953): old: 2 - new: 2
I/flutter (31953): old: 3 - new: 3
I/flutter (31953): old: 4 - new: 4
I/flutter (31953): old: 5 - new: 5
I/flutter (31953): old: 6 - new: 6

We should expect to have the old value to be always 1 less than the new value.

If this is indeed a bug, I can submit a PR with a fix.

[brianegan]

Yep, looks like a bug! I'd be happy to review a bug fix.

Please start by modifying the onDidChange test case to demonstrate this problem & make the test fail. Then, implement the suggested fix to make sure the test passes as expected. That will ensure we have proper test coverage so this bug doesn't happen again in the future in case of future bug fixes or improvements.

[brianegan]

Thanks again for the report. I've created #234 to fix the issue.

[brianegan]

Version 0.10.0 fixes this bug and has support for Flutter 3. Please let me know if you spot any more problems!