mobxjs / mobx.dart

MobX for the Dart language. Hassle-free, reactive state-management for your Dart and Flutter apps.
https://mobx.netlify.app
MIT License
2.41k stars 309 forks source link

Update examples to use latest Provider package #744

Open talamaska opened 2 years ago

talamaska commented 2 years ago

I was having an issue with lost state on hot reload. Looking at various examples in the repo I've stumbled upon this one

ProxyProvider<PreferencesService, SettingsStore>(
                builder: (_, preferencesService, __) =>
                    SettingsStore(preferencesService)),

This is no longer a valid syntax. Builder is already used only for building a widget and the signature has only a build context. This is now replaced by a create and update callbacks. Where create callback is creating the instance and update is updating an already existing instance with the additional dependency or we can recreate the instance with the new dependency aka the service. The side effect of this is causing a state loss on hot reload. So I would recommend using a different way of providing the service to the mobx store/state. I tested that out with get_it and it seems fine now. This is especially visible when we want to reuse a service instance in multiple stores. If we use the service only in one store, then we can bypass providing that service and avoid using the ProxyProvider, thus using only the create callback, ending up with a code that looks like this:

final SharedPreferences sharedPreferences = await SharedPreferences.getInstance();
Provider<SettingsStore>(
          create: (_) {
            return SettingsStore(
              service: PreferencesService(storage: sharedPreferences),
            );
          },
        ),

or using get_it

final GetIt di = GetIt.instance;

void setup(
  SharedPreferences sharedPreferences,
) {
  di.registerSingleton<PreferencesService>(
      PreferencesService(sharedPreferences));
}

.....

Provider<SettingsStore>(
          create: (_) {
            return SettingsStore(
              service: di.get<PreferencesService>(),
            );
          },
        ),
MaikuB commented 2 years ago

If you are finding that state is being lost due to hot reload then that would suggest to me that if you were to continue using ProxyProvider for your specific scenarios that it should actually be defined higher up the widget tree where it wraps the widget that represents the app itself and the code for this would be in separate file. That way, this becomes more closer to how you'd have a singleton instance e.g. like the get_it approach. I don't know if you did this for your app though.

Regarding updating the examples, from what I've seen at https://github.com/mobxjs/mobx.dart/blob/master/mobx_examples/lib/main.dart, @pavanpodila had already done so in late December

With how you changed to using a Provider widget, does that change much in the end? I would've though that even if we stuck with ProxyProvider, the update callback wouldn't be called multiple times with the way the examples in the repo are written

talamaska commented 2 years ago

The ProxyProvider is used in the top level widget above MaterialApp. With how I changed to Provider and get_it i don't have any issues with losing state whatsoever. What should i do to prove you that update is running multiple times, thus creating new instances. Btw it is sufficient to read the documentation of the Provider package. https://pub.dev/documentation/provider/latest/provider/ProxyProvider-class.html

As opposed to create, update may be called more than once. It will be called once the first time the value is obtained, then once whenever ProxyProvider rebuilds or when one of the providers it depends on updates.

And this https://pub.dev/documentation/provider/latest/provider/ChangeNotifierProxyProvider-class.html because mobx store is like a ChangeNotifier

DON'T create the ChangeNotifier inside update directly. This will cause your state to be lost when one of the values used updates. It will also cause unnecessary overhead because it will dispose the previous notifier, then subscribes to the new one. Instead reuse the previous instance, and update some properties or call some methods.

ChangeNotifierProxyProvider<MyModel, MyChangeNotifier>(
  // may cause the state to be destroyed involuntarily
  update: (_, myModel, myNotifier) => MyChangeNotifier(myModel: myModel),
  child: ...
);
MaikuB commented 2 years ago

What should i do to prove you that update is running multiple times, thus creating new instances.

Hmm apologies I stand corrected. I thought hot reload won't trigger trying to call the update callback. Given the Dart VM is only meant to inject updated source files, I thought it would only trigger a rebuild on the file that was updated but. Turns out it rebuilds the entire widget tree.

So to your point earlier, I would suggest examples get updated if they stick with provider to use it like the way you suggested and with alternative approaches suggested like get_it or completely change them to use a different approach. Keen to know @pavanpodila's thoughts here. For what it's worth, I've been using injectable of late and it's been working well. This uses get_it behind the scenes

pavanpodila commented 2 years ago

We can have one example using get_it. I've seen some teams use it exclusively. Happy to take a PR 😇

fzyzcjy commented 2 years ago

I've seen some teams use it exclusively.

Including me :)

Rohithgilla12 commented 2 years ago

Been using MobX + get_it, sweet combo. If it helps, dropping a small snippet below.

GetIt locator = GetIt.instance;

void setupLocator() {
    locator.registerLazySingleton<AppStore>(() => AppStore());
}

you can call the setupLocator before your runApp and then anywhere in the UI, you could use it like this

final AppStore appStore = locator<AppStore>();