Skip to content

Bugfix: bind MoleculeViewModels as ViewModel and complete Metro wiring#2962

Merged
StylianosGakis merged 6 commits into
eng/metro-nav3-pr1-koin-to-metrofrom
eng/metro-nav3-pr1-koin-to-metro-start-crush-fix
Jun 2, 2026
Merged

Bugfix: bind MoleculeViewModels as ViewModel and complete Metro wiring#2962
StylianosGakis merged 6 commits into
eng/metro-nav3-pr1-koin-to-metrofrom
eng/metro-nav3-pr1-koin-to-metro-start-crush-fix

Conversation

@panasetskaya
Copy link
Copy Markdown
Contributor

After the Koin→Metro migration, the app crashed on startup:

  java.lang.IllegalArgumentException: Unknown model class CrossSellSheetViewModel                                                                                      
      at dev.zacsweers.metrox.viewmodel.MetroViewModelFactory.create(...)      

Root cause

Metro's @ContributesIntoMap infers the bound type from a class's single declared supertype. Our ViewModels extend MoleculeViewModel<Event, State>, so they were contributed as MoleculeViewModel<…> instead of ViewModel — and never landed in the Map<KClass<out ViewModel>, () -> ViewModel> that MetroViewModelFactory reads. They were silently dropped, then crashed when first requested. 40 ViewModels were affected (assisted-factory ViewModels were not, since they bind via a factory interface).

Fixing that surfaced a second layer: because the dropped ViewModels were never validated, the graph compiled despite genuinely missing dependency bindings. Once the VMs were correctly in the graph, Metro reported 10 MissingBinding errors.

Decisions

  • binding<ViewModel>() — per Metro's docs, a ViewModel that extends an intermediate base must specify the bound type explicitly. Applied to all 40 MoleculeViewModel subclasses.
  • Public provider surface over graph extensions — feature-internal Provider<UseCase> bindings couldn't feed the single shared AppGraph (:app can only reference public types). Graph extensions were considered but rejected: a parent graph can't see child-extension bindings, and MetroViewModelFactory lives in the parent — so they'd require re-architecting how every screen resolves its ViewModel. Instead we made the relevant use cases/repositories + their providers public, matching the existing pattern (cross-sell already worked because its model lives in a public module).
  • ApplicationScope over raw CoroutineScope — HomeViewModel was the lone outlier injecting CoroutineScope; switched to ApplicationScope like the rest of the codebase.
  • Assisted factories for SavedStateHandleSavedStateHandle is only available via CreationExtras at creation time, not as a graph binding, so those ViewModels were converted to metrox's ViewModelAssistedFactory.

Changes

  • 40 ViewModels: added binding<ViewModel>() to @ContributesIntoMap.
  • data-addons: provide Provider<GetAddonBannerInfoUseCase> (base type) from DataAddonsMetroProviders.
  • cross-sell, forever, home, payments, profile: made use-case/repository interfaces, impls, demos, providers and @ContributesTo modules public so their Provider bindings reach AppGraph (includes the public model cascade, e.g. HomeData, PaymentOverview, ContactInformation).
  • home: bound SeenImportantMessagesStorage (@Inject + @ContributesBinding); HomeViewModel/HomePresenter now inject ApplicationScope.
  • SavedStateHandle VMs (SwedishLogin, movingflow EnterNewAddress / AddHouseInformation / Summary): converted to @AssistedInject + nested ViewModelAssistedFactory; call sites updated to assistedMetroViewModel().

@panasetskaya panasetskaya requested a review from a team as a code owner June 1, 2026 16:35
Construct the prod and demo implementations inline inside the Metro
@provides factories instead of contributing them as bindings. Only the
factory needs the impls, so they (and their ProdOrDemoProviders) can drop
their DI annotations and become internal, keeping GraphQL/impl details out
of cross-module visibility.
Construct GetHomeDataUseCase prod/demo impls inline in the Metro @provides
factory and make them (plus the provider and SeenImportantMessagesStorageImpl)
internal, so the impls are no longer exposed across the module boundary.
Construct the prod and demo ContactInfoRepository impls inline in the Metro
@provides factory and make them and ProfileRepositoryProvider internal.
Construct the prod and demo ForeverRepository impls inline in the Metro
@provides factory and make them and ForeverRepositoryProvider internal.
Construct the prod and demo GetCrossSellSheetDataUseCase impls inline in the
Metro @provides factory and make them internal.
@StylianosGakis StylianosGakis merged commit 6b98c6c into eng/metro-nav3-pr1-koin-to-metro Jun 2, 2026
3 of 4 checks passed
@StylianosGakis StylianosGakis deleted the eng/metro-nav3-pr1-koin-to-metro-start-crush-fix branch June 2, 2026 10:45
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.

2 participants