google / dagger

A fast dependency injector for Android and Java.
https://dagger.dev
Apache License 2.0
17.42k stars 2.01k forks source link

Dagger-Hilt 2.49: Activity onCreate() and onDestroy() code is generated, but our base Activity class has these final #4202

Open caltseng opened 9 months ago

caltseng commented 9 months ago

Upgrading Hilt version from 1.0.0 to 1.1.0 (dagger hilt android from 2.45 to 2.50). We have a custom Activity that we inherit from that is final, but Hilt generates code that overrides these methods and compiling doesn't work.

Edit - looks like this was added in 2.49, PR

Error message:

error: onDestroy() in Hilt_MainActivity cannot override onDestroy() in AppCompatCuustomActivity public final void onDestroy() { ^ overridden method is final 2 errors Generated Code:

  @CallSuper
  @Override
  public final void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    initSavedStateHandleHolder();
  }

  @Override
  public final void onDestroy() {
    super.onDestroy();
    if (savedStateHandleHolder != null) {
      savedStateHandleHolder.clear();
    }
  }
wanyingd1996 commented 8 months ago

Hi, Caleb, we had to override the lifecycle methods in order to lazily provide SavedStateHandle, I'll try to improve the error message, do you have to use final for those methods?

caltseng commented 8 months ago

Let me follow up to see if we can get our own custom activity to remove final. I have a suspicion those were added a long time ago and is now obsolete. It's honestly also a bit of an antipattern anyways.

caltseng commented 8 months ago

@wanyingd1996 - We removed final from our base Activity classes. Can probably close this out. (That said, still wondering if there's another pattern of implementation that could avoid this, but probably another conversation to be had if this is a more common issue)

kngako commented 3 months ago

This effects any other abstract/custom implementation with final interface methods (like BroadcastReceivers on an SMS app using klinker logic).

https://github.com/klinker41/android-smsmms/blob/master/library/src/main/java/com/klinker/android/send_message/MmsReceivedReceiver.java#L88

Chang-Eric commented 3 months ago

Unfortunately, being able to override standard methods is going to be a requirement for using Hilt. In that BroadcastReceiver example, we inject in onReceive, so there's not much we can do if it is final. I know there is a new onMessageReceived that users are supposed to override, but there's not really going to be a good way for Hilt to know how to use that. Same for any other method we might try to use. Like for onCreate there happens to be the lifecycle as an alternative (though that changes the timing and order as of calls), but for things like getDefaultViewModelProviderFactory, we have to be able to override it. It would be too limiting to say Hilt has to deal with the possibility of any standard method being possibly unavailable to override.

The best you can do is raise issues to libraries that make these methods final and ask them to use something like @CallSuper instead. Otherwise, as a workaround, you can try to use @EntryPoint annotations to inject dependencies, though for things like Activities that have their own component that unfortunately won't help you create a component for the activity to own, it'll just help you get things from the SingletonComponent.

I'm going to leave this issue open though since we could give a better error message in these cases, rather than just generating the method and letting the compiler catch the override issue.