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

Support for RN New Architecture #436

Closed
Avadon opened this issue Sep 5, 2024 · 11 comments · Fixed by #483
Closed

Support for RN New Architecture #436

Avadon opened this issue Sep 5, 2024 · 11 comments · Fixed by #483
Labels
enhancement New feature or request
Milestone

Comments

@Avadon
Copy link

Avadon commented Sep 5, 2024

Upgrading to the latest RN version breaks Android runtime with an error Native part of Mapbox React Native libraries were not registered properly, double check our native installation guides. due to native module is not loading (even thought RN interoperability layer should keep legacy native modules working)

I assume the error happens somewhere here:
https://github.com/maplibre/maplibre-react-native/blob/main/javascript/components/MapView.tsx#L33

Steps to Trigger Behavior

  1. Update RN to 0.73+ (any version with bridgeless mode)
  2. run react-native run-android

Expected Behavior

A running Android app

Actual Behavior

An error Native part of Mapbox React Native libraries were not registered properly, double check our native installation guides.

Screenshots (if applicable)

https://i.ibb.co/hW2gSWS/Screenshot-2024-09-05-at-18-15-49.png

Version(s) affected

  • Platform: Android
  • OS version: Android 12
  • Device type: Google Pixel
  • Emulator/ Simulator: no
  • Development OS: OSX 13.5
  • maplibre-react-native Version: 9.1.0
  • MapLibre GL version [e.g. 6.3.0]
@tyrauber
Copy link
Collaborator

tyrauber commented Sep 5, 2024

@Avadon You are going to have better luck running 10.0.0 (10.0.0-alpha.10) with recent versions of react and react-native, as the 10.0.0 is using the latest native sdk libraries. The 9.0.0 codebase is quite old. Although, even 10.0.0 may not yet work with the new architecture. I wouldn't be surprised if that required some library changes.

@jhemmje
Copy link

jhemmje commented Oct 22, 2024

I get the same error message on Android and iOS, even with the newest version 10.0.0-alpha.22 and RN 0.74.5:

 ERROR  Native part of Mapbox React Native libraries were not registered properly, double check our native installation guides.
 ERROR  TypeError: Cannot read property 'StyleURL' of null, js engine: hermes

The error happens on the import.

Edit: Even with RN 0.72.17 I get these errors.

@ElSeniorMikael
Copy link

I also encounter this problem on Android with the version 10.0.0-alpha.22

"EarlyJsError: Cannot read property 'StyleURL' of null stack: anonymous@104461:35"

@KiwiKilian
Copy link
Collaborator

Could one of you create a reproduction repository?

@tyrauber
Copy link
Collaborator

tyrauber commented Oct 24, 2024

I'd suggest we branch the repo and update packages/expo-app to enable the new arch. That's probably the fastest way to see what breaks and debug it.

{
  "expo": {
    "plugins": [
      [
        "expo-build-properties",
        {
          "ios": {
            "newArchEnabled": true
          },
          "android": {
            "newArchEnabled": true
          }
        }
      ]
    ]
  }
}

@ElSeniorMikael
Copy link

@KiwiKilian

https://github.com/ElSeniorMikael/maplibre-android-error

There is my repo with the actual android problem i'm facing

@jhemmje
Copy link

jhemmje commented Nov 2, 2024

Got it working. :)

2 weeks ago I had the following error messages:

  1. Native part of Mapbox React Native libraries were not registered properly, double check our native installation guides.
  2. ERROR TypeError: Cannot read property 'StyleURL' of null, js engine: hermes

I fixed the first one by using the command npx expo run:android instead of using Expo Go. Please don't use the Expo Go app, because MapLibre uses native libraries, which is not supported by Expo Go.

The second was due to incompatible versions of various packages. I have fixed it by running npx expo install --check.

The command npx react-native doctor is also helpful for me. It checks the versions of Java, Gradle and some other tools.

And maybe you need also the command npx expo prebuild in order to prebuild the native libraries.

@thibaultcapelli
Copy link

thibaultcapelli commented Nov 4, 2024

I'm currently on the 10.0.0-alpha.23 version.

Got it working by:

  • Removing the RCT prefix in Android native modules

=> Is this working with the old arch ?

  • Avoiding spread operator on a native module

=> I guess changes are okay for the old arch

  • Changing code to get the UIManager

=> Need to make conditions to make it backwards compatible

Here is the diff:

diff --git a/android/rctmln/src/main/java/com/maplibre/rctmln/components/AbstractEventEmitter.java b/android/rctmln/src/main/java/com/maplibre/rctmln/components/AbstractEventEmitter.java
index 952c717754e6a80d03f9bbb61ebdbc2e3101a413..f59f32848c3a137591789d9c8be110edf56444d5 100644
--- a/android/rctmln/src/main/java/com/maplibre/rctmln/components/AbstractEventEmitter.java
+++ b/android/rctmln/src/main/java/com/maplibre/rctmln/components/AbstractEventEmitter.java
@@ -9,6 +9,9 @@ import com.facebook.react.uimanager.UIManagerModule;
 import com.facebook.react.uimanager.ViewGroupManager;
 import com.facebook.react.uimanager.events.EventDispatcher;
 import com.maplibre.rctmln.events.IEvent;
+import com.facebook.react.uimanager.UIManagerHelper;
+import com.facebook.react.fabric.FabricUIManager;
+import com.facebook.react.uimanager.common.UIManagerType;
 
 import java.util.HashMap;
 import java.util.Map;
@@ -45,7 +48,8 @@ abstract public class AbstractEventEmitter<T extends ViewGroup> extends ViewGrou
 
     @Override
     protected void addEventEmitters(ThemedReactContext context, @Nonnull T view) {
-        mEventDispatcher = context.getNativeModule(UIManagerModule.class).getEventDispatcher();
+        FabricUIManager uiManager = (FabricUIManager) UIManagerHelper.getUIManager(context, UIManagerType.FABRIC);
+        mEventDispatcher = uiManager.getEventDispatcher();
     }
 
     @Nullable
diff --git a/android/rctmln/src/main/java/com/maplibre/rctmln/modules/RCTMLNLocationModule.java b/android/rctmln/src/main/java/com/maplibre/rctmln/modules/RCTMLNLocationModule.java
index 1e47c4204afdf9d1a27c7499e1e7b273aaff4f2c..df753e27e5da3a99149440f7c6665af2d7ff5e27 100644
--- a/android/rctmln/src/main/java/com/maplibre/rctmln/modules/RCTMLNLocationModule.java
+++ b/android/rctmln/src/main/java/com/maplibre/rctmln/modules/RCTMLNLocationModule.java
@@ -18,7 +18,7 @@ import com.maplibre.rctmln.location.LocationManager;
 
 @ReactModule(name = RCTMLNLocationModule.REACT_CLASS)
 public class RCTMLNLocationModule extends ReactContextBaseJavaModule {
-    public static final String REACT_CLASS = "RCTMLNLocationModule";
+    public static final String REACT_CLASS = "MLNLocationModule";
     public static final String LOCATION_UPDATE = "MapboxUserLocationUpdate";
 
     private boolean isEnabled;
diff --git a/android/rctmln/src/main/java/com/maplibre/rctmln/modules/RCTMLNLogging.java b/android/rctmln/src/main/java/com/maplibre/rctmln/modules/RCTMLNLogging.java
index 7da5a3d2ed74d9b19bddd1686133c22684a951c8..2057ebb7e5e8687fdc62e7153dc35980d2d134eb 100644
--- a/android/rctmln/src/main/java/com/maplibre/rctmln/modules/RCTMLNLogging.java
+++ b/android/rctmln/src/main/java/com/maplibre/rctmln/modules/RCTMLNLogging.java
@@ -13,7 +13,7 @@ import android.util.Log;
 
 @ReactModule(name = RCTMLNLogging.REACT_CLASS)
 public class RCTMLNLogging extends ReactContextBaseJavaModule {
-    public static final String REACT_CLASS = "RCTMLNLogging";
+    public static final String REACT_CLASS = "MLNLogging";
     private ReactApplicationContext mReactContext;
 
     public RCTMLNLogging(ReactApplicationContext reactApplicationContext) {
diff --git a/android/rctmln/src/main/java/com/maplibre/rctmln/modules/RCTMLNModule.java b/android/rctmln/src/main/java/com/maplibre/rctmln/modules/RCTMLNModule.java
index 7fd6a2aedd4b2b1c3f3aac196d6a696933129ca3..5c55315b882baaf8da459b837514393e512a1e2b 100644
--- a/android/rctmln/src/main/java/com/maplibre/rctmln/modules/RCTMLNModule.java
+++ b/android/rctmln/src/main/java/com/maplibre/rctmln/modules/RCTMLNModule.java
@@ -36,7 +36,7 @@ import javax.annotation.Nullable;
 
 @ReactModule(name = RCTMLNModule.REACT_CLASS)
 public class RCTMLNModule extends ReactContextBaseJavaModule {
-    public static final String REACT_CLASS = "RCTMLNModule";
+    public static final String REACT_CLASS = "MLNModule";
 
     private static boolean customHeaderInterceptorAdded = false;
 
diff --git a/android/rctmln/src/main/java/com/maplibre/rctmln/modules/RCTMLNOfflineModule.java b/android/rctmln/src/main/java/com/maplibre/rctmln/modules/RCTMLNOfflineModule.java
index 1e0604d0706870f070fa3f53b37cccb1a92ea03b..3e73bd619bfdebcbf346bbf9fa888dffb3e70603 100644
--- a/android/rctmln/src/main/java/com/maplibre/rctmln/modules/RCTMLNOfflineModule.java
+++ b/android/rctmln/src/main/java/com/maplibre/rctmln/modules/RCTMLNOfflineModule.java
@@ -38,7 +38,7 @@ import java.util.Locale;
 
 @ReactModule(name = RCTMLNOfflineModule.REACT_CLASS)
 public class RCTMLNOfflineModule extends ReactContextBaseJavaModule {
-    public static final String REACT_CLASS = "RCTMLNOfflineModule";
+    public static final String REACT_CLASS = "MLNOfflineModule";
 
     public static final int INACTIVE_REGION_DOWNLOAD_STATE = OfflineRegion.STATE_INACTIVE;
     public static final int ACTIVE_REGION_DOWNLOAD_STATE = OfflineRegion.STATE_ACTIVE;
diff --git a/android/rctmln/src/main/java/com/maplibre/rctmln/modules/RCTMLNSnapshotModule.java b/android/rctmln/src/main/java/com/maplibre/rctmln/modules/RCTMLNSnapshotModule.java
index 22e774a43f8df09eab5bab079e5014789f78cfc1..35b0185dce225d6cfd934bcacc33d0089e74e7d8 100644
--- a/android/rctmln/src/main/java/com/maplibre/rctmln/modules/RCTMLNSnapshotModule.java
+++ b/android/rctmln/src/main/java/com/maplibre/rctmln/modules/RCTMLNSnapshotModule.java
@@ -40,7 +40,7 @@ import static android.content.Context.CONTEXT_IGNORE_SECURITY;
 
 @ReactModule(name = RCTMLNSnapshotModule.REACT_CLASS)
 public class RCTMLNSnapshotModule extends ReactContextBaseJavaModule {
-    public static final String REACT_CLASS = "RCTMLNSnapshotModule";
+    public static final String REACT_CLASS = "MLNSnapshotModule";
 
     private ReactApplicationContext mContext;
 
diff --git a/javascript/MLNModule.ts b/javascript/MLNModule.ts
index 78a28d9a1c151c71017bb176252ed8b8f413a783..6dc5a2c06a06085d033cd4c533025129c5e3daae 100644
--- a/javascript/MLNModule.ts
+++ b/javascript/MLNModule.ts
@@ -24,7 +24,7 @@ interface IMLNModule {
   setConnected(connected: boolean): void;
 }
 
-const MLNModule: IMLNModule = { ...NativeModules.MLNModule };
+const MLNModule: IMLNModule = Object.create(NativeModules.MLNModule);
 
 export const {
   StyleURL,
diff --git a/javascript/components/MapView.tsx b/javascript/components/MapView.tsx
index 850b490bff1458c1982f63e9cf2edc670005230c..19891d608e6941b9d0996e48eed1dc3392ca8692 100644
--- a/javascript/components/MapView.tsx
+++ b/javascript/components/MapView.tsx
@@ -31,6 +31,7 @@ import { FilterExpression } from "../utils/MaplibreStyles";
 import { getFilter } from "../utils/filterUtils";
 
 const MapLibreGL = NativeModules.MLNModule;
+
 if (MapLibreGL == null) {
   console.error(
     "Native module of @maplibre/maplibre-react-native library was not registered properly, please consult the docs: https://github.com/maplibre/maplibre-react-native",

I think I will use yarn patch for now to migrate to RN 0.76.

@KiwiKilian
Copy link
Collaborator

KiwiKilian commented Nov 4, 2024

Thanks, I also played around a bit and I think I was missing the object spread.

See also #481. There the UIManager was switched to:

mEventDispatcher = UIManagerHelper.getUIManager(context, UIManagerType.FABRIC).getEventDispatcher();

Migration is documented as:

if(BuildConfig.IS_NEW_ARCHITECTURE_ENABLED) {	
    UIManager uiManager = UIManagerHelper.getUIManager(this.reactContext, UIManagerType.FABRIC);
} else {
    UIManagerModule uiManager = this.reactContext.getNativeModule(UIManagerModule.class);
}

@tyrauber
Copy link
Collaborator

tyrauber commented Nov 4, 2024

This is awesome! Are additional changes to #481 required? It would be great if we could get a PR with these changes, and configure packages/expo to use the new architecture. I'd probably keep packages/react-native on the previous architecture so we can ensure backwards compatibility.

@KiwiKilian
Copy link
Collaborator

@tyrauber working on a proper PR, #481 has quite a bit of debugging in it. Was aiming for the exact setup you imagined.

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

Successfully merging a pull request may close this issue.

6 participants