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

React Native 0.75.1 android build issue #353

Closed
dembeEdward opened this issue Aug 16, 2024 · 12 comments
Closed

React Native 0.75.1 android build issue #353

dembeEdward opened this issue Aug 16, 2024 · 12 comments

Comments

@dembeEdward
Copy link
Contributor

dembeEdward commented Aug 16, 2024

Describe the bug
After upgrading to react-native 0.75.1 the following error is being thrown when building android due to the deprecation of JSIModulePackage.

> Task :react-native-mmkv-storage:compileReleaseJavaWithJavac FAILED
RNMMKVJSIModulePackage.java:3: error: cannot find symbol
import com.facebook.react.bridge.JSIModulePackage;
                                ^
  symbol:   class JSIModulePackage
  location: package com.facebook.react.bridge

Additional context
I would like to open a PR that resolves this issue. I patched the package on my current project and it is working as expected. Looking forward to hearing back from you

@DonBrowny
Copy link

Hey @dembeEdward i am facing the same issue with the latest RN version because of the deprecated JSIModulePackage.

@dembeEdward
Copy link
Contributor Author

dembeEdward commented Aug 16, 2024

@DonBrowny here is my patch, hope it helps you out:

patch name: react-native-mmkv-storage+0.10.2.patch

diff --git a/node_modules/react-native-mmkv-storage/android/build.gradle b/node_modules/react-native-mmkv-storage/android/build.gradle
index 4bd1d66..1b90ce4 100644
--- a/node_modules/react-native-mmkv-storage/android/build.gradle
+++ b/node_modules/react-native-mmkv-storage/android/build.gradle
@@ -29,6 +29,10 @@ def reactNativeArchitectures() {
   return value ? value.split(",") : ["armeabi-v7a", "x86", "x86_64", "arm64-v8a"]
 }
 
+def isNewArchitectureEnabled() {
+    return project.hasProperty("newArchEnabled") && project.newArchEnabled == "true"
+}
+
 def USE_HERMES = rootProject.ext.hermesEnabled
 
 repositories {
@@ -52,6 +56,7 @@ android {
 
   buildFeatures {
     prefab true
+    buildConfig true
   }
 
   
@@ -59,6 +64,7 @@ android {
   defaultConfig {
     minSdkVersion safeExtGet('minSdkVersion', 19)
     targetSdkVersion safeExtGet('targetSdkVersion', 31)
+    buildConfigField("boolean", "IS_NEW_ARCHITECTURE_ENABLED", isNewArchitectureEnabled().toString())
     var appProject = rootProject.allprojects.find {it.plugins.hasPlugin('com.android.application')}
     externalNativeBuild {
       cmake {
diff --git a/node_modules/react-native-mmkv-storage/android/build/generated/source/buildConfig/release/com/ammarahmed/mmkv/BuildConfig.java b/node_modules/react-native-mmkv-storage/android/build/generated/source/buildConfig/release/com/ammarahmed/mmkv/BuildConfig.java
new file mode 100644
index 0000000..a3813e5
--- /dev/null
+++ b/node_modules/react-native-mmkv-storage/android/build/generated/source/buildConfig/release/com/ammarahmed/mmkv/BuildConfig.java
@@ -0,0 +1,10 @@
+/**
+ * Automatically generated file. DO NOT MODIFY
+ */
+package com.ammarahmed.mmkv;
+
+public final class BuildConfig {
+  public static final boolean DEBUG = false;
+  public static final String LIBRARY_PACKAGE_NAME = "com.ammarahmed.mmkv";
+  public static final String BUILD_TYPE = "release";
+}
diff --git a/node_modules/react-native-mmkv-storage/android/src/main/java/com/ammarahmed/mmkv/RNMMKVJSIModulePackage.java b/node_modules/react-native-mmkv-storage/android/src/main/java/com/ammarahmed/mmkv/RNMMKVJSIModulePackage.java
deleted file mode 100644
index 1cbb97e..0000000
--- a/node_modules/react-native-mmkv-storage/android/src/main/java/com/ammarahmed/mmkv/RNMMKVJSIModulePackage.java
+++ /dev/null
@@ -1,15 +0,0 @@
-package com.ammarahmed.mmkv;
-
-import com.facebook.react.bridge.JSIModulePackage;
-import com.facebook.react.bridge.JSIModuleSpec;
-import com.facebook.react.bridge.JavaScriptContextHolder;
-import com.facebook.react.bridge.ReactApplicationContext;
-import java.util.Collections;
-import java.util.List;
-
-public class RNMMKVJSIModulePackage implements JSIModulePackage { 
-    @Override
-    public List<JSIModuleSpec> getJSIModules(ReactApplicationContext reactApplicationContext, JavaScriptContextHolder jsContext) {
-        return Collections.emptyList();
-    }
-}
\ No newline at end of file
diff --git a/node_modules/react-native-mmkv-storage/android/src/main/java/com/ammarahmed/mmkv/RNMMKVPackage.java b/node_modules/react-native-mmkv-storage/android/src/main/java/com/ammarahmed/mmkv/RNMMKVPackage.java
index e0b51d6..4f6481b 100644
--- a/node_modules/react-native-mmkv-storage/android/src/main/java/com/ammarahmed/mmkv/RNMMKVPackage.java
+++ b/node_modules/react-native-mmkv-storage/android/src/main/java/com/ammarahmed/mmkv/RNMMKVPackage.java
@@ -4,14 +4,59 @@ package com.ammarahmed.mmkv;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
+import java.util.HashMap;
+
+import androidx.annotation.Nullable;
 
 import com.facebook.react.ReactPackage;
 import com.facebook.react.bridge.NativeModule;
 import com.facebook.react.bridge.ReactApplicationContext;
 import com.facebook.react.uimanager.ViewManager;
 import com.facebook.react.bridge.JavaScriptModule;
+import com.facebook.react.TurboReactPackage;
+import com.facebook.react.module.annotations.ReactModule;
+import com.facebook.react.module.model.ReactModuleInfo;
+import com.facebook.react.module.model.ReactModuleInfoProvider;
+
+public class RNMMKVPackage extends TurboReactPackage implements ReactPackage {
+
+  @Nullable
+  @Override
+  public NativeModule getModule(String name, ReactApplicationContext reactContext) {
+    return new RNMMKVModule(reactContext);
+  }
+
+  @Override
+  public ReactModuleInfoProvider getReactModuleInfoProvider() {
+    Class<? extends NativeModule>[] moduleList = new Class[] {
+        RNMMKVModule.class
+    };
+
+    final Map<String, ReactModuleInfo> reactModuleInfoMap = new HashMap<>();
+    for (Class<? extends NativeModule> moduleClass : moduleList) {
+      ReactModule reactModule = moduleClass.getAnnotation(ReactModule.class);
+
+      reactModuleInfoMap.put(
+          reactModule.name(),
+          new ReactModuleInfo(
+              reactModule.name(),
+              moduleClass.getName(),
+              true,
+              reactModule.needsEagerInit(),
+              reactModule.hasConstants(),
+              reactModule.isCxxModule(),
+              BuildConfig.IS_NEW_ARCHITECTURE_ENABLED));
+    }
+
+    return new ReactModuleInfoProvider() {
+      @Override
+      public Map<String, ReactModuleInfo> getReactModuleInfos() {
+        return reactModuleInfoMap;
+      }
+    };
+  }
 
-public class RNMMKVPackage implements ReactPackage {
   @Override
   public List<NativeModule> createNativeModules(ReactApplicationContext reactContext) {
     return Arrays.<NativeModule>asList(new RNMMKVModule(reactContext));
diff --git a/node_modules/react-native-mmkv-storage/ios/MMKVNative.mm b/node_modules/react-native-mmkv-storage/ios/MMKVNative.mm
index 34a9449..c2d29be 100644
--- a/node_modules/react-native-mmkv-storage/ios/MMKVNative.mm
+++ b/node_modules/react-native-mmkv-storage/ios/MMKVNative.mm
@@ -7,7 +7,6 @@
 
 #import <React/RCTBridge+Private.h>
 #import <React/RCTUtils.h>
-#import <ReactCommon/RCTTurboModule.h>
 
 
 using namespace facebook;

@ammarahm-ed
Copy link
Owner

Hey @dembeEdward can you send a PR with the fix. I would be happy to merge it. Also does this break compatibility with other versions of react native?

@dembeEdward
Copy link
Contributor Author

dembeEdward commented Aug 16, 2024

Hey @ammarahm-ed I was thinking about that myself but I have not had the chance to test it against older RN versions. I can open the PR for now but suggest testing it with older versions first. If it does break then the patch could be a temporary solution for those who have upgraded. What do you think?

@DonBrowny
Copy link

@ammarahm-ed the was due to the RN removing the JSI modules in V-0.75 - https://reactnative.dev/blog/2024/08/12/release-0.75#android-jsimodule-has-been-deleted
And the suggested alternative to this is the turbo-modules https://github.com/reactwg/react-native-new-architecture/blob/main/docs/turbo-modules.md which @dembeEdward have implemented.
So this should not break the code for older version since turbo-modules were supported even in V0.74 (as far as i know)

@ammarahm-ed
Copy link
Owner

ammarahm-ed commented Aug 16, 2024

@dembeEdward I think we should update the library to the latest RN version. As @DonBrowny It should work for older version since turbo modules have been there for quite a while now.

Also are there any similar changes required for iOS?

@dembeEdward
Copy link
Contributor Author

@DonBrowny okay that sounds great! @ammarahm-ed I do have a branch ready to push to open the PR just need permission to do so

@ammarahm-ed
Copy link
Owner

@dembeEdward It seems you have cloned the main repository. You must first fork the repo, clone it, make the changes and then send a PR.

@dembeEdward
Copy link
Contributor Author

dembeEdward commented Aug 16, 2024

@ammarahm-ed sorry about that, I am done.

@DonBrowny
Copy link

@dembeEdward I think we should update the library to the latest RN version. As @DonBrowny It should work for older version since turbo modules have been there for quite a while now.

Also are there any similar changes required for iOS?

I didn't test this on IOS, but none of breaking changes in IOS API/Methods are used in this library, So IOS doesn't need any changes.

@dembeEdward
Copy link
Contributor Author

@dembeEdward I think we should update the library to the latest RN version. As @DonBrowny It should work for older version since turbo modules have been there for quite a while now.

Also are there any similar changes required for iOS?

I didn't test this on IOS, but none of breaking changes in IOS API/Methods are used in this library, So IOS doesn't need any changes.

I did have an issue when building iOS but removing import <ReactCommon/RCTTurboModule.h> solved it

@ammarahm-ed
Copy link
Owner

A new version is released, v0.10.3.

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

No branches or pull requests

3 participants