Fixes reload and refresh not updating internal completer#395
Fixes reload and refresh not updating internal completer#395davidmartos96 wants to merge 3 commits intorodydavis:mainfrom
Conversation
|
@davidmartos96 The same thing happened to me, how did you solve it without that change? |
|
@alfonsovgs I wasn't able to solve it without this. @rodydavis mentioned in the linked issue that it works as expected though. To me it makes much more sense the behavior with the proposed changes. |
I think the same. For now, I did this and it works, as I understand Signals a little better. class NonDistinct<T> {
const NonDistinct(this.value);
final T value;
@override
bool operator ==(Object other) => false;
@override
int get hashCode => identityHashCode(this);
}final currentLocation = futureSignal<NonDistinct<LatLng?>>(() async {
await Future.delayed(Durations.short4);
return LatLng(40.73061, -73.9);
}, debugLabel: 'currentLocation'); |
|
I still believe something else could be changed. Even with the equality added, the signal isn't properly updating when going from reloading to data. |
|
I think something that would help me in reviewing this, would be a detailed explainer on the expected behavior. Maybe a mermaid graph with the states? |
|
@rodydavis My expectation was the following after using reload(). Take the following signal as an example: final myAsyncSignal = futureSignal<int>(() async {
await Future.delayed(Duration(milliseconds: 2000));
return 10;
});It turns out that it was fixed in 6d8c4c2 by changing If this is expected, I can update the PR to just reuse the test cases, I believe they should work now with the current behavior. |
…f reloading while it didn't complete
962454e to
2a1867a
Compare
|
Yep the latest commit should be correct now! |
|
@rodydavis What do you think about the proposed tests and minor fix for the refresh() and reload() completers? |
Legacy PR
Fixes #394
The problem was that the equality in AsyncState wasn't being done properly when going from AsyncDataReloading to AsyncData because as a side effect to the reloading state being a subclass of AsyncData. I've added the equality with the 3 loading states. I think that would be the correct solution for the exhaustiveness.
Additionally, while creating a test, I've noticed that a future signal .reload() wasn't propagating the reload future because the internal Completer wasn't being recreated.
For this, would it be ok to remove the batch? I've added it because it is how it's written in
setValue, but I'm not sure it's needed.The PR updates some tests to prevent regression on issue #394 . Additionally, it makes sure that the async signals update their internal Completer instance when calling reload()/refresh(), otherwise, the value of the future returned by signal.reload() will be referring to the Future before reload() was called.
signals.dart/packages/signals_core/lib/src/async/future.dart
Lines 191 to 194 in 14c0fa9