android / architecture-components-samples

Samples for Android Architecture Components.
https://d.android.com/arch
Apache License 2.0
23.42k stars 8.29k forks source link

Fragments create new instances of Observers after onActivityCreated #47

Closed nexuscomputing closed 4 years ago

nexuscomputing commented 7 years ago

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

UPDATE 12 Oct 2019

using viewLifecycleOwner as LifecycleOwner as proposed seems to solve my issue. Typically what I do now is the following:

class MainFragment : Fragment() {
// ... declare viewmodel lazy
    override fun onActivityCreated(savedInstanceState: Bundle?) {
        super.onActivityCreated(savedInstanceState)
        viewModel.liveData.observe(viewLifecycleOwner, Observer { item ->
           // ... code that deals with item / state goes here
        })
    }
//...
}
channae commented 5 years ago

I managed to solve this by moving adapter and viewmodel related code to onCreate (within the fragment)

quoteViewModel = ViewModelProviders.of(this, viewModelFactory).get(QuoteViewModel::class.java)

        mAdapter = QuotesAdapter(context!!, quoteViewModel)

        quoteViewModel.quoteList.observe(this, Observer { quotes ->
            mAdapter.quoteList = quotes
        })

Finally no more duplicates!!! But this is really fussy. Is this happening because LiveData are not aware of fragment lifecycle and keep ending up creating multiple observers on every onCreateView? Well, for anyone who tried this, removing observer did not work for me either. Just move logic to onCreate,

cbeyls commented 5 years ago

People should stop struggling with this, a solution has been released some time ago.

Zhuinden commented 5 years ago

@channae they added getViewLifecycleOwner() so that you can create your bindings inside onViewCreated.

dendrocyte commented 5 years ago

I am create a customized view to detect the OnActivityResult() in fragment.

I implement these 2 lib:

implementation 'androidx.appcompat:appcompat:1.1.0-rc01'
implementation 'androidx.core:core-ktx:1.2.0-alpha02'

and I feed the viewLifecycleOwner.lifecycle in onActivityCreated(savedInstanceState: Bundle?) method

override fun onActivityCreated(savedInstanceState: Bundle?) {
        super.onActivityCreated(savedInstanceState)
        ImgDisplayLifecycleObserver.registerLifecycle(viewLifecycleOwner.lifecycle)
    }

but I still get my customized view's context is from activity. I am sure I settle my customized view in fragment layout not in activity layout.

It doesn't make sense...

pavelsust commented 4 years ago

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?

i am trying to use this solution but what happened when it return null should I pass getActivity() or what?

Zhuinden commented 4 years ago

Just use the one built-in in the latest Jetpack libs

ianhanniballake commented 4 years ago

getViewLifecycleOwner() is the correct solution. There's a lint check in Fragment 1.2.0 that suggests the correct LifecycleOwner.

mvillamizar74 commented 11 months ago

Hi,

I am having the same problem, it is 2023 now, but the use of getViewLifecycleOwner() doesn't work, android Studio keep recommending to change it to its property access syntax, in other words to viewLifecycleOwner, this is what I have been using, but still have the same problem.

Zhuinden commented 11 months ago

@mvillamizar74 whether it is property access or not has no difference in behavior. If you're registering your listeners in onStart instead of onViewCreated, then it will keep happening. It will also happen if you add new listeners inside an observe block for some reason as you shouldn't.

mvillamizar74 commented 9 months ago

@Zhuinden I registered my listener on onViewCreated, but still same behaviour.

I am using Navigation View, a Single Activity application with multiple fragments, each fragment has its viewmodel, but it is like the viewmodel wasn't destroyed when I navigate to a new fragment, then when I come back LiveData observer triggered again on fragment back navigation.

Zhuinden commented 9 months ago

@mvillamizar74 this is what happens when you use sticky event bus (or LiveData, considering the same way) to deliver events that you don't even want to be sticky...

Anyway I use this https://github.com/Zhuinden/live-event