Skip to content

docs: clarify HomeViewModel lifecycle in Compass sample#2838

Open
harshyadavDeveloper wants to merge 2 commits into
flutter:mainfrom
harshyadavDeveloper:clarify-viewmodel-lifecycle
Open

docs: clarify HomeViewModel lifecycle in Compass sample#2838
harshyadavDeveloper wants to merge 2 commits into
flutter:mainfrom
harshyadavDeveloper:clarify-viewmodel-lifecycle

Conversation

@harshyadavDeveloper
Copy link
Copy Markdown

Adds a comment explaining why HomeViewModel is not disposed in the Compass sample.

This clarifies that the current implementation is safe because the ViewModel does not manage disposable resources. It also highlights that in real applications, proper lifecycle management is required when working with streams, controllers, or subscriptions.

Fixes #2788

Pre-launch Checklist

  • I read the [Flutter Style Guide] recently, and have followed its advice.
  • I signed the [CLA].
  • I have added sample code updates to the [changelog].
  • I updated/added relevant documentation (doc comments with ///).

@Turskyi
Copy link
Copy Markdown

Turskyi commented May 12, 2026

If I am not mistaken flutter style guide discourages the use of "Note:" in comments. Also, even though the comment is nice, but it just proves that in the long run using this sample as template we would need to dispose of it, comment basically discouraging people to use this sample as sample, when in reality what developers are looking for is a sample that they can safely follow, preferably without introducing third party library. This comment kind of saying that flutter doesn't provide any out of the box state management, and if it doesn't that sample is a bad sample.

@harshyadavDeveloper
Copy link
Copy Markdown
Author

Thank you for the feedback!

Just to make sure I'm heading in the right direction — are you suggesting that instead of adding a comment, the better fix would be to refactor the code so that HomeViewModel is created and disposed within a StatefulWidget, removing its creation from the router entirely?

Wanting to confirm before I update the PR. Happy to make that change if so!

@Turskyi
Copy link
Copy Markdown

Turskyi commented May 13, 2026

@harshyadavDeveloper , at the very least, I think the NOTE: prefix should be removed to align with the Flutter style guide:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo/5c279fe8d7b52d31ebcf05385f69c636a6b8e676#avoid-empty-prose

Technically, the added comment already addresses the original issue (#2788) because it clarifies lifecycle ownership and explains why the current implementation is safe.

That said, I still think the underlying concern remains: samples are often used as production templates, and this particular pattern becomes unsafe as soon as the ViewModel starts owning disposable resources (streams, subscriptions, controllers, etc.).

The comment itself implicitly acknowledges that limitation by explaining that “real applications” should manage disposal differently. Because of that, I think there is value in considering a small refactor that demonstrates lifecycle management directly within the sample itself, ideally using Flutter’s built-in primitives rather than relying on third-party state management libraries.

For example, creating and disposing the ViewModel from a StatefulWidget would make the ownership explicit and demonstrate a pattern that developers could safely copy into real applications without accidentally introducing leaks later.

If the Flutter team intentionally prefers the current architecture for educational simplicity, then the explanatory comment still improves the situation significantly.

@harshyadavDeveloper
Copy link
Copy Markdown
Author

Thanks for the detailed clarification — that makes sense.

Instead of keeping the explanatory comment, I’m thinking of updating the sample to move HomeViewModel ownership into a StatefulWidget, where it can be created in initState and disposed in dispose.

That would allow the sample itself to demonstrate a lifecycle-safe pattern using Flutter’s built-in primitives, while avoiding the need for additional comments about disposal ownership.

Does that direction align with what you had in mind before I proceed with the refactor?

@Turskyi
Copy link
Copy Markdown

Turskyi commented May 13, 2026

@harshyadavDeveloper, your direction sounds much closer to what I had in mind, yes.

I think if such a refactor were accepted by the Flutter team, it would make the sample significantly stronger as a real-world reference implementation, because developers frequently copy architecture patterns from official samples directly into production code.

Demonstrating explicit ViewModel ownership and disposal using only Flutter’s built-in primitives would:

  • make lifecycle ownership clear,
  • avoid ambiguity around ChangeNotifier disposal,
  • and provide a safer pattern that developers can confidently follow without accidentally introducing leaks later as the ViewModel evolves.

At the same time, it would still keep the sample framework-agnostic and avoid coupling the architecture example to a particular state-management package.

Of course, the Flutter team may still prefer the current approach for simplicity or separation-of-concerns reasons, but from an educational/sample perspective I think the refactor would provide long-term value if approved.

@harshyadavDeveloper
Copy link
Copy Markdown
Author

Updated the sample to move HomeViewModel ownership into a StatefulWidget, where it is now created in initState and disposed in dispose.

This removes the need for the explanatory comment while demonstrating an explicit lifecycle-safe pattern using Flutter’s built-in primitives.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clarify ViewModel lifecycle & disposal in Compass sample

2 participants