[native][draft] Calling Rust get_42() function from JS async
AbandonedPublic
Actions

Authored by varun on Mar 20 2023, 9:11 PM.

Details

Reviewers

• jon
ashoat

Summary

Example showing how to call Rust asynchronously from JS on native

Depends on D7113

Test Plan

Built and ran iOS simulator and Android emulator. As can be seen in
the JS code, we call await get42 twice in sequence, and after 10 seconds both
promises were resolved and 42 was returned, indicating that the functions didn't
block.

Diff Detail

Repository

rCOMM Comm

Branch

varun/poc3 (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

varun created this revision.Mar 20 2023, 9:11 PM

Herald added a reviewer: • jon. · View Herald TranscriptMar 20 2023, 9:11 PM

Herald added subscribers: atul, tomek, ashoat. · View Herald Transcript

i'm going to discard this diff, just putting it up for previous diff's test plan and for future reference

Harbormaster returned this revision to the author for changes because remote builds failed.Mar 20 2023, 9:48 PM

Harbormaster failed remote builds in B17452: Diff 23911!

Why did this diff fail the Android build?

ashoat mentioned this in D7113: [native] RustPromiseManager.Mar 21 2023, 7:00 AM

In D7114#211306, @ashoat wrote:

Why did this diff fail the Android build?

no idea... ran the exact same command as we do on CI and got the following:

BUILD SUCCESSFUL in 8m 54s
971 actionable tasks: 929 executed, 42 up-to-date
 varun  …  comm  native  android  ./gradlew clean && ./gradlew bundleRelease --no-daemon "-Dorg.gradle.jvmargs=-Xmx32g -XX:MaxPermSize=32g -XX:+HeapDumpOnOutOfMemoryError"

nvm figured it out, gonna put up the fix in a sec

I'm assuming this should be split into diffs:

Adding the async stuff (should get merged)
Testing the async stuff with pseudo code (should be used for testing only)

native/android/app/CMakeLists.txt
129 ↗	(On Diff #23911)	If it's just a single file, let's avoid GLOB
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
11 ↗	(On Diff #23911)	Assuming that we will have one and only one rust project being exported to cxx, this should be fine. Just want to make sure we won't have multiple of these in the future.
native/native_rust_library/RustCallback.cpp
3–5 ↗	(On Diff #23911)	CommonCpp should already be in the android header include path, and the xcode one might need to be added `$(SRCROOT)/../cpp/CommonCpp` Either way, I would like to get away from searching parent paths for the header paths.

update build.gradle to fix android

Harbormaster completed remote builds in B17601: Diff 24093.Mar 24 2023, 1:37 PM

varun requested review of this revision.Mar 24 2023, 1:37 PM

Is this still meant to be a dummy / example diff that doesn't get landed?

In D7114#212563, @jon wrote:

I'm assuming this should be split into diffs:

Adding the async stuff (should get merged)

Testing the async stuff with pseudo code (should be used for testing only)

Agree with this, it seems like some of this stuff needs to get landed but other parts don't

This revision now requires changes to proceed.Mar 24 2023, 2:00 PM

yeah fair i should just split this out into two diffs

i also missed jon's feedback from earlier so i'll address those comments now

varun abandoned this revision.Mar 27 2023, 9:41 PM