oblador / react-native-vector-icons

Customizable Icons for React Native with support for image source and full styling.
https://oblador.github.io/react-native-vector-icons/
MIT License
17.31k stars 2.12k forks source link

RN 0.72.3 with use_frameworks! fix and backward compatible #1538

Closed billnbell closed 8 months ago

billnbell commented 11 months ago

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

See below.

billnbell commented 11 months ago

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 commented 10 months ago

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 commented 10 months ago

@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 commented 10 months ago

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

johnf commented 10 months ago

@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 commented 10 months ago

OK it depends. above works on < 0.72 RN and above, and the PR https://github.com/oblador/react-native-vector-icons/pull/1530 works with 0.72+.

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

billnbell commented 10 months ago

I would go with above :)

lucaswitch commented 10 months ago

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

johnf commented 8 months ago

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

oblador commented 8 months ago

Should be fixed in 10.0.1