search

dpa99c / cordova-diagnostic-plugin

Cordova/Phonegap plugin to manage device settings
540 stars 361 forks source link

[FEATURE] AndroidX support #350

Closed EmeryGuillaume closed 5 years ago

EmeryGuillaume commented 5 years ago

I'm submitting a ... (check one with "x"):

Bug report

Current behavior:

When adding this plugin on a Cordova-based project that uses AndroidX, the build fails (see errors in the console output below)

Expected behavior:

The plugin should use the newer dependency in order to build with AndroidX based project. See: https://developer.android.com/jetpack/androidx/migrate

Steps to reproduce:

Create a clean Cordova Android project (using cordova-android@8.0.0 and cordova cli)

cordova create testapp
cd testapp
cordova platform add android
cordova plugin add cordova.plugins.diagnostic
cordova build android

So far, the build should succeed. Now, add these two lines in the gradle.properties file:

android.useAndroidX=true
android.enableJetifier=true

And build again --> the build fails.

Environment information

Console output

console output ``` ~/testapp/platforms/android/gradlew: Command failed with exit code 1 Error output: ~/testapp/platforms/android/app/src/main/java/cordova/plugins/Diagnostic_Notifications.java:35: error: cannot find symbol import android.support.v4.app.NotificationManagerCompat; ^ symbol: class NotificationManagerCompat location: package android.support.v4.app ~/testapp/platforms/android/app/src/main/java/cordova/plugins/Diagnostic.java:57: error: cannot find symbol import android.support.v4.app.ActivityCompat; ^ symbol: class ActivityCompat location: package android.support.v4.app ~/testapp/platforms/android/app/src/main/java/cordova/plugins/Diagnostic_External_Storage.java:28: error: cannot find symbol import android.support.v4.os.EnvironmentCompat; ^ symbol: class EnvironmentCompat location: package android.support.v4.os ~/testapp/platforms/android/app/src/main/java/cordova/plugins/Diagnostic_Notifications.java:123: error: cannot find symbol NotificationManagerCompat notificationManagerCompat = NotificationManagerCompat.from(this.cordova.getActivity().getApplicationContext()); ^ symbol: class NotificationManagerCompat location: class Diagnostic_Notifications ~/testapp/platforms/android/app/src/main/java/cordova/plugins/Diagnostic_Notifications.java:123: error: cannot find symbol NotificationManagerCompat notificationManagerCompat = NotificationManagerCompat.from(this.cordova.getActivity().getApplicationContext()); ^ symbol: variable NotificationManagerCompat location: class Diagnostic_Notifications ~/testapp/platforms/android/app/src/main/java/cordova/plugins/Diagnostic.java:639: error: cannot find symbol java.lang.reflect.Method method = ActivityCompat.class.getMethod("shouldShowRequestPermissionRationale", Activity.class, java.lang.String.class); ^ symbol: class ActivityCompat location: class Diagnostic ~/testapp/platforms/android/app/src/main/java/cordova/plugins/Diagnostic_External_Storage.java:216: error: cannot find symbol addPath = Environment.MEDIA_MOUNTED.equals(EnvironmentCompat.getStorageState(file)); ^ symbol: variable EnvironmentCompat location: class Diagnostic_External_Storage Note: Some input files use or override a deprecated API. Note: Recompile with -Xlint:deprecation for details. Note: Some input files use unchecked or unsafe operations. Note: Recompile with -Xlint:unchecked for details. 7 errors FAILURE: Build failed with an exception. ```


**Other information:** See related threads: https://github.com/apache/cordova/issues/69 https://github.com/apache/cordova-android/issues/565
dpa99c commented 5 years ago

I think that this would require careful consideration due to complexities of the Cordova Android ecosystem: in a native Android project you are in control of both the native source code and the Gradle config. However in a Cordova Android project, the plugin author is in control of the source code and the plugin user is in control of the project's Gradle config. Given that cordova@9 itself doesn't enable AndroidX by default, this plugin could be used either with or without AndroidX and it needs to support both use cases.

Changing this plugin's source code to use AndroidX class names would be a backwardly-incompatible breaking change, i.e. if the relevant class namespaces were changed to androidx in this plugin and the relevant Gradle config properties for were not set in the project, the build would also fail. So existing users of the plugin would have builds fail unless they enabled AndroidX and that could have knock on consequences for other plugins.

My thoughts are that this is not a trivial fix. A possible solution is to use a hook script along with a plugin variable at installation time to dynamically replace the classnames in the source code. That would enable the plugin to support projects both with and without AndroidX enabled.

Note: this is a feature request rather than a bug report since existing functionality works fine in projects where AndroidX is not optionally enabled.

dpa99c commented 5 years ago

I've implemented a solution for AndroidX but rather than doing it specifically in this plugin, I've created a generic solution using two new plugins:

This means that by installing both of these plugins in a Cordova project that already contains this Diagnostic plugin, the build will now succeed where it failed before due to the legacy Support Library name references.

For confirmation of this, try this test case:

cordova create test && cd test
cordova platform add android@8
cordova build android
    => build succeeds
cordova plugin add cordova.plugins.diagnostic
cordova build android
    => build succeeds
cordova plugin add cordova-plugin-androidx
cordova build android
    => build fails
cordova plugin add cordova-plugin-androidx-adapter
cordova build android
    => build succeeds
EmeryGuillaume commented 5 years ago

Hello @dpa99c, thanks, your solution is working :)

dpa99c commented 5 years ago

Great. I'll close this as there's now a viable solution.