Account for existing _forceLastValidStreamState when rebuilding

[kuhnroyal]

• potential fix for StoreConnector.shouldUpdateModel and VmFactory.fromStore called with different state instances #89

[kuhnroyal]

There are 2 test failures in the persistence test but that has to be unrelated.

[marcglasberg]

Yes, the persistence code is correct, but the persistence tests that are failing because they depend on precise timing. I was hoping to think of a way to make them more stable, but maybe they need to be removed.

[marcglasberg]

Ideally we should first create a failing test that was fixed by this change.

[kuhnroyal]

Ideally we should first create a failing test that was fixed by this change.

I agree but that needs to be a widget test. Will see what I can do.

[marcglasberg]

Yes, if the problem is in the didUpdateWidget you need a widget test. Thank you!!

[kuhnroyal]

I added a test case which fails without the change.

I also needed to make one more change in the StoreConnector. Without setting the initial state as _mostRecentValidState, it could still fail if shouldUpdateModel returns false for the first state change.
In that case _mostRecentValidState would still be null in line https://github.com/marcglasberg/async_redux/blob/master/lib/src/store_connector.dart#L477

[kuhnroyal]

I added a test workflow, if you want to enable Github Actions.

[kuhnroyal]

Additionally I found with my test that the converter does not work together with shouldUpdateModel.
The incorrect state is passed to the converter function in https://github.com/marcglasberg/async_redux/blob/master/lib/src/store_connector.dart#L600

[kuhnroyal]

We could an an assert or change the converter function parameter, both changes are breaking.

[marcglasberg]

What changes to the converter function do you recommend?

[marcglasberg]

Typo here. "This is needed"

[kuhnroyal]

Fixed the typo.

What changes to the converter function do you recommend?

I would say, release the fix as a patch and change the parameter of the converter function in a breaking release.
So far nobody seems to have this problem, I hardly ever use the converter and only noticed this with the test case.

[marcglasberg]

@kuhnroyal Your fix is published in version 17.0.1. Thank you!