Skip to content

Update for React Native 0.78+ Compatibility: Enforce Stricter Kotlin Checks & Migrate to setViewTreeLifecycleOwner API#3816

Closed
ShahilMangroliya wants to merge 5 commits into
rnmapbox:mainfrom
ShahilMangroliya:main
Closed

Update for React Native 0.78+ Compatibility: Enforce Stricter Kotlin Checks & Migrate to setViewTreeLifecycleOwner API#3816
ShahilMangroliya wants to merge 5 commits into
rnmapbox:mainfrom
ShahilMangroliya:main

Conversation

@ShahilMangroliya

@ShahilMangroliya ShahilMangroliya commented Mar 28, 2025

Copy link
Copy Markdown
Contributor

Description

fix(android): support RN 0.78.0+

This PR addresses compatibility with React Native 0.78.0+ by enforcing stricter Kotlin checks, which are required for RN 0.78.0+. These changes ensure the module works reliably with the newer RN versions and prevent potential crashes due to Kotlin mismatch or type-safety issues.

Additionally, this PR updates the usage of ViewTreeLifecycleOwner.set(view, lifecycleOwner); to the newer and preferred API:
view.setViewTreeLifecycleOwner(lifecycleOwner).

Fixes #3815

Checklist

  • I've read CONTRIBUTING.md
  • I updated the doc/other generated code with running yarn generate in the root folder
  • I have tested the new feature on /example app.
    • In V11 mode/ios
    • In New Architecture mode/ios
    • In V11 mode/android
    • In New Architecture mode/android
  • I added/updated a sample - if a new feature was implemented (/example)

@mfazekas mfazekas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks much for the PR!, can you please add error logging ? So we don't just skip processing when it's null but we log some error. I think non of the changed place is expected to be null

@ShahilMangroliya

Copy link
Copy Markdown
Contributor Author

Thanks much for the PR!, can you please add error logging ? So we don't just skip processing when it's null but we log some error. I think non of the changed place is expected to be null

Added Error Logs

@mfazekas

mfazekas commented Apr 5, 2025

Copy link
Copy Markdown
Contributor

@ShahilMangroliya Can you update the description? Is this PR to prevent crashes? Or to enable building with 0.78.1, which has more strict kotlin checking I assume

@ShahilMangroliya

Copy link
Copy Markdown
Contributor Author

@ShahilMangroliya Can you update the description? Is this PR to prevent crashes? Or to enable building with 0.78.1, which has more strict kotlin checking I assume

Updated description

@ShahilMangroliya ShahilMangroliya changed the title fix(android): add null safety checks in various components to prevent… Update for React Native 0.78+ Compatibility: Enforce Stricter Kotlin Checks & Migrate to setViewTreeLifecycleOwner API Apr 11, 2025
@ShahilMangroliya

Copy link
Copy Markdown
Contributor Author

@ShahilMangroliya Can you update the description? Is this PR to prevent crashes? Or to enable building with 0.78.1, which has more strict kotlin checking I assume

Done

@shahzeb79

Copy link
Copy Markdown

Can we approve this please?

@msaqlain

Copy link
Copy Markdown

@mfazekas Much needed PR. Can we expedite the process, or how can I help?

import android.view.View
import com.facebook.react.bridge.*
import com.facebook.react.common.MapBuilder
//import com.facebook.react.common.MapBuilder

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls remove instead of commenting

ReadableType.Boolean -> Expression.literal(expressions.getBoolean(iExp))
ReadableType.Number -> Expression.literal(expressions.getDouble(iExp))
else -> Expression.literal(expressions.getString(iExp))
else -> expressions.getString(iExp)?.let { Expression.literal(it) }!!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a log here as well?

when (getType(i)) {
ReadableType.Map -> result.add(getMap(i).toJsonObject())
ReadableType.Array -> result.add(getArray(i).toJsonArray())
ReadableType.Map -> result.add(getMap(i)?.toJsonObject())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls log here as well

@mfazekas

Copy link
Copy Markdown
Contributor

@ShahilMangroliya pls look into ci failures as well

@phosimurg

Copy link
Copy Markdown

@mfazekas @ShahilMangroliya Guys, let's speed up the process please. Too many people are waiting for this.

@mfazekas

Copy link
Copy Markdown
Contributor

See: #3855 it's released as v10.1.39-rc.0

@mfazekas mfazekas closed this May 26, 2025
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.

fix(android): support RN 0.78.1

5 participants