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

RN 0.72.3 with use_frameworks! fix and backward compatible #1538

Closed
billnbell opened this issue Aug 5, 2023 · 10 comments
Closed

RN 0.72.3 with use_frameworks! fix and backward compatible #1538

billnbell opened this issue Aug 5, 2023 · 10 comments

Comments

@billnbell
Copy link

Ok this fixes RN 0.72.3 for use_frameworks static and NEW ARCHITECTURE.

See below.

@billnbell
Copy link
Author

File patches/react-native-vector-icons+10.0.0.patch:

diff --git a/node_modules/react-native-vector-icons/RNVectorIcons.podspec b/node_modules/react-native-vector-icons/RNVectorIcons.podspec
index 92652fd..3e36e08 100644
--- a/node_modules/react-native-vector-icons/RNVectorIcons.podspec
+++ b/node_modules/react-native-vector-icons/RNVectorIcons.podspec
@@ -1,6 +1,8 @@
 require 'json'
 version = JSON.parse(File.read('package.json'))["version"]
 
+fabric_enabled = ENV['RCT_NEW_ARCH_ENABLED'] == '1'
+
 folly_compiler_flags = '-DFOLLY_NO_CONFIG -DFOLLY_MOBILE=1 -DFOLLY_USE_LIBCPP=1 -Wno-comma -Wno-shorten-64-to-32'
 
 Pod::Spec.new do |s|
@@ -11,15 +13,15 @@ Pod::Spec.new do |s|
   s.homepage       = "https://github.com/oblador/react-native-vector-icons"
   s.license        = "MIT"
   s.author         = { "Joel Arvidsson" => "joel@oblador.se" }
-  s.platforms      = { :ios => "9.0", :tvos => "9.0" }
+  s.platforms      = { :ios => "12.4", :tvos => "9.0" }
   s.source         = { :git => "https://github.com/oblador/react-native-vector-icons.git", :tag => "v#{s.version}" }
-  s.source_files   = 'RNVectorIconsManager/**/*.{h,m}'
+  s.source_files   = 'RNVectorIconsManager/**/*.{h,m,mm,swift}'
   s.resources      = "Fonts/*.ttf"
   s.preserve_paths = "**/*.js"
   s.dependency 'React-Core'
 
   # This guard prevent to install the dependencies when we run `pod install` in the old architecture.
-  if ENV['RCT_NEW_ARCH_ENABLED'] == '1' then
+  if fabric_enabled
       s.compiler_flags = folly_compiler_flags + " -DRCT_NEW_ARCH_ENABLED=1"
       s.pod_target_xcconfig    = {
           "HEADER_SEARCH_PATHS" => "\"$(PODS_ROOT)/boost\"",
@@ -31,6 +33,20 @@ Pod::Spec.new do |s|
       s.dependency "RCTRequired"
       s.dependency "RCTTypeSafety"
       s.dependency "ReactCommon/turbomodule/core"
+      s.dependency "React-utils"
+      s.subspec "xxxutils" do |ss|
+        ss.dependency "ReactCommon"
+        ss.dependency "React-utils"
+        ss.source_files         = "react/utils/**/*.{cpp,h}"
+        ss.header_dir           = "react/utils"
+        ss.pod_target_xcconfig  = { "HEADER_SEARCH_PATHS" => "\"${PODS_CONFIGURATION_BUILD_DIR}/React-utils/React_utils.framework/Headers\"" }
+      end
+
+      install_modules_dependencies(s)
+  else
+    s.pod_target_xcconfig = {
+      "CLANG_CXX_LANGUAGE_STANDARD" => "c++17"
+    }
   end
 
 end
diff --git a/node_modules/react-native-vector-icons/RNVectorIconsManager/RNVectorIconsManager.m b/node_modules/react-native-vector-icons/RNVectorIconsManager/RNVectorIconsManager.mm
similarity index 98%
rename from node_modules/react-native-vector-icons/RNVectorIconsManager/RNVectorIconsManager.m
rename to node_modules/react-native-vector-icons/RNVectorIconsManager/RNVectorIconsManager.mm
index 02a5f49..760819f 100644
--- a/node_modules/react-native-vector-icons/RNVectorIconsManager/RNVectorIconsManager.m
+++ b/node_modules/react-native-vector-icons/RNVectorIconsManager/RNVectorIconsManager.mm
@@ -173,7 +173,7 @@ - (NSString *)createGlyphImagePathForFont:(NSString *)fontName
 - (std::shared_ptr<facebook::react::TurboModule>)getTurboModule:
     (const facebook::react::ObjCTurboModule::InitParams &)params
 {
-    return std::make_shared<facebook::react::RNVectorIconsSpecJSI>(params);
+    return std::make_shared<facebook::react::NativeRNVectorIconsSpecJSI>(params);
 }
 #endif

@timothyerwin
Copy link

this is great thanks. however when I copy / pasted it didn't work, so I'm trying to manually create the changes with patch-package.

@johnf
Copy link
Collaborator

johnf commented Aug 15, 2023

@billnbell is this still required on top of #1530?

I've tested that and old and new arch work.
What changes would I make to the example app to test foruse_frameworks?

@billnbell
Copy link
Author

My patch works for both use_frameworks and without it. SO it fixes both.

@johnf
Copy link
Collaborator

johnf commented Aug 17, 2023

@billnbell just to confirm 100%, the PR #1530 is sufficient and none of the changes in the comment above are needed?

Great if so and I'll work on getting the PR merged

@billnbell
Copy link
Author

billnbell commented Aug 17, 2023

OK it depends. above works on < 0.72 RN and above, and the PR #1530 works with 0.72+.

If you want broader acceptance I can create a PR for the patch above ?

@billnbell
Copy link
Author

I would go with above :)

@lucaswitch
Copy link

This PR is important to make 0.72 and newer to work with new arch enabled.

@johnf
Copy link
Collaborator

johnf commented Oct 8, 2023

Can anyone experiencing this please test with #1550 and I'll work with @oblador to get it mrrged

@oblador
Copy link
Owner

oblador commented Oct 26, 2023

Should be fixed in 10.0.1

@oblador oblador closed this as completed Oct 26, 2023
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

5 participants