voxeet / voxeet-uxkit-cordova

Dolby.io UXKit for Cordova
Other
1 stars 14 forks source link

[Bug] cordova-plugin-voxeet@1.4.7 build is failing on Android #49

Closed gabides closed 3 years ago

gabides commented 3 years ago

Describe the bug

Android build fails with version 1.4.7

Expected Behaviour

Android should build successfully

Steps to Reproduce the Problem

  1. Add cordova-plugin-voxeet@1.4.7 on a cordova project
  2. Build for android (Android Studio or CLI)
  3. Build fails with the following :
    > Task :capacitor-cordova-android-plugins:compileDebugJavaWithJavac
    android/capacitor-cordova-android-plugins/src/main/java/com/voxeet/toolkit/VoxeetCordova.java:890: error: cannot find symbol
        VoxeetToolkit.instance().getConferenceToolkit().minimize();
                                                       ^
    symbol:   method minimize()
    location: class ConferenceToolkitController
    android/capacitor-cordova-android-plugins/src/main/java/com/voxeet/toolkit/VoxeetCordova.java:895: error: cannot find symbol
        VoxeetToolkit.instance().getConferenceToolkit().maximize();
                                                       ^
    symbol:   method maximize()
    location: class ConferenceToolkitController
    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.
    2 errors

Looks like the cordova plugin was updated to 1.4.7 and the code relies on android-uxkit 3.0.4 new functions, but the dependencies still point to com.voxeet.sdk:uxkit:3.0.3-BETA3

codlab commented 3 years ago

A 1.5.0 release has been pushed to make it working properly. Note that it introduces a change (unrelated to the issue in this ticket) where you will need to adapt the variable injected inside your project

You will need to replace the VoxeetSDK (window.VoxeetSDK) to VoxeetCordova. This VoxeetCordova contains a VoxeetSDK class instance and UserInfo class definition

from there you can use all the method like join, leave etc... via VoxeetSDK.join(...), VoxeetSDK.leave(...) ...

weiz18 commented 3 years ago

Hey, @codlab , thanks 1.5.0 work great, we do however have any issue with the build on our CI pipeline. Do you know why cordova 10 is now required ? https://github.com/voxeet/voxeet-uxkit-cordova/compare/v1.4.8...v1.5.0#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R70 Thanks Wei

gabides commented 3 years ago

@codlab thanks for the fix above

FYI the issue we're having : when adding cordova-plugin-voxeet@1.5.0 to our project, npm install works but npm ci fails with the following :

installing NPM dependencies
npm at "/Users/runner/hostedtoolcache/node/14.15.5/x64/bin/npm"
/Users/runner/hostedtoolcache/node/14.15.5/x64/bin/npm ci
npm ERR! cordova not accessible from cordova-plugin-voxeet

Not sure cordova versions should be added to the plugin's dependencies, no other plugins are doing this

codlab commented 3 years ago

@gabides I tried to reproduce the issue using various projects but couldn't. Can you send us your package-lock.json file ? Various items online points toward the direction of out of sync lock file with the content of this file. (the fact that npm install is working is another indicator that it may easily the issue here). If you can provide me as well your npm version I would try to reproduce the issue using your environment as much as possible when it comes to the npm ci command

gabides commented 3 years ago

Hi @codlab , yes this issue goes away when we sync our package-lock.json. But adding cordova version to the dependencies is a hard requirement, any reason for this in the plugin's code ?

codlab commented 3 years ago

It will be removed in the new version - we published the day before yesterday the 3.1.0 version of the SDK and the new Cordova's plugin will reflect that as well as removing the Cordova dep

gabides commented 3 years ago

Ok great, thanks for the quick support

codlab commented 3 years ago

The 1.5.1 is available as we speak, it removes this. Can you test? It normally removes the issue (and the explicit dep on your own app)

FabienLavocat commented 3 years ago

This has been fixed in the latest versions of the plugin.