google / dagger

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

Using DagggerAndroid in Library Projects #1007

Closed mohamad-amin closed 6 years ago

mohamad-amin commented 6 years ago

I want to use dagger in my library project but i've encountered a problem. I have a singleton in my library named LibraryCore class that acts as an Application class in an android project. I want to use the android related dagger features and therefor i'm implementing HasActivityInjector in LibraryCore. So my LibraryCore class looks like this:

public class LibraryCore implements HasActivityInjector {

    private static LibraryCore INSTANCE;

    @Inject
    DispatchingAndroidInjector<Activity> activityInjector;

    public static void init(Context context) {
        INSTANCE = new LibraryCore(context);
    }

    private LibraryCore(Context context) {
        beginInjection(context);
    }

    private void beginInjection(Context context) {
        DaggerLibraryComponent.builder()
                .context(context)
                .build()
                .inject(this);
    }

    @Override
    public AndroidInjector<Activity> activityInjector() {
        return activityInjector;
    }

}

But when I start an activity in my library i get the following error: "Application does not implement HasActivityInjector". I think its because of line 5 in this method from the AndroidInjection class:

public static void inject(Activity activity) {
    ...
    Application application = activity.getApplication();
    if(!(application instanceof HasActivityInjector)) {
        throw new RuntimeException(String.format("%s does not implement %s", new Object[]{application.getClass().getCanonicalName(), HasActivityInjector.class.getCanonicalName()}));
    } else {
        AndroidInjector<Activity> activityInjector = ((HasActivityInjector)application).activityInjector();
        ...
    }
}

And because of dagger assuming that the Application class of the project is always the class that initiates the main dagger Component of the app. This assumption prevents you from using these dagger features inside a library, because we are publishing the library for everyone out there and they might not like to implement HasActivityInjector in their project's application classes.

I think this can be solved by simply providing a method like AndroidInjection.inject(Activity, HasActivityInjector) or ...

P.S: here's my LibraryComponent class if you're interested:

@Singleton
@Component(modules = { SomeModule.class })
public interface LibraryComponent {

    @Component.Builder
    interface Builder {
        @BindsInstance Builder context(Context context);
        LibraryComponent build();
    }

    void inject(LibraryCore libraryCore);

}
mohamad-amin commented 6 years ago

Also we can inject in activities manually by calling

LibraryCore.getInstance().activityInjector().inject(this);
ronshapiro commented 6 years ago

Why do you need LibraryCore? Can your library provide the necessary activities/fragments and then rely on the application setting up dagger.android as described in the docs? It's not clear to me what you're trying to do honestly.

Either way, this is probably better suited for StackOverflow.

mohamad-amin commented 6 years ago

First, why this problem relates to #1004 and marked as a duplicate of it? Second:

Can your library provide the necessary activities/fragments and then rely on the application setting up dagger.android as described in the docs?

No! As we can use static fields for views, but we don't! Library is separated from the application, as I said in the first post:

And because of dagger assuming that the Application class of the project is always the class that initiates the main dagger Component of the app. This assumption prevents you from using these dagger features inside a library, because we are publishing the library for everyone out there and they might not like to implement HasActivityInjector in their project's application classes.

We can't assume the application that imports the library also wants to use dagger! Many people might library X while not interested in using dagger in their applications and implementing HasActivityInjector in it, whilst the library X uses dagger. It is also a better choice to separate the library and the application as we want to separate our concerns, so the application only connects to the library through loosely coupled interfaces. I don't think its a good idea to tell the application developers to implement HasActivityInjector and all the implementation details about library X just because the library uses dagger!

Also, I think this is a feature request! not a StackOverflow question!

JakeWharton commented 6 years ago

How is the LibraryCore instance supposed to be looked up. Your proposal lacks any clear solution to this problem. If you're holding a static instance of it then dagger.android serves no purpose as you don't need decoupling and can reference it directly from your fragments and activities.

On Fri, Jan 5, 2018, 6:48 PM Mohamad Amin Mohamadi notifications@github.com wrote:

First, why this problem relates to #1004 https://github.com/google/dagger/issues/1004 and marked as a duplicate of it? Second:

Can your library provide the necessary activities/fragments and then rely on the application setting up dagger.android as described in the docs?

No! As we can use static fields for views, but we don't! Library is separated from the application, as I said in the first post:

And because of dagger assuming that the Application class of the project is always the class that initiates the main dagger Component of the app. This assumption prevents you from using these dagger features inside a library, because we are publishing the library for everyone out there and they might not like to implement HasActivityInjector in their project's application classes.

We can't assume the application that imports the library also wants to use dagger! Many people might library X while not interested in using dagger in their applications and implementing HasActivityInjector in it, whilst the library X uses dagger. It is also a better choice to separate the library and the application as we want to separate our concerns, so the application only connects to the library through loosely coupled interfaces. I don't think its a good idea to tell the application developers to implement HasActivityInjector and all the implementation details about library X just because the library uses dagger!

Also, I think this is a feature request! not a StackOverflow question!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/google/dagger/issues/1007#issuecomment-355696066, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEQqmj_e1x8p9KBQLkLKV0Vs1OWFBks5tHrS5gaJpZM4RMVY3 .

mohamad-amin commented 6 years ago

Yeah, I couldn't propose a better way for having dagger and a class that servers as an Application class for a library but creating a singleton like LibraryCore that servers the library like the Application class servers the application, and having a static reference to it. But the problem is that it took me about 1 hour to identify the problem and use something like

LibraryCore.getInstance().activityInjector().inject(this);

in my activity classes to solve the problem. I think this can be solved by creating a method like AndroidInjection.inject(Activity or sth else, HasActivityInjector) or by providing a document on how to use dagger in android libraries, cause it's better for developers to just use dagger and not be concerned about how to it uses the HasActivityInjector.

JakeWharton commented 6 years ago

So you're basically saying you want an overload which allows you specify an explicit HasInjector instance? And then the single-arg version would just be a convenience method for passing (HasInjector)getApplication() as that second argument.

That's pretty reasonable from an API standpoint. Design-wise, though, it undermines the point of using Dagger's Android library which is to decouple the Android components from knowing where their parent injector comes from.

On your LibraryCore class, you could have a static method inject(Activity) which called getClass() to get a Class<? extends Activity> key look up the corresponding ActivityInjector from the map and then called inject() on it passing in the instance (which is the exact same implementation of AndroidInjector). This ties your Android components directly to the parent injector, but that's fine because you actually don't want the separation that Dagger's Android module otherwise provides.

On Fri, Jan 5, 2018 at 7:11 PM Mohamad Amin Mohamadi < notifications@github.com> wrote:

Yeah, I couldn't propose a better way for having dagger and a class that servers as an Application class for a library but creating a singleton like LibraryCore that servers the library like the Application class servers the application, and having a static reference to it. But the problem is that it took me about 1 hour to identify the problem and use something like

LibraryCore.getInstance().activityInjector().inject(this);

in my activity classes to solve the problem. I think this can be solved by creating a method like AndroidInjection.inject(Activity or sth else, HasActivityInjector) or by providing a document on how to use dagger in android libraries, cause it's better for developers to just use dagger and not be concerned about how to it uses the HasActivityInjector.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/google/dagger/issues/1007#issuecomment-355699214, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEfCmZ7HRe8yWvbGh7tDu0hVYqxt7ks5tHro4gaJpZM4RMVY3 .

msamyatl commented 6 years ago

@mohamad-amin having the same issue here... how did you get around this in your library?

@JakeWharton I don't see how the additional API will undermine using Dagger Android... Currently, the AndroidInjection class goes up the hierarchy of Android components (fragment -> activity -> application) because it assumes that Application is at the top of this hierarchy, which is not true for library projects. Providing that API will not mean that the Android components will know where their parent injector comes from. It will just mean that a custom class that is not Application can be used to implement HasActivityInjector.

JakeWharton commented 6 years ago

Providing that API will not mean that the Android components will know where their parent injector comes from. It will just mean that a custom class that is not Application can be used to implement HasActivityInjector.

These are contradictory statements. How can you declare a custom class at the injection site and also not know about that custom class?

msamyatl commented 6 years ago

These are contradictory statements. How can you declare a custom class at the injection site and also not know about that custom class?

Oh sure the solution given above will mean the injection site will need to know about the custom class. But it doesn't need to be that way, for example instead of calling activity.getApplication() in AndroidInjection, maybe an alternative would be calling:

(HasActivityInjector) activity.getIntent().getSerializableExtra("DAGGER_ROOT");

I realise this is not pretty but I don't see another option for libraries.

JakeWharton commented 6 years ago

A component / owner of a component can't be serialized.

The only way that the existing system works is because of a defined hierarchy that's intrinsic to these components. If you need to circumvent that hierarchy I don't see how you can do it without hardcoding the relationships directly or reaching out to some other registry or sorts that knows how to locate the parent (the latter of which will be specific to your library and not general purpose).

RudolfHladik commented 5 years ago

So If I understand that, there is no way to use Dagger2 in android library project?

IonutNegru87 commented 5 years ago

I'm also interested in this. Any updates for this?