Skip to content
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

fix: make Android example app runnable #23

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mdole
Copy link

@mdole mdole commented Mar 23, 2023

Tried to set up the Android example app and ran into a couple problems. First, the ./gradlew file was not an executable, fixed with a chmod +x (second commit in this PR). Second, the build would fail with errors like:

yarn run v1.22.19
$ react-native run-android
info JS server already running.
info Installing the app...

> Task :sift-react-native:compileDebugKotlin FAILED

Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

See https://docs.gradle.org/7.5.1/userguide/command_line_interface.html#sec:command_line_warnings
32 actionable tasks: 3 executed, 29 up-to-date
e: Incompatible classes were found in dependencies. Remove them from the classpath or use '-Xskip-metadata-version-check' to suppress errors
e: /Users/mattdole/.gradle/caches/transforms-3/1714374d0a1b6cc0c72f1d9e890ecb22/transformed/jetified-react-native-0.70.6-debug-api.jar!/META-INF/ReactAndroid_debug.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.6.0, expected version is 1.1.15.
e: /Users/mattdole/.gradle/caches/transforms-3/2cfe106a332c6a6f2245988cf5920bcb/transformed/jetified-kotlin-stdlib-common-1.6.10.jar!/META-INF/kotlin-stdlib-common.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.6.0, expected version is 1.1.15.

...many more lines of that

Specifying the Kotlin version fixes it. Credit to: react-native-webview/react-native-webview#2578

It's possible that this issue is specific to my setup, but I did have a colleague try the setup and he had the exact same issues. So maybe a fix is necessary!

Let me know if I missed things 🙂

Prior to this change, attempts to build the Android
example app failed with errors like 'Module was
compiled with an incompatible version of Kotlin.
The binary version of its metadata is 1.6.0, expected
version is 1.1.15.' See this thread for a similar
problem and the source of the solution:
react-native-webview/react-native-webview#2578
Prior to this change, the 'yarn example android' command would
fail because the gradlew file in the android example
app was not an executable.
@viaskal-sift
Copy link
Contributor

@mdole hi, we actually released a new version of the example recently, you may want to check that one out

@mdole
Copy link
Author

mdole commented Apr 11, 2023

Hi @viaskal-sift! I tried deleting and re-cloning the repo and had the same problem as before. That makes sense to me - assuming this is the change you mean (since that is the only PR merged since this one was created that I can see), the change was only to the test setup and would not affect running the app.

@viaskal-sift
Copy link
Contributor

@mdole okay, I see, worth double checking things. Thanks for raising this!

@mdole
Copy link
Author

mdole commented Apr 11, 2023

Thanks @viaskal-sift! While you're here: the Android part of the wrapper (not the example app, but the wrapper itself) also does not appear to be correctly sending events to Sift. I've flagged this with our rep, also sharing here in case this hasn't been reported to you yet. I'm happy to open an issue with it if that would be helpful.

I recorded a video demo showing the problem here (3 videos sounds like a lot, but it's 10 min total 😄):

Part 1
Part 2
Part 3

Feel free to reach out if I can provide more info or you have questions!

@viaskal-sift
Copy link
Contributor

@mdole yes, we received this information already and currently working on it so no need to open it here in Github. The problem looks a bit trickier than we originally expected, but our goal is to release the fix later this week (optimistic scenario) or early next week (realistic/pessimistic scenario).
And btw, while you're here: Thanks a lot for reporting this in so much convenient way! We really appreciate your effort to record the video with all necessary information. And we'll try to make you happy with the fix as soon as possible :)

@mdole
Copy link
Author

mdole commented Apr 11, 2023

Oh that's great to hear! Thanks for the update and for looking into it 🙂 and I'm glad to hear that my videos helped!

@viaskal-sift
Copy link
Contributor

@mdole sorry for being silent for some time. The problem was trickier than we originally thought, but we are about to provide you an example of the integration code that would help to address the problem of not having events for Android case. One thing I would like to double check - what is the navigation lib you're using in your app? Cause in the example we gonna provide we rely on react-navigation

@mdole
Copy link
Author

mdole commented May 3, 2023

Hey @viaskal-sift, all good! Yes, we use react-navigation

@viaskal-sift
Copy link
Contributor

Hi @mdole ! I am reaching you about the issue you mentioned here - #23 (comment). We've forwarded the answer some time ago but seems we didn't hear back for some time, so I decided to reach you directly here

Eventually, we can provide the workaround. We have a branch GitHub - SanoopC/sift-react-native at screen_navigation_example with the example of the integration code that should fix the problem. To reiterate - the problem we observed with missing events from Android flow in ReactNative was caused by the limitations of the events propagation between React-Native and Android. The proposed fix is not in the SDK code itself, but in the way we integrate the SDK into an app, thus, so no need to release a new version of the code for now

@mdole The instructions can be found on the branch’s README (Track Screen Navigation section) , so please try this one and let us know if it is working for you. Worth mentioning we've got a feedback from other customer that it was working for them so there is high chance it will address your problem. Looking forward to hearing back from your side!

@mdole
Copy link
Author

mdole commented Jul 13, 2023

Hey @viaskal-sift ! Thanks for touching base.

We've forwarded the answer some time ago but seems we didn't hear back for some time, so I decided to reach you directly here

Oh, how did you send it? I don't recall receiving anything! Sorry if I missed it.

The proposed fix is not in the SDK code itself, but in the way we integrate the SDK into an app, thus, so no need to release a new version of the code for now

We tried this out, and it works fine :) that said, it's still relying on manual calls rather than the queueing system the native Android SDK and iOS SDKs have, which automatically upload events after a certain number of events or a certain amount of time. So I hope that Sift will still eventually ship a "real" fix as part of the sift-react-native package, since this is how they provide access to their tools.

Thanks again for your work on this!

@viaskal-sift
Copy link
Contributor

viaskal-sift commented Jul 14, 2023

@mdole Thanks a lot for the quick answer!

Oh, how did you send it? I don't recall receiving anything! Sorry if I missed it.

I have an impression we were unfortunate enough to lose some communication steps between our engineering and your team since we communicated the workaround for some already

So I hope that Sift will still eventually ship a "real" fix as part of the sift-react-native package, since this is how they provide access to their tools.

We will try to find a better solution, but unfortunately we have no prospects at the moment of a quick fix since there are some limitations for events propagation between React Native and Android that look like rather a blocker to us at the moment :(

@mdole
Copy link
Author

mdole commented Jul 14, 2023

All good, thanks @viaskal-sift :) appreciate the info and the context!

Also, AFAIK the issue that I originally opened this PR to address (the Android example app setup not working out-of-the-box) is still happening - not a high priority or anything, but is this mergeable?

@viaskal-sift
Copy link
Contributor

but is this mergeable?

sure, I will push the team to verify this sooner, sorry for the delay

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants