[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?

samples/compass_app/app/lib/ui/home/widgets/home_title.dart

Lines 40 to 45 in 5a7decd

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

[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?

[akurnaz]

In my ViewModel classes that extend ChangeNotifier, there are certain resources that need to be discarded, and I handle this by overriding the dispose method. As previously mentioned, ViewModels created within GoRouter's builder method are re-instantiated on every route change but are not being disposed of. Using ChangeNotifierProvider from the Provider package seems appropriate for addressing this.

The ViewModel is passed to the screen widget as a constructor parameter. I believe the reason for this is to reduce dependency and increase testability. Exposing the ViewModel within the view instead of receiving it as a parameter would violate this principle.

In this case, should the responsibility for managing the ViewModel's lifecycle reside where the view is being used?

Additionally, the LoginViewModel and LogoutViewModel classes are not ChangeNotifiers. Aside from the Command object, they could be considered stateless. Therefore, it might not be an issue if they are re-instantiated with every build call.

[LeonardoRochaInacio]

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?

Did you solve that? i can't understand if the compass app is handling that in the correct way or not.

[mozammal-hossain]

@ericwindmill ,

LogoutViewModel was being constructed inside HomeHeader.build(), so it could be recreated whenever that subtree rebuilt (for example when HomeViewModel notified listeners). That did not match how other Compass view models are scoped (created in the GoRouter route builder and passed down) and could interfere with an in-flight logout.

I opened a PR that constructs LogoutViewModel once in the /home route alongside HomeViewModel, passes it through HomeScreen → HomeHeader → LogoutButton, and updates the home screen tests accordingly.

PR: #2819