GIfatahTH / states_rebuilder

a simple yet powerful state management technique for Flutter
494 stars 56 forks source link

notifyAllReactiveInstances producing grey screens in profile and release mode (Android) #119

Closed indigo-dev closed 4 years ago

indigo-dev commented 4 years ago

Hi folks,

I'm using the .asNew("some seed") function for creating an additional reative instance in some places of my app. When doing that, I also use the notifyAllReactiveInstances: true flag to let the "main" reactive model know that stuff happened and that it should notify its observers about it.

After my last release, when my app was getting all-in on States Rebuilder, some Android users reported that they suddenly see grey screens, so I started investigating. What did I found out? Some weird behaviour!

Everything is working fine and smooth when playing in dev mode on android emulator or real device connected. No errors at all. That's how I usually test.

But now weird things happen when the app is started in profile or release mode: suddenly, when moving to pages/widgets with the code mentioned above, the complete page gets grey. When I remove the notifyAllReactiveInstances paramter, the page is not becoming grey and shows the loading progress indicator as intended, but after the future (of the new reactive instance) completes the main reactive instance is not being notified, so the rest of the UI does not update.

As running in profile/release mode debugging is hard as it can get, I'm totally out of ideas what happens here. Especially because the functionality works perfectly in dev mode, I don't think that I'm doing something completely wrong.

@GIfatahTH Any experience on running things productively?

How do I setup a new reactive instances?

Variant A) -> by leaving the observe-paramter away Widget _buildRaceLapsChart() { return WhenRebuilder<DriverDetailStore>( initState: (_, storeRM) => storeRM.setState( (store) => store.getRaceLapTimes(), // notifyAllReactiveInstances: true ), onIdle: () => Container(child: Text("OnIdle")), ...

Variant B) --> building it explicitely and assigning it to a variable Widget _buildRaceLapsChart() { ReactiveModel<DriverDetailStore> storeRMrace = _storeRM.asNew("race"); return WhenRebuilder<DriverDetailStore>( observe: () => storeRMrace, initState: (_, storeRM) => storeRM.setState( (store) => store.getRaceLapTimes(), // notifyAllReactiveInstances: true ), onIdle: () => Container(child: Text("OnIdle")), ...

Variant C) --> shortcut for creating it directly in the observe-paramter Widget _buildRaceLapsChart() { return WhenRebuilder<DriverDetailStore>( observe: () => _storeRM.asNew("race"), initState: (_, storeRM) => storeRM.setState( (store) => store.getRaceLapTimes(), // notifyAllReactiveInstances: true ), onIdle: () => Container(child: Text("OnIdle")), ...

grey_screen

GIfatahTH commented 4 years ago

@indigo-dev thanks for your issue, it is an important issue.

Grey screens in release mode means a non caught assertion is thrown.

When I remove the notifyAllReactiveInstances paramter, the page is not becoming grey and shows the loading progress indicator as intended, but after the future (of the new reactive instance) completes the main reactive instance is not being notified, so the rest of the UI does not update.

The widget that shows the loading progress indicator is it registered to the main reactive instance or to the new reactive instance?

What exactly do you mean by main reactive instance? Is it injected using Injector?

indigo-dev commented 4 years ago

Hi Mellati, Right, I meant the injected one when speaking of"main reactive instance".

It turned out I had some structural problem in that corner. So thank you very much for your explanations above regarding assertions and grey screens.

After spending half of the weekend reading the docs over and over again, testing, trying, testing, I finally managed to find a working solution. In the meantime, my brain was pretty close to implode.

I've now come up with a solution that leverages the observeMany parameter, so that a StateBuilder widget observes the injected RM as well as two new reactive instances. This makes my widget tree "listen" to the three async functions I trigger in one and the same reactive model. As two of those functions are long runners, they have to be treated separately. With observeMany I also don't need the notifyListeners anymore.

I don't know if this is the ideal solution, but at least it works quite smooth and makes sense to me.

Btw., big thank you for your effort you are putting into this library.

Best regards, Daniel

GIfatahTH commented 4 years ago

I don't know if this is the ideal solution, but at least it works quite smooth and makes sense to me.

The ideal solution is the one that works.

The specificity of notifyAllReactiveInstancesis to notify all reactiveModels without changing their state (isIdle, isWaiting, hasData, hasError) except for the reactive model which issues the notification.

The implementation of notifyAllReactiveInstancesis that it emits a notification whenever the state status of the new reactive model changes.

This can create bugs, especially if the state in onWating is not defined (maybe this is your case)

I change the implement ion of notifyAllReactiveInstances so that it only emits notification if the data is ready (hasData is true) so to avoid any state inconstancy.

I released a new update, try it with your old code and see what we get (for the sake scientific).

indigo-dev commented 4 years ago

I reverted the changes and tried again. In my pubspec file, I referenced states_rebuilder: 2.3.1 , which is hopefully the version you meant. In my case, the error stayed the same. So it seems as if the problem might have been bigger than the "old" behaviour of notifyAllReactiveInstances set to true.