stephanenicolas / toothpick

A scope tree based Dependency Injection (DI) library for Java / Kotlin / Android.
Apache License 2.0
1.12k stars 115 forks source link

Fixed Proguard rules for TP 3.x #376

Closed zawadz88 closed 5 years ago

zawadz88 commented 5 years ago

This is to fix issues #370 & #330 .

-adaptclassstrings was removed as it was breaking Firebase.

toothpick.ktp.delegate rules were added to work in Kotlin with by inject(), by lazy() etc.

I've also removed -keep @android.support.annotation.Keep class * as it probably shouldn't be there in the first place (that's my best guess though, I might be missing something).

Moreover, I've added a sample rule on how to custom scope rules in your own projects (the one at the bottom).

I've tested this set of rules in our production app with R8 (ProGuard rules fallback version) & Firebase Performance plugin.

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 95.618% when pulling e6421adc6cb9809793275b19890078b90b60f603 on zawadz88:bugfix/issue-370-fix-proguard-rules-for-tp3 into 11786a8042d3daefcb10e0dea87de95cef057d80 on stephanenicolas:master.

stephanenicolas commented 5 years ago

It.looks good to me. I got 2 questions though: why do we need a rules for the delegates ? And aren't we missing one to keep the @scope annotated custom scope annotations ?

zawadz88 commented 5 years ago

Hi @stephanenicolas,

why do we need a rules for the delegates?

We actually had crashes without this e.g.

Fatal Exception: toothpick.locators.NoFactoryFoundException: No factory could be found for class foo.bar.presentation.view.a. Check that the class has either a @Inject annotated constructor or contains @Inject annotated members.
       at toothpick.locators.FactoryLocator.getFactory + 37(FactoryLocator.java:37)
       at toothpick.ScopeImpl.lookupProvider + 359(ScopeImpl.java:359)
       at toothpick.ScopeImpl.getInstance + 93(ScopeImpl.java:93)
       at toothpick.ktp.delegate.EagerDelegate.onEntryPointInjected + 50(EagerDelegate.java:50)
       at toothpick.ktp.delegate.DelegateNotifier.notifyDelegates + 49(DelegateNotifier.java:49)
       at toothpick.ktp.KTP$1.inject + 37(KTP.java:37)
       at toothpick.Toothpick.inject + 213(Toothpick.java:213)
       at toothpick.ScopeImpl.inject + 140(ScopeImpl.java:140)
       ...

it seems it's the same reason why there are are rules for @Inject annotated properties and fields?

aren't we missing one to keep the @Scope annotated custom scope annotations ?

I doesn't seem to be needed at least in our app. We have our own @ActivityScope annotation and the only rule we needed to add was:

-keepnames @foo.bar.ActivityScope class *

the annotation we use is in Java:

@javax.inject.Scope
@Documented
@Retention(RUNTIME)
public @interface ActivityScope {
}
dlemures commented 5 years ago

great thx :)