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

@Inject constructor plus scope annotation can't provide local singleton #358

Closed
Piasy opened this Issue Apr 11, 2016 · 9 comments

Comments

Projects
None yet
3 participants
@Piasy

Piasy commented Apr 11, 2016

I have a dependency class which has a @Inject constructor:

public class DemoDirectInjectDependency {
    private final Context mContext;

    @ActivityScope
    @Inject
    public DemoDirectInjectDependency(Context context) {
        mContext = context;
        Timber.d("new DemoDirectInjectDependency");
    }
}

Although I annotate it with @ActivityScope, it will be created multiple times when I inject it into multiple places.

The generated inject code looks like below:

    this.demoDirectInjectDependencyProvider =
        DemoDirectInjectDependency_Factory.create(provideContextProvider);

We can see the provider is not a ScopedProvider.

Full code could be found here.

Of course I could create a @Provide method in a @Module class, which could utilize the scope annotation. I just wonder could dagger2 utilize the scope annotation at constructor and create a ScopedProvider to provide this dependency?

@tbroyer

This comment has been minimized.

Show comment
Hide comment
@tbroyer

tbroyer Apr 11, 2016

Scope annotations should be on the class, not the constructor: https://docs.oracle.com/javaee/7/api/javax/inject/Scope.html

Maybe Dagger 2 could warn about it though.

tbroyer commented Apr 11, 2016

Scope annotations should be on the class, not the constructor: https://docs.oracle.com/javaee/7/api/javax/inject/Scope.html

Maybe Dagger 2 could warn about it though.

@Piasy

This comment has been minimized.

Show comment
Hide comment
@Piasy

Piasy Apr 11, 2016

@tbroyer Well, I mean the self defined @ActivityScope annotation, not @Scope.

Piasy commented Apr 11, 2016

@tbroyer Well, I mean the self defined @ActivityScope annotation, not @Scope.

@tbroyer

This comment has been minimized.

Show comment
Hide comment
@tbroyer

tbroyer Apr 11, 2016

@Piasy Of course. @Scope only applies to annotation types; I too meant your @ActivityScope annotation; the documentation of scope annotations is in the @Scope javadoc though, hence the link.

tbroyer commented Apr 11, 2016

@Piasy Of course. @Scope only applies to annotation types; I too meant your @ActivityScope annotation; the documentation of scope annotations is in the @Scope javadoc though, hence the link.

@Piasy

This comment has been minimized.

Show comment
Hide comment
@Piasy

Piasy Apr 11, 2016

Oh yes my mistake. But through my experiment project, I find out that my custom scope annotation must be on the @Provides method, otherwise (on the @Module class), the generated code won't use ScopedProvider to provide the dependency, hence no scoped singleton.

Piasy commented Apr 11, 2016

Oh yes my mistake. But through my experiment project, I find out that my custom scope annotation must be on the @Provides method, otherwise (on the @Module class), the generated code won't use ScopedProvider to provide the dependency, hence no scoped singleton.

@tbroyer

This comment has been minimized.

Show comment
Hide comment
@tbroyer

tbroyer Apr 11, 2016

In JSR330, scope annotations only appear on classes:

A scope annotation applies to a class containing an injectable constructor and governs how the injector reuses instances of the type.

But this is not enforced, to allow for other usage, such as configuration:

A scope annotation: […] may have restricted usage if annotated with @Target. While this specification covers applying scopes to classes only, some injector configurations might use scope annotations in other places (on factory method results for example).

Dagger uses scope annotations in configuration in 2 places: the @Component, and the @Provides methods. On an injectable class though, the annotation follows the JSR330 rule and must be on the class.

tbroyer commented Apr 11, 2016

In JSR330, scope annotations only appear on classes:

A scope annotation applies to a class containing an injectable constructor and governs how the injector reuses instances of the type.

But this is not enforced, to allow for other usage, such as configuration:

A scope annotation: […] may have restricted usage if annotated with @Target. While this specification covers applying scopes to classes only, some injector configurations might use scope annotations in other places (on factory method results for example).

Dagger uses scope annotations in configuration in 2 places: the @Component, and the @Provides methods. On an injectable class though, the annotation follows the JSR330 rule and must be on the class.

@Piasy

This comment has been minimized.

Show comment
Hide comment
@Piasy

Piasy Apr 11, 2016

So that means dagger2 won't support @Inject constructor plus scope annotation to provide 'local singleton'?

Piasy commented Apr 11, 2016

So that means dagger2 won't support @Inject constructor plus scope annotation to provide 'local singleton'?

@tbroyer

This comment has been minimized.

Show comment
Hide comment
@tbroyer

tbroyer Apr 11, 2016

All I'm saying is that you have to put your @ActivityScope on the DemoDirectInjectDependency class, not its constructor.

tbroyer commented Apr 11, 2016

All I'm saying is that you have to put your @ActivityScope on the DemoDirectInjectDependency class, not its constructor.

@Piasy

This comment has been minimized.

Show comment
Hide comment
@Piasy

Piasy Apr 11, 2016

Got it! When I put the @ActivityScope on the DemoDirectInjectDependency class, dagger2 use ScopedProvider to provide the DemoDirectInjectDependency instance.

Thanks!

Piasy commented Apr 11, 2016

Got it! When I put the @ActivityScope on the DemoDirectInjectDependency class, dagger2 use ScopedProvider to provide the DemoDirectInjectDependency instance.

Thanks!

@Piasy Piasy closed this Apr 11, 2016

@netdpb

This comment has been minimized.

Show comment
Hide comment
@netdpb

netdpb Apr 18, 2016

Member

An upcoming version of Dagger 2 will report a compile-time error if you put a scope annotation on an @Inject constructor.

Member

netdpb commented Apr 18, 2016

An upcoming version of Dagger 2 will report a compile-time error if you put a scope annotation on an @Inject constructor.

@netdpb netdpb self-assigned this Apr 18, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment