New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fragments create new instances of Observers after onActivityCreated #47

Open
nexuscomputing opened this Issue Jun 4, 2017 · 45 comments

Comments

Projects
None yet
@nexuscomputing

nexuscomputing commented Jun 4, 2017

First of all I want to say thanks for the three samples which enlighten working with these new components significantly, looking at the code there is a lot to learn.

After trying to create a project, making use of the new arch components, I noticed that one of my Fragments received more and more events from LiveData within the Observer code after navigating back and forth in the UI.

This happens due to the Fragment instance being retained and popped back from the stack after "back" navigation.

In these examples typically in onActivityCreated LiveData is observed and a new Observer is created. In order to solve recreating new Observers, checking if onActivityCreated has been called on the instance of Fragment before seems to be the goto solution for the moment.

@yigit how would you go about this? check if savedInstanceState is null and then create Observers? I also noticed that declaring the Observers as final fields seems to solve the issue as well, meaning that registering the same Observer instance several times seems to be fine, is this something you would recommend?

Thanks a lot and keep up the good work!
Manuel

@brenoptsouza

This comment has been minimized.

Show comment
Hide comment
@brenoptsouza

brenoptsouza Jun 7, 2017

Yup, same happened to me, the two best solutions came across were:

1 - Register your observers in onCreate instead of onActivityCreated. Though I imagine this may create some errors if events are received before the views are created.

2 - Create a helper method - or if you are in Kotlin land, an extension function :) - that removes any observers previously registered to your lifecycle before observing a LiveData again. And then using it instead of LiveData.observe(). My example:

inline fun <T> LiveData<T>.reobserve(owner: LifecycleOwner, crossinline func: (T?) -> (Unit)) { removeObservers(owner) observe(owner, Observer<T> { t -> func(t) }) }

I don't know if this is the best solution either.

brenoptsouza commented Jun 7, 2017

Yup, same happened to me, the two best solutions came across were:

1 - Register your observers in onCreate instead of onActivityCreated. Though I imagine this may create some errors if events are received before the views are created.

2 - Create a helper method - or if you are in Kotlin land, an extension function :) - that removes any observers previously registered to your lifecycle before observing a LiveData again. And then using it instead of LiveData.observe(). My example:

inline fun <T> LiveData<T>.reobserve(owner: LifecycleOwner, crossinline func: (T?) -> (Unit)) { removeObservers(owner) observe(owner, Observer<T> { t -> func(t) }) }

I don't know if this is the best solution either.

@yigit

This comment has been minimized.

Show comment
Hide comment
@yigit

yigit Jun 8, 2017

Collaborator

This is an oversight on my end, sorry about it. As @brenoptsouza mentioned, there does not seem to be an obvious solution to this. We'll fix this, thanks.

Collaborator

yigit commented Jun 8, 2017

This is an oversight on my end, sorry about it. As @brenoptsouza mentioned, there does not seem to be an obvious solution to this. We'll fix this, thanks.

@yigit yigit self-assigned this Jun 8, 2017

@yigit yigit added the bug label Jun 8, 2017

@yigit

This comment has been minimized.

Show comment
Hide comment
@yigit

yigit Jun 8, 2017

Collaborator

btw, @@brenoptsouza about 1, great news is that it won't be a problem because LiveData callbacks are not called outside onStart-onStop so view will be ready for sure.

Collaborator

yigit commented Jun 8, 2017

btw, @@brenoptsouza about 1, great news is that it won't be a problem because LiveData callbacks are not called outside onStart-onStop so view will be ready for sure.

@TonicArtos

This comment has been minimized.

Show comment
Hide comment
@TonicArtos

TonicArtos Jun 25, 2017

The problem arises because the sample code has the fragments injected after Fragment::onCreate is called. So the viewModels can only be fetched after that in Fragment::onActivityCreated. Which causes the problem with observing live data multiple times.

My solution is to do fragment injection on the fragment pre-attached life cycle callback. As this is called multiple times by the framework, I set a flag on the Injectable interface to ensure the injection is only done once.

interface Injectable {
    var injected: Boolean
}

and the injection

fun OpticsPlannerApplication.initDagger() {
    DaggerAppComponent.builder()
            .application(this)
            .build()
            .inject(this)
    registerActivityLifecycleCallbacks {
        onActivityCreated { activity, _ -> activity.inject() }
    }
}

private fun Activity.inject() {
    if (this is HasSupportFragmentInjector) AndroidInjection.inject(this)
    if (this is FragmentActivity) registerFragmentLifeCycleCallbacks(recursive = true) {
        onFragmentPreAttached { _, fragment, _ ->
            fragment.inject()
        }
    }
}

private fun Fragment.inject() {
    if (this is Injectable && !injected) {
        AndroidSupportInjection.inject(this)
        injected = true
    }
}

TonicArtos commented Jun 25, 2017

The problem arises because the sample code has the fragments injected after Fragment::onCreate is called. So the viewModels can only be fetched after that in Fragment::onActivityCreated. Which causes the problem with observing live data multiple times.

My solution is to do fragment injection on the fragment pre-attached life cycle callback. As this is called multiple times by the framework, I set a flag on the Injectable interface to ensure the injection is only done once.

interface Injectable {
    var injected: Boolean
}

and the injection

fun OpticsPlannerApplication.initDagger() {
    DaggerAppComponent.builder()
            .application(this)
            .build()
            .inject(this)
    registerActivityLifecycleCallbacks {
        onActivityCreated { activity, _ -> activity.inject() }
    }
}

private fun Activity.inject() {
    if (this is HasSupportFragmentInjector) AndroidInjection.inject(this)
    if (this is FragmentActivity) registerFragmentLifeCycleCallbacks(recursive = true) {
        onFragmentPreAttached { _, fragment, _ ->
            fragment.inject()
        }
    }
}

private fun Fragment.inject() {
    if (this is Injectable && !injected) {
        AndroidSupportInjection.inject(this)
        injected = true
    }
}
@cbeyls

This comment has been minimized.

Show comment
Hide comment
@cbeyls

cbeyls Jun 25, 2017

Contributor

Newest version of the support library (26.0.0-beta2) has FragmentLifecycleCallbacks.onFragmentPreCreated().
Once stable and released this will be the best place to inject the fragments.

Contributor

cbeyls commented Jun 25, 2017

Newest version of the support library (26.0.0-beta2) has FragmentLifecycleCallbacks.onFragmentPreCreated().
Once stable and released this will be the best place to inject the fragments.

@yigit

This comment has been minimized.

Show comment
Hide comment
@yigit

yigit Jun 25, 2017

Collaborator

We can move the injection to onFragmentPreAttached so we don't need beta2.
On the other hand, a part of the problem is the conflict w/ data-binding.
When initialized w/ a retained view, data binding has no way of recovering its values. This is not even feasible since every variable needs to be Parcelable for that to work. So data binding just goes and sets whatever values it has. (it could avoid setting but that would break callbacks which may require actual values)

When data binding starts supporting LiveData as input, we can start passing the ViewModel to avoid these issues.

Until then, if we start observing in onCreate, when fragment's view is recreated (when it goes into backstack and comes back), we have to update the binding from the ViewModel manually. This is necessary because LiveData will not call the observers in this case because it already delivered the last result to that observer. We may also have a mod that will always call the LiveData callback on onStart even if it has the latest value. That is actually the fundamental problem here. Lifecycles does not have a way to know the views accessed by the callback is recreated.

This is annoying yet there is no easy solution for that until data binding supports LiveData.

Btw, this is still a tricky case to handle even w/o data binding, to be able to handle callbacks that requires non-parcelable instances. (e.g. clicking on an Entity to save it, you need the Entity).

Of course, another solution is to keep the views around when onDestroyView is called but that will increase the memory consumption so not desired.

Btw, on a side note, current state does not have any efficiency problems since data binding lazily updates UI. (in some callbacks, we force it because Espresso cannot track the path data binding is using to delay view setting but that is a bug in espresso, not data binding).

tl;dr; The whole purpose of Architecture Components is to reduce the number of edge cases like these so we'll eventually solve them.

Collaborator

yigit commented Jun 25, 2017

We can move the injection to onFragmentPreAttached so we don't need beta2.
On the other hand, a part of the problem is the conflict w/ data-binding.
When initialized w/ a retained view, data binding has no way of recovering its values. This is not even feasible since every variable needs to be Parcelable for that to work. So data binding just goes and sets whatever values it has. (it could avoid setting but that would break callbacks which may require actual values)

When data binding starts supporting LiveData as input, we can start passing the ViewModel to avoid these issues.

Until then, if we start observing in onCreate, when fragment's view is recreated (when it goes into backstack and comes back), we have to update the binding from the ViewModel manually. This is necessary because LiveData will not call the observers in this case because it already delivered the last result to that observer. We may also have a mod that will always call the LiveData callback on onStart even if it has the latest value. That is actually the fundamental problem here. Lifecycles does not have a way to know the views accessed by the callback is recreated.

This is annoying yet there is no easy solution for that until data binding supports LiveData.

Btw, this is still a tricky case to handle even w/o data binding, to be able to handle callbacks that requires non-parcelable instances. (e.g. clicking on an Entity to save it, you need the Entity).

Of course, another solution is to keep the views around when onDestroyView is called but that will increase the memory consumption so not desired.

Btw, on a side note, current state does not have any efficiency problems since data binding lazily updates UI. (in some callbacks, we force it because Espresso cannot track the path data binding is using to delay view setting but that is a bug in espresso, not data binding).

tl;dr; The whole purpose of Architecture Components is to reduce the number of edge cases like these so we'll eventually solve them.

@TonicArtos

This comment has been minimized.

Show comment
Hide comment
@TonicArtos

TonicArtos Jun 26, 2017

Since you mentioned there is no easy solution, here is my solution (using Kotlin coroutines). I push values coming from LiveData into a ConflatedChannel, which only keeps the latest value. Then when the data binding has been created, I also create a consumer of the ConflatedChannel to bind the values from it. To prevent leaking on the consumer coroutine, you need to use a LifecycleObserver or one of the older life cycle callbacks.

With some extensions, this looks someting like the following.

val incomingData = ConflatedChannel<Data>()

fun onCreate(savedInstanceState: Bundle?) {
    // snip
    observeNotNull(viewModel.loadData(query)) {
        incomingData.post(UI, it)
    }
}

fun onViewCreated(view: View?, savedInstanceState: Bundle?) {
    // snip
    launch(UI) {
        incomingData.consumeEach {
            binding.data = it
        }
    }.cancelOn(this, FragmentEvent.VIEW_DESTROYED)
}

TonicArtos commented Jun 26, 2017

Since you mentioned there is no easy solution, here is my solution (using Kotlin coroutines). I push values coming from LiveData into a ConflatedChannel, which only keeps the latest value. Then when the data binding has been created, I also create a consumer of the ConflatedChannel to bind the values from it. To prevent leaking on the consumer coroutine, you need to use a LifecycleObserver or one of the older life cycle callbacks.

With some extensions, this looks someting like the following.

val incomingData = ConflatedChannel<Data>()

fun onCreate(savedInstanceState: Bundle?) {
    // snip
    observeNotNull(viewModel.loadData(query)) {
        incomingData.post(UI, it)
    }
}

fun onViewCreated(view: View?, savedInstanceState: Bundle?) {
    // snip
    launch(UI) {
        incomingData.consumeEach {
            binding.data = it
        }
    }.cancelOn(this, FragmentEvent.VIEW_DESTROYED)
}
@cbeyls

This comment has been minimized.

Show comment
Hide comment
@cbeyls

cbeyls Sep 8, 2017

Contributor

After some thinking I realized fragments actually provide 2 distinct lifecycles:

  • The lifecycle of the fragment itself
  • The lifecycle of each view hierarchy.

My proposed solution is to create a fragment which allows accessing the lifecycle of the current view hierarchy in addition to its own.

/**
 * Fragment providing separate lifecycle owners for each created view hierarchy.
 * <p>
 * This is one possible way to solve issue https://github.com/googlesamples/android-architecture-components/issues/47
 *
 * @author Christophe Beyls
 */
public class ViewLifecycleFragment extends Fragment {

	static class ViewLifecycleOwner implements LifecycleOwner {
		private final LifecycleRegistry lifecycleRegistry = new LifecycleRegistry(this);

		@Override
		public LifecycleRegistry getLifecycle() {
			return lifecycleRegistry;
		}
	}

	@Nullable
	private ViewLifecycleOwner viewLifecycleOwner;

	/**
	 * @return the Lifecycle owner of the current view hierarchy,
	 * or null if there is no current view hierarchy.
	 */
	@Nullable
	public LifecycleOwner getViewLifeCycleOwner() {
		return viewLifecycleOwner;
	}

	@Override
	public void onViewCreated(View view, @Nullable Bundle savedInstanceState) {
		super.onViewCreated(view, savedInstanceState);
		viewLifecycleOwner = new ViewLifecycleOwner();
		viewLifecycleOwner.getLifecycle().handleLifecycleEvent(Event.ON_CREATE);
	}

	@Override
	public void onStart() {
		super.onStart();
		if (viewLifecycleOwner != null) {
			viewLifecycleOwner.getLifecycle().handleLifecycleEvent(Event.ON_START);
		}
	}

	@Override
	public void onResume() {
		super.onResume();
		if (viewLifecycleOwner != null) {
			viewLifecycleOwner.getLifecycle().handleLifecycleEvent(Event.ON_RESUME);
		}
	}

	@Override
	public void onPause() {
		if (viewLifecycleOwner != null) {
			viewLifecycleOwner.getLifecycle().handleLifecycleEvent(Event.ON_PAUSE);
		}
		super.onPause();
	}

	@Override
	public void onStop() {
		if (viewLifecycleOwner != null) {
			viewLifecycleOwner.getLifecycle().handleLifecycleEvent(Event.ON_STOP);
		}
		super.onStop();
	}

	@Override
	public void onDestroyView() {
		if (viewLifecycleOwner != null) {
			viewLifecycleOwner.getLifecycle().handleLifecycleEvent(Event.ON_DESTROY);
			viewLifecycleOwner = null;
		}
		super.onDestroyView();
	}
}

It can be used to register an observer in onActivityCreated() that will be properly unregistered in onDestroyView() automatically:

@Override
public void onActivityCreated(@Nullable Bundle savedInstanceState) {
	super.onActivityCreated(savedInstanceState);

	viewModel.getSampleData().observe(getViewLifeCycleOwner(), new Observer<String>() {
		@Override
		public void onChanged(@Nullable String result) {
			// Update views
		}
	});
}

When the fragment is detached and re-attached, the last result will be pushed to the new view hierarchy automatically as expected.

@yigit Do you think this is the right approach to solve the problem in the official library?

Contributor

cbeyls commented Sep 8, 2017

After some thinking I realized fragments actually provide 2 distinct lifecycles:

  • The lifecycle of the fragment itself
  • The lifecycle of each view hierarchy.

My proposed solution is to create a fragment which allows accessing the lifecycle of the current view hierarchy in addition to its own.

/**
 * Fragment providing separate lifecycle owners for each created view hierarchy.
 * <p>
 * This is one possible way to solve issue https://github.com/googlesamples/android-architecture-components/issues/47
 *
 * @author Christophe Beyls
 */
public class ViewLifecycleFragment extends Fragment {

	static class ViewLifecycleOwner implements LifecycleOwner {
		private final LifecycleRegistry lifecycleRegistry = new LifecycleRegistry(this);

		@Override
		public LifecycleRegistry getLifecycle() {
			return lifecycleRegistry;
		}
	}

	@Nullable
	private ViewLifecycleOwner viewLifecycleOwner;

	/**
	 * @return the Lifecycle owner of the current view hierarchy,
	 * or null if there is no current view hierarchy.
	 */
	@Nullable
	public LifecycleOwner getViewLifeCycleOwner() {
		return viewLifecycleOwner;
	}

	@Override
	public void onViewCreated(View view, @Nullable Bundle savedInstanceState) {
		super.onViewCreated(view, savedInstanceState);
		viewLifecycleOwner = new ViewLifecycleOwner();
		viewLifecycleOwner.getLifecycle().handleLifecycleEvent(Event.ON_CREATE);
	}

	@Override
	public void onStart() {
		super.onStart();
		if (viewLifecycleOwner != null) {
			viewLifecycleOwner.getLifecycle().handleLifecycleEvent(Event.ON_START);
		}
	}

	@Override
	public void onResume() {
		super.onResume();
		if (viewLifecycleOwner != null) {
			viewLifecycleOwner.getLifecycle().handleLifecycleEvent(Event.ON_RESUME);
		}
	}

	@Override
	public void onPause() {
		if (viewLifecycleOwner != null) {
			viewLifecycleOwner.getLifecycle().handleLifecycleEvent(Event.ON_PAUSE);
		}
		super.onPause();
	}

	@Override
	public void onStop() {
		if (viewLifecycleOwner != null) {
			viewLifecycleOwner.getLifecycle().handleLifecycleEvent(Event.ON_STOP);
		}
		super.onStop();
	}

	@Override
	public void onDestroyView() {
		if (viewLifecycleOwner != null) {
			viewLifecycleOwner.getLifecycle().handleLifecycleEvent(Event.ON_DESTROY);
			viewLifecycleOwner = null;
		}
		super.onDestroyView();
	}
}

It can be used to register an observer in onActivityCreated() that will be properly unregistered in onDestroyView() automatically:

@Override
public void onActivityCreated(@Nullable Bundle savedInstanceState) {
	super.onActivityCreated(savedInstanceState);

	viewModel.getSampleData().observe(getViewLifeCycleOwner(), new Observer<String>() {
		@Override
		public void onChanged(@Nullable String result) {
			// Update views
		}
	});
}

When the fragment is detached and re-attached, the last result will be pushed to the new view hierarchy automatically as expected.

@yigit Do you think this is the right approach to solve the problem in the official library?

@thanhpd56

This comment has been minimized.

Show comment
Hide comment
@thanhpd56

thanhpd56 Sep 25, 2017

@yigit is there any progress to fix this issue?

thanhpd56 commented Sep 25, 2017

@yigit is there any progress to fix this issue?

@Zhuinden

This comment has been minimized.

Show comment
Hide comment
@Zhuinden

Zhuinden Oct 30, 2017

Is this really even a bug? The lifecycle aware observers are registered until the component is destroyed. The fragment is not destroyed, only its view hierarchy is.

Zhuinden commented Oct 30, 2017

Is this really even a bug? The lifecycle aware observers are registered until the component is destroyed. The fragment is not destroyed, only its view hierarchy is.

@cbeyls

This comment has been minimized.

Show comment
Hide comment
@cbeyls

cbeyls Oct 30, 2017

Contributor

It's not a bug in itself, but the official samples show an incorrect use of LiveData in fragments and the documentation should inform about this confusion between Fragment lifecycle and View lifecycle. I wrote a full article about this last week.

Contributor

cbeyls commented Oct 30, 2017

It's not a bug in itself, but the official samples show an incorrect use of LiveData in fragments and the documentation should inform about this confusion between Fragment lifecycle and View lifecycle. I wrote a full article about this last week.

@Zhuinden

This comment has been minimized.

Show comment
Hide comment
@Zhuinden

Zhuinden Oct 30, 2017

@cbeyls oh yeah you're right! In the BasicSample - and I messed this up in my own sample too, registering in onViewCreated (with an anonymous observer) but not unregistering in onDestroyView!

Whoops >. <

Zhuinden commented Oct 30, 2017

@cbeyls oh yeah you're right! In the BasicSample - and I messed this up in my own sample too, registering in onViewCreated (with an anonymous observer) but not unregistering in onDestroyView!

Whoops >. <

@cbeyls

This comment has been minimized.

Show comment
Hide comment
@cbeyls

cbeyls Nov 21, 2017

Contributor

The video only shows a simple example involving an Activity. Things are more complex with fragment views for all the reasons mentioned above.

Contributor

cbeyls commented Nov 21, 2017

The video only shows a simple example involving an Activity. Things are more complex with fragment views for all the reasons mentioned above.

@dustedrob

This comment has been minimized.

Show comment
Hide comment
@dustedrob

dustedrob Feb 4, 2018

Was there ever a conclusion to this issue? just ran into this problem using fragments. What's the preferred way to move forward?

dustedrob commented Feb 4, 2018

Was there ever a conclusion to this issue? just ran into this problem using fragments. What's the preferred way to move forward?

@ohjames

This comment has been minimized.

Show comment
Hide comment
@ohjames

ohjames Feb 4, 2018

@dustedrob rob the article at https://medium.com/@BladeCoder/architecture-components-pitfalls-part-1-9300dd969808 lists a few possible solutions. I use the superclass + customised lifecycle one.

Hopefully one will be integrated, it's kinda funny how Google's own samples contain these errors ;)

ohjames commented Feb 4, 2018

@dustedrob rob the article at https://medium.com/@BladeCoder/architecture-components-pitfalls-part-1-9300dd969808 lists a few possible solutions. I use the superclass + customised lifecycle one.

Hopefully one will be integrated, it's kinda funny how Google's own samples contain these errors ;)

@Martindgadr

This comment has been minimized.

Show comment
Hide comment
@Martindgadr

Martindgadr Feb 14, 2018

@cbeyls @yigit I have an strange behaviour on mi ViewModel Observable, I don't know why it's called more than once if I setup observers onActivityCreated Fragment event. It's called more than one between fragment transitions. Example, I have 1 Activity 3 fragments, If I move from first to third observable on first screen is called more than once. Code Example Below:

override fun onActivityCreated(savedInstanceState: Bundle?) {
volumenViewModel.misRuedasRemuneracion.reObserve(this, observerRuedasRemu)
volumenViewModel.misSubordinados.reObserve(this, observerMisSubordinados)
volumenViewModel.remuneracionMensualActualizada.reObserve(this, observerRemuneracionMensualActualizada)
}

Base Fragment implementation:
fun LiveData.reObserve(owner: LifecycleOwner, observer: Observer) {
removeObserver(observer)
observe(owner, observer)
}

If I move these to onCreate event they are calling just once.... So why is this behaviour strange? is there any reason for that? are coming some changes on Architecture component to solve this or just it's not possible using Fragment? what happen If I just have 1 activity and some fragments and share viewmodel with multiples LiveData and need observe same live data on multiples fragments?
My question, Is because I need put observables just once on onActivityCreated or onCreateView is the same, but need call on that because a reuse of sections on fragment not onCreate.
Note: My architecture is Service from backend - Repository - Room Database with livedata - Viewmodel - Activity / Fragment with databinding.
Thanks in advance.

Martindgadr commented Feb 14, 2018

@cbeyls @yigit I have an strange behaviour on mi ViewModel Observable, I don't know why it's called more than once if I setup observers onActivityCreated Fragment event. It's called more than one between fragment transitions. Example, I have 1 Activity 3 fragments, If I move from first to third observable on first screen is called more than once. Code Example Below:

override fun onActivityCreated(savedInstanceState: Bundle?) {
volumenViewModel.misRuedasRemuneracion.reObserve(this, observerRuedasRemu)
volumenViewModel.misSubordinados.reObserve(this, observerMisSubordinados)
volumenViewModel.remuneracionMensualActualizada.reObserve(this, observerRemuneracionMensualActualizada)
}

Base Fragment implementation:
fun LiveData.reObserve(owner: LifecycleOwner, observer: Observer) {
removeObserver(observer)
observe(owner, observer)
}

If I move these to onCreate event they are calling just once.... So why is this behaviour strange? is there any reason for that? are coming some changes on Architecture component to solve this or just it's not possible using Fragment? what happen If I just have 1 activity and some fragments and share viewmodel with multiples LiveData and need observe same live data on multiples fragments?
My question, Is because I need put observables just once on onActivityCreated or onCreateView is the same, but need call on that because a reuse of sections on fragment not onCreate.
Note: My architecture is Service from backend - Repository - Room Database with livedata - Viewmodel - Activity / Fragment with databinding.
Thanks in advance.

@ohjames

This comment has been minimized.

Show comment
Hide comment
@ohjames

ohjames Feb 15, 2018

@Martindgadr I saw similar strange behaviour when using the reobserve strategy, that's why I went for a solution involving a superclass and customised lifecycle instead, see the links pasted above.

ohjames commented Feb 15, 2018

@Martindgadr I saw similar strange behaviour when using the reobserve strategy, that's why I went for a solution involving a superclass and customised lifecycle instead, see the links pasted above.

@Zhuinden

This comment has been minimized.

Show comment
Hide comment
@Zhuinden

Zhuinden Feb 15, 2018

I don't think I've ever used onActivityCreated() for anything in a Fragment.

What's it for? I'd assume onAttach(Context context) can get the activity reference already?

Zhuinden commented Feb 15, 2018

I don't think I've ever used onActivityCreated() for anything in a Fragment.

What's it for? I'd assume onAttach(Context context) can get the activity reference already?

@bernaferrari

This comment has been minimized.

Show comment
Hide comment
@bernaferrari

bernaferrari Feb 20, 2018

I am having the same problem with Activities, when user press back button and return, onCreate() is called (and since I'm observing on it..)... Is this normal?

bernaferrari commented Feb 20, 2018

I am having the same problem with Activities, when user press back button and return, onCreate() is called (and since I'm observing on it..)... Is this normal?

@Zhuinden

This comment has been minimized.

Show comment
Hide comment
@Zhuinden

Zhuinden Feb 20, 2018

@bernaferrari it's normal after process death.

Zhuinden commented Feb 20, 2018

@bernaferrari it's normal after process death.

@bernaferrari

This comment has been minimized.

Show comment
Hide comment
@bernaferrari

bernaferrari Feb 20, 2018

So, is there already a right way to fix? I hate duplicated/triplicated/... log messages.
BTW, wow, the hero from /androiddev here.. 😂

Edit: Forget everything I said. I was having a very dumb problem, where I was initialising Logger in onCreate, so it was initialised +1 time every time I would press back and open it..

bernaferrari commented Feb 20, 2018

So, is there already a right way to fix? I hate duplicated/triplicated/... log messages.
BTW, wow, the hero from /androiddev here.. 😂

Edit: Forget everything I said. I was having a very dumb problem, where I was initialising Logger in onCreate, so it was initialised +1 time every time I would press back and open it..

@cuichanghao

This comment has been minimized.

Show comment
Hide comment
@cuichanghao

cuichanghao Mar 9, 2018

@cbeyls
because recently support Fragment implementation LifecycleOwner so just need below code for remove observe.

open class ViewLifecycleFragment : Fragment() {
    override fun onDestroyView() {
        super.onDestroyView()
        (lifecycle as? LifecycleRegistry)?.handleLifecycleEvent(Lifecycle.Event.ON_DESTROY)
    }
}

EDIT
@Kernald you're right.

open class ViewLifecycleFragment : Fragment() {

    override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? {
        (lifecycle as? LifecycleRegistry)?.handleLifecycleEvent(Lifecycle.Event.ON_CREATE)
        return super.onCreateView(inflater, container, savedInstanceState)
    }

    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
        (lifecycle as? LifecycleRegistry)?.handleLifecycleEvent(Lifecycle.Event.ON_CREATE)
        super.onViewCreated(view, savedInstanceState)
    }
    override fun onDestroyView() {
        super.onDestroyView()
        (lifecycle as? LifecycleRegistry)?.handleLifecycleEvent(Lifecycle.Event.ON_DESTROY)
    }
}

don't forgot call super.onViewcreate or super.onCreateView

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
        super.onViewCreated(view, savedInstanceState)
        model.searchArticle.observe(this, Observer { articleData ->
            setData(articleData)
        })
    }

cuichanghao commented Mar 9, 2018

@cbeyls
because recently support Fragment implementation LifecycleOwner so just need below code for remove observe.

open class ViewLifecycleFragment : Fragment() {
    override fun onDestroyView() {
        super.onDestroyView()
        (lifecycle as? LifecycleRegistry)?.handleLifecycleEvent(Lifecycle.Event.ON_DESTROY)
    }
}

EDIT
@Kernald you're right.

open class ViewLifecycleFragment : Fragment() {

    override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? {
        (lifecycle as? LifecycleRegistry)?.handleLifecycleEvent(Lifecycle.Event.ON_CREATE)
        return super.onCreateView(inflater, container, savedInstanceState)
    }

    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
        (lifecycle as? LifecycleRegistry)?.handleLifecycleEvent(Lifecycle.Event.ON_CREATE)
        super.onViewCreated(view, savedInstanceState)
    }
    override fun onDestroyView() {
        super.onDestroyView()
        (lifecycle as? LifecycleRegistry)?.handleLifecycleEvent(Lifecycle.Event.ON_DESTROY)
    }
}

don't forgot call super.onViewcreate or super.onCreateView

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
        super.onViewCreated(view, savedInstanceState)
        model.searchArticle.observe(this, Observer { articleData ->
            setData(articleData)
        })
    }
@Kernald

This comment has been minimized.

Show comment
Hide comment
@Kernald

Kernald Mar 9, 2018

@cuichanghao with the solution you propose, your lifecycle doesn't really matches the fragment's one. And the creation side of your lifecycle doesn't match the destruction side anymore either (if you emit the destroy event in onDestroyView, you should emit the create event in onViewCreated).

While your solution probably works for this specific case, the idea of exposing a fragment lifecycle and a view lifecycle makes much more sense, and is probably less error prone.

Kernald commented Mar 9, 2018

@cuichanghao with the solution you propose, your lifecycle doesn't really matches the fragment's one. And the creation side of your lifecycle doesn't match the destruction side anymore either (if you emit the destroy event in onDestroyView, you should emit the create event in onViewCreated).

While your solution probably works for this specific case, the idea of exposing a fragment lifecycle and a view lifecycle makes much more sense, and is probably less error prone.

@yperess

This comment has been minimized.

Show comment
Hide comment
@yperess

yperess Jun 7, 2018

Hey guys, chiming in on this "old" bug. The correct solution (which I have working in 2 production apps) is:

  1. Either extend DaggerFragment or add AndroidSupportInjection.inject(this) to onAttach as well as implement HasSupportFragmentInjector for your fragment. Note the lifecycle on: https://developer.android.com/guide/components/fragments
  2. Register all of your observers in onCreate

The key here is that onAttach and onCreate are the only methods that are guaranteed to run once per fragment instance.

yperess commented Jun 7, 2018

Hey guys, chiming in on this "old" bug. The correct solution (which I have working in 2 production apps) is:

  1. Either extend DaggerFragment or add AndroidSupportInjection.inject(this) to onAttach as well as implement HasSupportFragmentInjector for your fragment. Note the lifecycle on: https://developer.android.com/guide/components/fragments
  2. Register all of your observers in onCreate

The key here is that onAttach and onCreate are the only methods that are guaranteed to run once per fragment instance.

@Zhuinden

This comment has been minimized.

Show comment
Hide comment
@Zhuinden

Zhuinden Jun 7, 2018

onCreate isn't good because if the fragment is detached and reattached (think fragment state pager) then you won't be able to update the re-created view with the latest data in ViewModel.

One day they'll release Fragments with the viewLifecycle and that'll solve issues.

For now, I think the solution is to subscribe in onViewCreated, and remove observers in onDestroyView.

Zhuinden commented Jun 7, 2018

onCreate isn't good because if the fragment is detached and reattached (think fragment state pager) then you won't be able to update the re-created view with the latest data in ViewModel.

One day they'll release Fragments with the viewLifecycle and that'll solve issues.

For now, I think the solution is to subscribe in onViewCreated, and remove observers in onDestroyView.

@cbeyls

This comment has been minimized.

Show comment
Hide comment
@cbeyls

cbeyls Jun 7, 2018

Contributor

The proper fix is to use Fragment.getViewLifecycleOwner() which is already part of AndroidX.

Contributor

cbeyls commented Jun 7, 2018

The proper fix is to use Fragment.getViewLifecycleOwner() which is already part of AndroidX.

@yperess

This comment has been minimized.

Show comment
Hide comment
@yperess

yperess Jun 12, 2018

Thanks @cbeyls, any idea when androix will make its way into Dagger2's DaggerFragment and DaggerAppCompatActivity?

yperess commented Jun 12, 2018

Thanks @cbeyls, any idea when androix will make its way into Dagger2's DaggerFragment and DaggerAppCompatActivity?

@JobGetabu

This comment has been minimized.

Show comment
Hide comment
@JobGetabu

JobGetabu Jun 23, 2018

any workable solution. fragments are not updating real time data :(

JobGetabu commented Jun 23, 2018

any workable solution. fragments are not updating real time data :(

@Zhuinden

This comment has been minimized.

Show comment
Hide comment
@Zhuinden

Zhuinden Jun 23, 2018

There's a pretty good chance that that's an error on your side, though?

This Github repo doesn't have anything with "real time data" i think

Zhuinden commented Jun 23, 2018

There's a pretty good chance that that's an error on your side, though?

This Github repo doesn't have anything with "real time data" i think

@hardysim

This comment has been minimized.

Show comment
Hide comment
@hardysim

hardysim Jun 25, 2018

The proper fix is to use Fragment.getViewLifecycleOwner() which is already part of AndroidX.

It seems that in support lib 28.0.0-alpha01 it is not included. Can someone confirm? I cannot upgrade to androidx at the moment.

hardysim commented Jun 25, 2018

The proper fix is to use Fragment.getViewLifecycleOwner() which is already part of AndroidX.

It seems that in support lib 28.0.0-alpha01 it is not included. Can someone confirm? I cannot upgrade to androidx at the moment.

@Hydrino

This comment has been minimized.

Show comment
Hide comment
@Hydrino

Hydrino Jun 26, 2018

Yes hardysim, it is not included in 28.0.0 - alpha01.

Hydrino commented Jun 26, 2018

Yes hardysim, it is not included in 28.0.0 - alpha01.

@cs-matheus-candido

This comment has been minimized.

Show comment
Hide comment
@cs-matheus-candido

cs-matheus-candido Jul 2, 2018

FYI: alpha03 includes this, thank god

cs-matheus-candido commented Jul 2, 2018

FYI: alpha03 includes this, thank god

@Hydrino

This comment has been minimized.

Show comment
Hide comment
@Hydrino

Hydrino Jul 3, 2018

Hydrino commented Jul 3, 2018

@nicolasjafelle

This comment has been minimized.

Show comment
Hide comment
@nicolasjafelle

nicolasjafelle Jul 20, 2018

Hello! I am new using ViewModel and LiveData arch components and have the problem when using fragments and rotate the screen the observer get triggered...
I tried to move the viewModel = ViewModelProviders.of(this).get(MainViewModel::class.java) in all the fragment lifecycle methods but with no success.

My scenario is kind of really easy one:

  1. Login screen with email and password
  2. User clicks on the "login" button
  3. the viewmodel calls the login(email, password
  4. the viewModel set the value of the LiveData object
  5. Just for now simple show a Toast

At this point everything is Ok. But when I rotate the screen the Toast appears again without any user interaction.

Do I have to do something in onDestroyView() ?

nicolasjafelle commented Jul 20, 2018

Hello! I am new using ViewModel and LiveData arch components and have the problem when using fragments and rotate the screen the observer get triggered...
I tried to move the viewModel = ViewModelProviders.of(this).get(MainViewModel::class.java) in all the fragment lifecycle methods but with no success.

My scenario is kind of really easy one:

  1. Login screen with email and password
  2. User clicks on the "login" button
  3. the viewmodel calls the login(email, password
  4. the viewModel set the value of the LiveData object
  5. Just for now simple show a Toast

At this point everything is Ok. But when I rotate the screen the Toast appears again without any user interaction.

Do I have to do something in onDestroyView() ?

@JobGetabu

This comment has been minimized.

Show comment
Hide comment
@JobGetabu

JobGetabu Jul 20, 2018

Have you tried to get same activity with:
viewModel = ViewModelProviders.of(getActivity()).get(MainViewModel::class.java)

JobGetabu commented Jul 20, 2018

Have you tried to get same activity with:
viewModel = ViewModelProviders.of(getActivity()).get(MainViewModel::class.java)

@arekolek

This comment has been minimized.

Show comment
Hide comment
@arekolek

arekolek Jul 20, 2018

arekolek commented Jul 20, 2018

@victsomie

This comment has been minimized.

Show comment
Hide comment
@victsomie

victsomie Aug 5, 2018

Could somebody be having this challenge still?

I encountered the same while using ViewModel inside fragment, which were working on my Activities.
I solved by making my Fragment inherit from DaggerFragment. (MyFragment extends DaggerFragment)

victsomie commented Aug 5, 2018

Could somebody be having this challenge still?

I encountered the same while using ViewModel inside fragment, which were working on my Activities.
I solved by making my Fragment inherit from DaggerFragment. (MyFragment extends DaggerFragment)

@AdamSHurwitz

This comment has been minimized.

Show comment
Hide comment
@AdamSHurwitz

AdamSHurwitz Aug 17, 2018

Hi @yigit, @cbeyls, and @victsomie, I am working through a similar issue outlined on Stackoverflow.

I have multiple LiveData Observers that are being triggered in a Fragment after navigating to a new Fragment, popping the new Fragment, and returning to the original Fragment.

Details: The architecture consists of MainActivity that hosts a HomeFragment as the start destination in the MainActivity's navigation graph. Within HomeFragment is a programmatically inflated PriceGraphFragment. The HomeFragment is using the navigation component to launch a new child Fragment ProfileFragment. On back press the ProfileFragment is popped and the app returns to the HomeFragment hosting the PriceGraphFragment. The PriceGraphFragment is where the Observer is being called multiple times.

You can see the StackOverflow more detailed implementation.

Attempted Solutions

  1. Creating the Fragment's ViewModel in the onCreate() method.
    priceViewModel = ViewModelProviders.of(this).get(PriceDataViewModel::class.java)
  2. Moving methods that create the Observers to the Fragment's onCreate() method.
  3. Using viewLifecycleOwner instead of this for the LifecycleOwner in the method observe(@NonNull LifecycleOwner owner, @NonNull Observer<? super T> observer).

AdamSHurwitz commented Aug 17, 2018

Hi @yigit, @cbeyls, and @victsomie, I am working through a similar issue outlined on Stackoverflow.

I have multiple LiveData Observers that are being triggered in a Fragment after navigating to a new Fragment, popping the new Fragment, and returning to the original Fragment.

Details: The architecture consists of MainActivity that hosts a HomeFragment as the start destination in the MainActivity's navigation graph. Within HomeFragment is a programmatically inflated PriceGraphFragment. The HomeFragment is using the navigation component to launch a new child Fragment ProfileFragment. On back press the ProfileFragment is popped and the app returns to the HomeFragment hosting the PriceGraphFragment. The PriceGraphFragment is where the Observer is being called multiple times.

You can see the StackOverflow more detailed implementation.

Attempted Solutions

  1. Creating the Fragment's ViewModel in the onCreate() method.
    priceViewModel = ViewModelProviders.of(this).get(PriceDataViewModel::class.java)
  2. Moving methods that create the Observers to the Fragment's onCreate() method.
  3. Using viewLifecycleOwner instead of this for the LifecycleOwner in the method observe(@NonNull LifecycleOwner owner, @NonNull Observer<? super T> observer).
@TonicArtos

This comment has been minimized.

Show comment
Hide comment
@TonicArtos

TonicArtos Aug 17, 2018

TonicArtos commented Aug 17, 2018

@cbeyls

This comment has been minimized.

Show comment
Hide comment
@cbeyls

cbeyls Aug 17, 2018

Contributor

You can do what @TonicArtos suggests but don't forget that if you do that, you'll have to update the views manually by fetching the latest result from the LiveData in onActivityCreated() when the fragment is re-attached. That's because the observer doesn't change so the latest result won't be pushed automatically to the new view hierarchy, until a new result arrives.

Contributor

cbeyls commented Aug 17, 2018

You can do what @TonicArtos suggests but don't forget that if you do that, you'll have to update the views manually by fetching the latest result from the LiveData in onActivityCreated() when the fragment is re-attached. That's because the observer doesn't change so the latest result won't be pushed automatically to the new view hierarchy, until a new result arrives.

@AdamSHurwitz

This comment has been minimized.

Show comment
Hide comment
@AdamSHurwitz

AdamSHurwitz Aug 17, 2018

Thanks for the feedback @TonicArtos and @cbeyls!

@TonicArtos - I've created all Observers in onCreate() method and am still seeing the Observers called 2 additional times. In my log below this is represented by the observeGraphData() statement I'm printing out when the Observer is fired.

screen shot 2018-08-17 at 10 50 47 am

@cbeyls - Thank you for the heads up. As I'm still seeing multiple Observers with @TonicArtos's strategy I haven't gotten to this stage of implementation.

Perhaps the issue is related to creating the nested ChildFragment (PriceGraphFragment) in the ParentFragment's (HomeFragment) onViewCreated()?

ParentFragment

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
        super.onViewCreated(view, savedInstanceState)
        user = viewModel.getCurrentUser()
        if (savedInstanceState == null) {
            fragmentManager
                    ?.beginTransaction()
                    ?.add(binding.priceDataContainer.id, PriceGraphFragment.newInstance())
                    ?.commit()
        }

AdamSHurwitz commented Aug 17, 2018

Thanks for the feedback @TonicArtos and @cbeyls!

@TonicArtos - I've created all Observers in onCreate() method and am still seeing the Observers called 2 additional times. In my log below this is represented by the observeGraphData() statement I'm printing out when the Observer is fired.

screen shot 2018-08-17 at 10 50 47 am

@cbeyls - Thank you for the heads up. As I'm still seeing multiple Observers with @TonicArtos's strategy I haven't gotten to this stage of implementation.

Perhaps the issue is related to creating the nested ChildFragment (PriceGraphFragment) in the ParentFragment's (HomeFragment) onViewCreated()?

ParentFragment

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
        super.onViewCreated(view, savedInstanceState)
        user = viewModel.getCurrentUser()
        if (savedInstanceState == null) {
            fragmentManager
                    ?.beginTransaction()
                    ?.add(binding.priceDataContainer.id, PriceGraphFragment.newInstance())
                    ?.commit()
        }
@TonicArtos

This comment has been minimized.

Show comment
Hide comment
@TonicArtos

TonicArtos Aug 19, 2018

TonicArtos commented Aug 19, 2018

@AdamSHurwitz

This comment has been minimized.

Show comment
Hide comment
@AdamSHurwitz

AdamSHurwitz Aug 21, 2018

@TonicArtos, great idea.

I'm logging the hashcode of the HashMap being emitted by the Observer and it is showing 2 unique hashcodes when I go to the child Fragment, pop the child Fragment, and return to the original Fragment.

This is opposed to the one hashcode seen from the HashMap when I rotate the screen without launching the child Fragment.

AdamSHurwitz commented Aug 21, 2018

@TonicArtos, great idea.

I'm logging the hashcode of the HashMap being emitted by the Observer and it is showing 2 unique hashcodes when I go to the child Fragment, pop the child Fragment, and return to the original Fragment.

This is opposed to the one hashcode seen from the HashMap when I rotate the screen without launching the child Fragment.

@TonicArtos

This comment has been minimized.

Show comment
Hide comment
@TonicArtos

TonicArtos Aug 22, 2018

TonicArtos commented Aug 22, 2018

@AdamSHurwitz

This comment has been minimized.

Show comment
Hide comment
@AdamSHurwitz

AdamSHurwitz Aug 22, 2018

First off, thank you to everyone who posted here. It was a combination of your advice and pointers that helped me solve this bug over the past 5 days as there were multiple issues involved. I've posted the answer below in the corresponding Stackoverflow. Please provide feedback there if my solution provides value to you.

Issues Resolved

  1. Creating nested Fragments properly in parent Fragment (HomeFragment).

Before:

override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?,
                          savedInstanceState: Bundle?): View? {

        if (savedInstanceState == null) {
        fragmentManager
                ?.beginTransaction()
                ?.add(binding.priceDataContainer.id, PriceGraphFragment.newInstance())
                ?.commit()
        fragmentManager
                ?.beginTransaction()
                ?.add(binding.contentFeedContainer.id, ContentFeedFragment.newInstance())
                ?.commit()
    }
...
}

After:

override fun onActivityCreated(savedInstanceState: Bundle?) {
    super.onActivityCreated(savedInstanceState)

    if (savedInstanceState == null
            && childFragmentManager.findFragmentByTag(PRICEGRAPH_FRAGMENT_TAG) == null
            && childFragmentManager.findFragmentByTag(CONTENTFEED_FRAGMENT_TAG) == null) {
        childFragmentManager.beginTransaction()
                .replace(priceDataContainer.id, PriceGraphFragment.newInstance(),
                        PRICEGRAPH_FRAGMENT_TAG)
                .commit()
        childFragmentManager.beginTransaction()
                .replace(contentFeedContainer.id, ContentFeedFragment.newInstance(),
                        CONTENTFEED_FRAGMENT_TAG)
                .commit()
    }
...
}
  1. Creating ViewModels in onCreate() as opposed to onCreateView() for both the parent and child Fragments.

  2. Initializing request for data (Firebase Firestore query) data of child Fragment (PriceFragment) in onCreate() rather than onViewCreated() but still doing so only when saveInstanceState is null.

Non Factors

A couple items were suggested but turned out to not have an impact in solving this bug.

  1. Creating Observers in onActivityCreated(). I'm keeping mine in onViewCreated() of the child Fragment (PriceFragment).

  2. Using viewLifecycleOwner in the Observer creation. I was using the child Fragment (PriceFragment)'s this before. Even though viewLifecycleOwner does not impact this bug it seems to be best practice overall so I'm keeping this new implementation.

AdamSHurwitz commented Aug 22, 2018

First off, thank you to everyone who posted here. It was a combination of your advice and pointers that helped me solve this bug over the past 5 days as there were multiple issues involved. I've posted the answer below in the corresponding Stackoverflow. Please provide feedback there if my solution provides value to you.

Issues Resolved

  1. Creating nested Fragments properly in parent Fragment (HomeFragment).

Before:

override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?,
                          savedInstanceState: Bundle?): View? {

        if (savedInstanceState == null) {
        fragmentManager
                ?.beginTransaction()
                ?.add(binding.priceDataContainer.id, PriceGraphFragment.newInstance())
                ?.commit()
        fragmentManager
                ?.beginTransaction()
                ?.add(binding.contentFeedContainer.id, ContentFeedFragment.newInstance())
                ?.commit()
    }
...
}

After:

override fun onActivityCreated(savedInstanceState: Bundle?) {
    super.onActivityCreated(savedInstanceState)

    if (savedInstanceState == null
            && childFragmentManager.findFragmentByTag(PRICEGRAPH_FRAGMENT_TAG) == null
            && childFragmentManager.findFragmentByTag(CONTENTFEED_FRAGMENT_TAG) == null) {
        childFragmentManager.beginTransaction()
                .replace(priceDataContainer.id, PriceGraphFragment.newInstance(),
                        PRICEGRAPH_FRAGMENT_TAG)
                .commit()
        childFragmentManager.beginTransaction()
                .replace(contentFeedContainer.id, ContentFeedFragment.newInstance(),
                        CONTENTFEED_FRAGMENT_TAG)
                .commit()
    }
...
}
  1. Creating ViewModels in onCreate() as opposed to onCreateView() for both the parent and child Fragments.

  2. Initializing request for data (Firebase Firestore query) data of child Fragment (PriceFragment) in onCreate() rather than onViewCreated() but still doing so only when saveInstanceState is null.

Non Factors

A couple items were suggested but turned out to not have an impact in solving this bug.

  1. Creating Observers in onActivityCreated(). I'm keeping mine in onViewCreated() of the child Fragment (PriceFragment).

  2. Using viewLifecycleOwner in the Observer creation. I was using the child Fragment (PriceFragment)'s this before. Even though viewLifecycleOwner does not impact this bug it seems to be best practice overall so I'm keeping this new implementation.

gobetti added a commit to gobetti/CodeChallenge-Android that referenced this issue Sep 3, 2018

Fixed multiple observers alive
googlesamples/android-architecture-components#47
As pointed out in that blogpost, everything's fine if a fragment is
destroyed (after a rotation for example), however if it's
detached/attached then the observer isn't removed and a new one is
created every time - unless we use `viewLifecycleOwner` instead of
`this` as the observation's owner.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment