[Compass app] Viewmodels being rebuild on state change

[stdNullPtr]

This is more of a question as I am still learning, but I see that we are using the Provider for DI which is great. However, I see in some places like here:

class HomeHeader extends StatelessWidget

that we do this:

LogoutButton(
  viewModel: LogoutViewModel(
    authRepository: context.read(),
    itineraryConfigRepository: context.read(),
  ),
),

We are creating a new LogoutViewModel instead of using a singleton like the one used with the Provider. Isn't this view model going to lose its' state across widget rebuilds? Shouldn't there be strictly one ViewModel of the same type existing in the app context?

https://github.com/flutter/samples/blob/5a7decd9ed1a9557d8451757baf4a3d05a61663a/compass_app/app/lib/ui/home/widgets/home_title.dart#L40-L45

[stdNullPtr]

Learning more about the architecture it turns out a ViewModel should exist as long as the screen exists to the user.

[stdNullPtr]

Hi @ericwindmill I am once again back to this as I am unsure. Is there an actual issue in the compass app? Aren't new viewmodels created and never disposed, upon navigating?

[morismoze]

@stdNullPtr Dart's garbage collection probably handles removing unused view models from memory.

But what I'm also interested in is if the view model should be memoized (a singleton) throughout the app session lifetime, or should we always create new view model instance for every view.

[stdNullPtr]

@stdNullPtr Dart's garbage collections probably handles removing unused view models from memory.

But what I'm also interested in is if the view model should be memoized (a singleton) throughout the app session lifetime, or should we always create new view model instance for every view.

Yes that's another question, I am still learning what is proper architecture for frentend apps (flutter apps in general) so it's hard for me to reason for most things.

That's why I opened this in hopes of being enlightened by someone more experienced. If there is an architectural issue in this aspect, I would be happy to try to correct it as I'm using the flutter architecture study and this app as sources of truth in architecture design for flutter apps.

[ericwindmill]

Howdy all -- The ViewModels should live as long as their associated widget lives, and no longer. ViewModels aren't a "source of truth" in this architecture, so their job is to tell the source of truth (the model layer). If the ViewModel was long-lived, there'd likely be state conflicts, where the ViewModel's state differs from the Model layer, which leads to buggy apps and lost data.

That's how it's supposed to work in theory, its been a long time since I've looked at this code so it's possible theres a bug.

[stdNullPtr]

Howdy @ericwindmill, thanks for the clarification. So the current approach in the compass app is -- on each navigation, the GoRouter creates a fresh instance of the ViewModel. This aligns with what you explained and it makes sense.

I was wondering if they are cleaned up properly and are causing memory leaks on navigation to the said screen multiple times.

I attempted to catch this by overriding the dispose method on the viewmodel

  @override
  void dispose() {
    _log.fine('HomeViewModel destroyed');
    print('HomeViewModel destroyed');
    super.dispose();
  }

And I logged each creation in the constructor:

  HomeViewModel({
    required BookingRepository bookingRepository,
    required UserRepository userRepository,
  }) : _bookingRepository = bookingRepository,
       _userRepository = userRepository {
    load = Command0(_load)..execute();
    deleteBooking = Command1(_deleteBooking);
    _log.fine('HomeViewModel created');
  }

And after navigating for a few minutes around the app I did not see once the destroy being called, while a new object is created on each navigation. I tried with a breakpoint on dispose() also. Are we supposed to manually call dispose()? If yes, I can push a fix for that.

Is there a way I can explore the allocated objects in memory through some tooling? I am using IntelliJ for development. Am I doing something wrong during my analysis?

[morismoze]

@ericwindmill Also interested in this, I tried printing inside a dispose method of a ChangeNotifier VM, but nothing was printed in the console. For example if I push to a screen with this VM and then push to another screen (or even if I just go back to previous screen from this VM screen), the VM dispose is not invoked.

Are we supposed to manually call dispose()? If yes, I can push a fix for that.

Are we supposed to do that in the view widget itself (inside its own dispose method)? Meaning, that every widget (which has its own VM) should be a stateful widget?