-
Notifications
You must be signed in to change notification settings - Fork 7
No explicit weak-node-api linking on Android
#229
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Removes explicit weak-node-api linking on Android to prevent symbol injection crashes and switches to dynamic loading approach.
- Removes static linking of weak-node-api library from CMakeLists.txt
- Changes code to use namespaced function calls instead of global Node API functions
- Configures build system to include weak-node-api for dynamic loading
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/host/cpp/RuntimeNodeApi.cpp | Updates function call to use namespaced version |
| packages/host/android/build.gradle | Adds weak-node-api to jniLibs for dynamic loading |
| packages/host/android/CMakeLists.txt | Removes static linking, converts to interface library |
| .changeset/upset-papayas-pump.md | Documents the fix in changelog |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
packages/host/android/build.gradle
Outdated
| main { | ||
| manifest.srcFile "src/main/AndroidManifestNew.xml" | ||
| // Include the weak-node-api to enable a dynamic load | ||
| jniLibs.srcDirs = ["../weak-node-api/weak-node-api.android.node"] |
Copilot
AI
Sep 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The jniLibs.srcDirs assignment overwrites any existing source directories. Use jniLibs.srcDirs += [\"../weak-node-api/weak-node-api.android.node\"] to append to existing directories instead of replacing them.
| jniLibs.srcDirs = ["../weak-node-api/weak-node-api.android.node"] | |
| jniLibs.srcDirs += ["../weak-node-api/weak-node-api.android.node"] |
a1a5d5a to
e30e625
Compare
|
For some reason building fails on iOS even touched just a single line of common C++, which I strongly believe is unrelated. |
We're entering the "how did this ever work" territory.
Merging this PR will:
ferric-examplepackage in the test app - I have no clue why this passed onmain😬 since d367b6e merged (perhaps the package-lock.json acted as a cache somehow).This should fix the crash I've been experiencing on #225.