-
Notifications
You must be signed in to change notification settings - Fork 90
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
plugin appears incompatible with phonegap-plugin-barcodescanner on Android (ZXing) #83
Comments
My initial reaction is that it would make the most sense for someone to fork a) Get rid of the local version of I don't think it makes sense for this plugin to try to fiddle with the Facebook SDK's internal dependencies, which could have unexpected results. Doing so would also require changing the way that the Facebook SDK is included to use |
Hi @noahcooper, thanks for the response and for creating and maintaining this plugin to begin with! I understand the reluctance to deal with this inside this plugin, makes sense. (and, I don't actually know why Facebook includes ZXing (the barcode scanner library for Android) in the first place). I'm an Android dev novice, I was hoping there might be a configuration trick to do this without forking either plugin, but sounds like there probably is not such a way. Excluding the ZXing library from the barcode scanner is probably the most practical solution in the long run, as the barcode scanner plugin isn't likely to change much (although it's a bit weird to be excluding the ZXing library from the barcode scanner plugin as it is the primary consumer :) ). This plugin likely needs to change more over time as Facebook Connect seems to need constant tending based on Facebook changes, so the fork I created of this plugin is going to create more maintenance/merge issues down the road for me. (I'm currently using that fork I created in production, it does seem to work ok). I may take a crack at forking the barcode scanner plugin at some point -- thanks for the suggested approach. There are some HTML5-based barcode scanning codes out there, which might be a more modern way of doing this in the first place, vs a plugin that hasn't been updated in years :) Thanks again! I'm going to close this issue based on your feedback. |
@viking2917 I did some analysis, and it looks like Facebook uses ZXing for only one thing -- device requests (i.e. when someone logs in from a device without a keyboard like a TV). Who knows if that will change though -- I wouldn't be surprised as they incorporate more features for Instagram and Messenger into the SDK if some new QR code scanning feature was added. Good point about the fact that an HTML5-based solution might be better for some use cases. FWIW - creating a fork of the barcode scanner plugin wouldn't require excluding ZXing from its dependencies. The problem is simply that it uses a different version of the library, and, that it uses hardcoded jar files (see https://github.com/phonegap/phonegap-plugin-barcodescanner/blob/master/src/android/barcodescanner-release-2.1.5.aar). If it instead used Maven, and allowed the version to be specified at runtime, it could then co-exist with plugins like this one. This is a common paradigm with other plugins, e.g. see how cordova-plugin-firebase-analytics allows developers to specify the version of Firebase Analytics to avoid the same kind of conflict. |
@noahcooper Oh that's super helpful. I did not understand that (exclude vs let Maven pick). When I get some cycles I'm going to try out the Maven approach. And a facepalm to me for forgetting about the device signin with QRcodes :). |
Hi! No fork or changes in the original plugins code needed :) |
Bug or feature request
[ X] I'm reporting a reproducible issue with the code
[ ] I'm reporting a feature request
Describe the Bug or feature request
cordova-plugin-facebook-connect appears to be incompatible with phonegap-plugin-barcodescanner on Android. Creating an ionic v1 project, and installing both plugins will result in a compile error of the form:
Expected Behavior
Ideally these plugins would co-exist. As with the precursor plugin, cordova-plugin-facebook4, they are incompatible as they both include the zxing libraries, which results in a duplicate class error. (see below).
Sample repo / To Reproduce
Create a sample ionic v1 project, e.g.:
ionic start myApp tabs --type ionic1
then:
and then build on Android, the error will occur.
Plugin version, OS, devices, etc
Problem occurs with latest version of plugin, and on Android only.
Additional Context
The issues with cordova-plugin-facebook4 are documented here: (phonegap/phonegap-plugin-barcodescanner#606, phonegap/phonegap-plugin-barcodescanner#535) and a workaround is described, involving creating a build instruction to not include the zxing libraries for the facebook plugin. I have created a fork of this plugin (cordova-plugin-facebook-connect) that implements these changes, and it compiles and works fine on both Android and iOS so far as I can tell, in case it is helpful to someone (https://github.com/viking2917/cordova-plugin-facebook-connect).
However I wonder if there is a way this can be accomplished using the official plugins without forking them?
The text was updated successfully, but these errors were encountered: