voyager icon indicating copy to clipboard operation
voyager copied to clipboard

navigator.pop() followed by navigator.push(screen) does not dispose the ScreenModel from the popped screen

Open projectdelta6 opened this issue 8 months ago • 9 comments

Describe the bug I have been trying to figure out why my screenModel retains its state and data after the screen is popped and the screenModel should be disposed, I have my screen defined as this:

class ScreenB(
	private val data: BDataClass
) : Screen {
	@Composable
	override fun Content() {
		val navigator = LocalNavigator.currentOrThrow
		val viewModel = getScreenModel<ScreenBViewModel>(
			parameters = {
				ParametersHolder().apply {
					add(data)
				}
			}
		)
		ScreenBContent(
			viewModel = viewModel,
			onNavigateBack = {
				navigator.pop()
			},
			onNavigateScreenC = { id ->
				navigator.pop()
				navigator.push(ScreenC(id))
			}
		)
}

//ScreenModel
class ScreenBViewModel(
	private val data: BDataClass
) : ScreenModel {
	init {
		Log.v("testing", "Created ScreenBViewModel")
	}

	override fun onDispose() {
		super.onDispose()
		Log.v("testing", "Disposed ScreenBViewModel")
	}
}

//Koin Module:
val viewModelModule = module {
	factory { params ->
		ScreenBViewModel(params.get())
	}
}

Then if I navigate from ScreenA to ScreenB, I see the "Created ScreenBViewModel" Log Then if I navigate back (either with system back button or my onNavigateBack callback I do see the "Disposed ScreenBViewModel" (this is as expected) however if, while on ScreenB, I call my onNavigateScreenC callback I do not see the "Disposed ScreenBViewModel" and then if I navigate back to ScreenA and then forward into ScreenB again I do not see the "Created ScreenBViewModel" Log meaning that this is still using the previous instance that should have been disposed.

in Testing I have found that if I replace the

navigator.pop()
navigator.push(ScreenC(id))

with

navigator.replace(ScreenC(id))

then the ScreenModel is disposed correctly which is fine for this example but in other places I may need to pop 2 or 3 times before pushing a new screen, and I need to screenModel(s) to be disposed correctly in those instances

To Reproduce Steps to reproduce the behavior:

  1. Set up App with ScreenA, ScreenB(with ViewModel) and ScreenC
  2. from ScreenA push ScreenB (with some data for the ScreenModel)
  3. from ScreenB pop and then push ScreenC
  4. from ScreenC pop (you should now be back on ScreenA)
  5. from ScreenA push ScreenB (with different data than on step 1)
  6. See that ScreenB's ScreenModel still has the data from step 1

Expected behavior I would expect that is I pop a screen, then it's associated ScreenModel should be disposed. Even if that pop is immediately followed by a push.

Voyager module and version:

  • voyager-navigator:1.0.0
  • voyager-screenmodel:1.0.0
  • voyager-tab-navigator:1.0.0
  • voyager-transitions:1.0.0
  • voyager-koin:1.0.0

Koin module and version:

  • koin-bom:3.5.3
  • koin-core
  • koin-compose
  • koin-android

Snippet or Sample project to help reproduce See code snippet above

projectdelta6 avatar Jun 04 '24 13:06 projectdelta6