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: SdkBridgeExt type mismatch when building on RN 0.77.0 #791

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

illestomas
Copy link

@illestomas illestomas commented Feb 3, 2025

What does this PR do?

Fixes the compilation error when running on RN 0.77.0

@datadog/mobile-react-native/android/src/main/kotlin/com/datadog/reactnative/DdSdkBridgeExt.kt:171:12 Return type mismatch: expected 'kotlin.collections.Map<kotlin.String, kotlin.Any>', actual 'java.util.HashMap<kotlin.String, kotlin.Any?>'. FAILURE: Build failed with an exception

Motivation

Fixes #787.
Triyng to update RN to 0.77.0 and this is the last missing piece to finalize the update. OFC i can patch it, and i probably will, just wanted to help you guys with a small piece of code. 🚀

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)
  • If this PR is auto-generated, please make sure also to manually update the code related to the change

@illestomas illestomas marked this pull request as ready for review February 3, 2025 22:58
@illestomas illestomas requested a review from a team as a code owner February 3, 2025 22:58
@illestomas illestomas changed the title fix: SdkBridgeExt type mismatch when running on RN 0.77.0 fix: SdkBridgeExt type mismatch when building on RN 0.77.0 Feb 3, 2025
@jonluca
Copy link

jonluca commented Feb 12, 2025

Can this be merged in?

@illestomas
Copy link
Author

Can this be merged in?

would be nice if someone from the engineering team could approve the workflow, @marco-saia-datadog maybe ?

@hirbod
Copy link

hirbod commented Feb 17, 2025

@marco-saia-datadog RN 0.78 is around the corner, and this is still breaking. This is unacceptably slow for paying customers. Y’all need to ramp up the speed and ensure your SDKs work as advertised for major RN updates—ideally when the beta is released, not 1-2 months after the stable version is out. Even now, with a PR, this still isn’t being handled with enough urgency.

@hirbod
Copy link

hirbod commented Feb 17, 2025

diff --git a/android/src/main/kotlin/com/datadog/reactnative/DdSdkBridgeExt.kt b/android/src/main/kotlin/com/datadog/reactnative/DdSdkBridgeExt.kt
index 09cec73fb785c5e6d033ab855d94b5ea0019984b..c2466d06a51dcb4c7e15096df5aef0cb04500d22 100644
--- a/android/src/main/kotlin/com/datadog/reactnative/DdSdkBridgeExt.kt
+++ b/android/src/main/kotlin/com/datadog/reactnative/DdSdkBridgeExt.kt
@@ -127,7 +127,7 @@ internal fun Map<*, *>.toWritableMap(
  * Recursively converts the [ReadableMap] to a [Map] which only contains Kotlin stdlib objects,
  * such as [List], [Map] and the raw types.
  */
-internal fun ReadableMap.toMap(): Map<String, Any> {
+internal fun ReadableMap.toMap(): HashMap<String, Any> {
     val map = this.toHashMap()
     val iterator = map.keys.iterator()
 
@@ -168,7 +168,14 @@ internal fun ReadableMap.toMap(): Map<String, Any> {
         }
     }
 
-    return map
+    val nonNullMap = HashMap<String, Any>()
+    map.entries.forEach { (key, value) ->
+        if (value != null) {
+            nonNullMap[key] = value
+        }
+    }
+
+    return nonNullMap
 }
 
 /**

Here’s a Bun patch for those paying a shitton of money to this company and still not getting a fucking PR resolved after three weeks.

@hirbod
Copy link

hirbod commented Feb 17, 2025

Imagine, they released a new SDK version JUST TODAY, but gave 0 fucks resolving this. LOLO

Screenshot 2025-02-17 at 21 54 12

@marco-saia-datadog
Copy link
Member

Hello 👋

We sincerely apologize for the delay in providing compatibility with the latest React Native versions. We understand how frustrating this has been.

The delay was primarily due to significant updates in our codebase, particularly around Session Replay, which introduced challenges in maintaining compatibility with React Native’s new architecture and RN 0.76. These changes required extensive testing to ensure stability and backward compatibility with the versions we support.

Recent internal organizational changes also contributed to significant slowdowns, but we are taking steps to improve our responsiveness moving forward.

Additionally, we’ve been actively improving our CI/CD processes to enhance the stability of our SDK and prevent similar delays in the future.

We acknowledge the frustration around the time it’s taking to support RN 0.77, and we are actively working on it. Ensuring compatibility with new versions remains a priority, and we are committed to improving our release cadence.

Regarding this PR, we want to assure you that it is being taken into account as we work on RN 0.77 support, which is now our top priority and will be released very soon. However, merging this PR alone is not sufficient for a full release, as additional work is required to ensure compatibility and stability. For this reason, we have opted to keep the recent release separate from the upcoming RN 0.77 update.

@hirbod
Copy link

hirbod commented Feb 18, 2025

@marco-saia-datadog thanks for your response, but this PR is a good bandaid to get things rolling. It might not fully satisfy QA or your processes, but it runs on 0.77 just fine (+ Fabric). I just shipped it to production.

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.

Does not compile on Android, RN 0.77
4 participants