Skip to content

Commit

Permalink
Break Circular Dependency between React-Codegen and React-Fabric (#36210
Browse files Browse the repository at this point in the history
)

Summary:
Pull Request resolved: #36210

One of the circular dependencies we have in OSS was between React-Codegen and React-Fabric.

React-Codegen generates component which has to depends on React-Fabric because they need to use the files contained in the `react/renderer/view` folder.

React-Fabric contains some components that depends on RNCore, which was generated inside the React-Codegen folder.

This change generates the RNCore components inside the `ReactCommon/react/renderer/components/rncore` folder, breaking the dependency as `rncore` folder is now contained by React-Fabric itself.

**Fun Fact:** That's how it always should have been. There was already a line in the `.gitignore` to exclude the content of `ReactCommon/react/renderer/components/rncore` folder. I guess that with some of the refactoring/previous projects on Codegen, this requirements has slipped.

## Changelog:
[iOS][Breaking] -  generates RNCore components inside the ReactCommon folder and create a new pod for platform-specific ImageManager classes

Reviewed By: sammy-SC, dmytrorykun

Differential Revision: D43304641

fbshipit-source-id: ebb5033ce73dbcd03f880c3e204511fdce04b816
  • Loading branch information
cipolleschi authored and facebook-github-bot committed Feb 20, 2023
1 parent 6d34952 commit 5d175c6
Show file tree
Hide file tree
Showing 16 changed files with 130 additions and 38 deletions.
2 changes: 2 additions & 0 deletions React/React-RCTFabric.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ if ENV['USE_FRAMEWORKS']
header_search_paths << "\"${PODS_CONFIGURATION_BUILD_DIR}/React-Fabric/React_Fabric.framework/Headers/react/renderer/imagemanager/platform/ios\""
header_search_paths << "\"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers\""
header_search_paths << "\"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers/react/renderer/graphics/platform/ios\""
header_search_paths << "\"${PODS_CONFIGURATION_BUILD_DIR}/React-ImageManager/React_ImageManager.framework/Headers\""
end

Pod::Spec.new do |s|
Expand Down Expand Up @@ -66,6 +67,7 @@ Pod::Spec.new do |s|
s.dependency "React-Core", version
s.dependency "React-Fabric", version
s.dependency "React-RCTImage", version
s.dependency "React-ImageManager"
s.dependency "RCT-Folly/Fabric", folly_version

s.test_spec 'Tests' do |test_spec|
Expand Down
16 changes: 9 additions & 7 deletions ReactCommon/React-Fabric.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ Pod::Spec.new do |s|
header_search_path = [
"\"$(PODS_ROOT)/boost\"",
"\"$(PODS_TARGET_SRCROOT)/ReactCommon\"",
"\"$(PODS_ROOT)/RCT-Folly\""
"\"$(PODS_ROOT)/RCT-Folly\"",
]

if ENV['USE_FRAMEWORKS']
Expand Down Expand Up @@ -161,13 +161,19 @@ Pod::Spec.new do |s|
sss.header_dir = "react/renderer/components/modal"
end

ss.subspec "rncore" do |sss|
sss.dependency folly_dep_name, folly_version
sss.compiler_flags = folly_compiler_flags
sss.source_files = "react/renderer/components/rncore/**/*.{m,mm,cpp,h}"
sss.header_dir = "react/renderer/components/rncore"
end

ss.subspec "root" do |sss|
sss.dependency folly_dep_name, folly_version
sss.compiler_flags = folly_compiler_flags
sss.source_files = "react/renderer/components/root/**/*.{m,mm,cpp,h}"
sss.exclude_files = "react/renderer/components/root/tests"
sss.header_dir = "react/renderer/components/root"

end

ss.subspec "safeareaview" do |sss|
Expand Down Expand Up @@ -243,13 +249,9 @@ Pod::Spec.new do |s|
end

s.subspec "imagemanager" do |ss|
ss.dependency "React-RCTImage", version
ss.dependency folly_dep_name, folly_version
ss.compiler_flags = folly_compiler_flags
ss.source_files = "react/renderer/imagemanager/**/*.{m,mm,cpp,h}"
ss.exclude_files = "react/renderer/imagemanager/tests",
"react/renderer/imagemanager/platform/android",
"react/renderer/imagemanager/platform/cxx"
ss.source_files = "react/renderer/imagemanager/*.{m,mm,cpp,h}"
ss.header_dir = "react/renderer/imagemanager"
end

Expand Down
1 change: 1 addition & 0 deletions ReactCommon/React-rncore.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,6 @@ Pod::Spec.new do |s|
:js_srcs_dir => "#{react_native_path}/Libraries",
:library_name => "rncore",
:library_type => "components",
:output_dir => "#{react_native_path}/../ReactCommon"
})
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# Copyright (c) Meta Platforms, Inc. and affiliates.
#
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.

require "json"

package = JSON.parse(File.read(File.join(__dir__, "..", "..", "..", "..", "..", "..", "package.json")))
version = package['version']

source = { :git => 'https://github.com/facebook/react-native.git' }
if version == '1000.0.0'
# This is an unpublished version, use the latest commit hash of the react-native repo, which we’re presumably in.
source[:commit] = `git rev-parse HEAD`.strip if system("git rev-parse --git-dir > /dev/null 2>&1")
else
source[:tag] = "v#{version}"
end

folly_compiler_flags = '-DFOLLY_NO_CONFIG -DFOLLY_MOBILE=1 -DFOLLY_USE_LIBCPP=1 -Wno-comma -Wno-shorten-64-to-32'
boost_compiler_flags = '-Wno-documentation'

Pod::Spec.new do |s|
source_files = "**/*.{m,mm,cpp,h}"
header_search_paths = [
"\"$(PODS_ROOT)/boost\"",
"\"$(PODS_TARGET_SRCROOT)/../../../\"",
"\"$(PODS_TARGET_SRCROOT)\"",
"\"$(PODS_ROOT)/RCT-Folly\"",
]

s.name = "React-ImageManager"
s.version = version
s.summary = "Fabric for React Native."
s.homepage = "https://reactnative.dev/"
s.license = package["license"]
s.author = "Meta Platforms, Inc. and its affiliates"
s.platforms = { :ios => "12.4" }
s.source = source
s.compiler_flags = folly_compiler_flags + ' ' + boost_compiler_flags
s.source_files = source_files
s.header_dir = "react/renderer/imagemanager"

if ENV['USE_FRAMEWORKS']
s.module_name = "React_ImageManager"
s.header_mappings_dir = "./"
header_search_paths = header_search_paths + [
"\"${PODS_CONFIGURATION_BUILD_DIR}/React-Fabric/React_Fabric.framework/Headers\"",
"\"$(PODS_ROOT)/DoubleConversion\"",
"\"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers\"",
"\"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers/react/renderer/graphics/platform/ios\"",
]
end

s.pod_target_xcconfig = {
"USE_HEADERMAP" => "NO",
"HEADER_SEARCH_PATHS" => header_search_paths.join(" "),
"CLANG_CXX_LANGUAGE_STANDARD" => "c++17"
}

s.dependency "RCT-Folly/Fabric"
s.dependency "React-Fabric"
s.dependency "React-Core/Default"
s.dependency "React-RCTImage"
end
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

#include "ImageManager.h"
#import <react/renderer/imagemanager/ImageManager.h>

#import <React/RCTImageLoaderWithAttributionProtocol.h>
#import <React/RCTUtils.h>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

#include "ImageRequest.h"
#include <react/renderer/imagemanager/ImageRequest.h>

namespace facebook {
namespace react {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

#import <UIKit/UIKit.h>

#import <RCTImageManagerProtocol.h>
#import "RCTImageManagerProtocol.h"

NS_ASSUME_NONNULL_BEGIN

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

#import <UIKit/UIKit.h>

#import <RCTImageManagerProtocol.h>
#import "RCTImageManagerProtocol.h"

NS_ASSUME_NONNULL_BEGIN

Expand Down
2 changes: 1 addition & 1 deletion packages/rn-tester/Podfile
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ end
def pods(target_name, options = {}, use_flipper: !IN_CI && !USE_FRAMEWORKS)
project 'RNTesterPods.xcodeproj'

fabric_enabled = true
fabric_enabled = ENV['USE_FRAMEWORKS'] == 'dynamic' ? false : true

# Hermes is now enabled by default.
# The following line will only disable Hermes if the USE_HERMES envvar is SET to a value other than 1 (e.g. USE_HERMES=0).
Expand Down
36 changes: 28 additions & 8 deletions packages/rn-tester/Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ PODS:
- RCTRequired
- RCTTypeSafety
- React-Core
- React-Fabric
- React-graphics
- React-jsi
- React-jsiexecutor
Expand Down Expand Up @@ -398,6 +399,7 @@ PODS:
- React-Fabric/components/inputaccessory (= 1000.0.0)
- React-Fabric/components/legacyviewmanagerinterop (= 1000.0.0)
- React-Fabric/components/modal (= 1000.0.0)
- React-Fabric/components/rncore (= 1000.0.0)
- React-Fabric/components/root (= 1000.0.0)
- React-Fabric/components/safeareaview (= 1000.0.0)
- React-Fabric/components/scrollview (= 1000.0.0)
Expand Down Expand Up @@ -449,6 +451,14 @@ PODS:
- React-jsi (= 1000.0.0)
- React-jsiexecutor (= 1000.0.0)
- ReactCommon/turbomodule/core (= 1000.0.0)
- React-Fabric/components/rncore (1000.0.0):
- RCT-Folly/Fabric (= 2021.07.22.00)
- RCTRequired (= 1000.0.0)
- RCTTypeSafety (= 1000.0.0)
- React-graphics (= 1000.0.0)
- React-jsi (= 1000.0.0)
- React-jsiexecutor (= 1000.0.0)
- ReactCommon/turbomodule/core (= 1000.0.0)
- React-Fabric/components/root (1000.0.0):
- RCT-Folly/Fabric (= 2021.07.22.00)
- RCTRequired (= 1000.0.0)
Expand Down Expand Up @@ -545,7 +555,6 @@ PODS:
- React-graphics (= 1000.0.0)
- React-jsi (= 1000.0.0)
- React-jsiexecutor (= 1000.0.0)
- React-RCTImage (= 1000.0.0)
- ReactCommon/turbomodule/core (= 1000.0.0)
- React-Fabric/leakchecker (1000.0.0):
- RCT-Folly/Fabric (= 2021.07.22.00)
Expand Down Expand Up @@ -629,6 +638,7 @@ PODS:
- React-jsiexecutor (= 1000.0.0)
- ReactCommon/turbomodule/core (= 1000.0.0)
- React-graphics (1000.0.0):
- glog
- RCT-Folly/Fabric (= 2021.07.22.00)
- React-Core/Default (= 1000.0.0)
- React-hermes (1000.0.0):
Expand All @@ -642,6 +652,11 @@ PODS:
- React-jsiexecutor (= 1000.0.0)
- React-jsinspector (= 1000.0.0)
- React-perflogger (= 1000.0.0)
- React-ImageManager (1000.0.0):
- RCT-Folly/Fabric
- React-Core/Default
- React-Fabric
- React-RCTImage
- React-jsi (1000.0.0):
- boost (= 1.76.0)
- DoubleConversion
Expand Down Expand Up @@ -688,6 +703,7 @@ PODS:
- RCT-Folly/Fabric (= 2021.07.22.00)
- React-Core (= 1000.0.0)
- React-Fabric (= 1000.0.0)
- React-ImageManager
- React-RCTImage (= 1000.0.0)
- React-RCTImage (1000.0.0):
- RCT-Folly (= 2021.07.22.00)
Expand Down Expand Up @@ -825,6 +841,7 @@ DEPENDENCIES:
- React-Fabric (from `../../ReactCommon`)
- React-graphics (from `../../ReactCommon/react/renderer/graphics`)
- React-hermes (from `../../ReactCommon/hermes`)
- React-ImageManager (from `../../ReactCommon/react/renderer/imagemanager/platform/ios`)
- React-jsi (from `../../ReactCommon/jsi`)
- React-jsiexecutor (from `../../ReactCommon/jsiexecutor`)
- React-jsinspector (from `../../ReactCommon/jsinspector`)
Expand Down Expand Up @@ -904,6 +921,8 @@ EXTERNAL SOURCES:
:path: "../../ReactCommon/react/renderer/graphics"
React-hermes:
:path: "../../ReactCommon/hermes"
React-ImageManager:
:path: "../../ReactCommon/react/renderer/imagemanager/platform/ios"
React-jsi:
:path: "../../ReactCommon/jsi"
React-jsiexecutor:
Expand Down Expand Up @@ -975,13 +994,14 @@ SPEC CHECKSUMS:
RCTTypeSafety: a41e253b4ed644708899857d912b2f50c7b6214d
React: 2fc6c4c656cccd6753016528ad41199c16fd558e
React-callinvoker: a7d5e883a83bb9bd3985b08be832c5e76451d18f
React-Codegen: 4f1e911c128928e425e11698ad7859dfd0f92e20
React-Codegen: 1d5974f7b1384b458c5e38f1aad902d0bfce1c80
React-Core: 279a6e5ee79e88faa99157169b560c49635973d7
React-CoreModules: d3ee40954b381edc514301341e8b895febfc1848
React-cxxreact: aff243750dad852080636e615d7ae5639381735b
React-Fabric: 62b9929a7345f941d8833630f37d9440b2dda438
React-graphics: cb8a85648695c60f33a00d732b985f734d1470d8
React-Fabric: 6b5c30b6e60a85446cc5d3702fa262fd1fc15619
React-graphics: e70886fff4b79bec3745de761900a770029591f2
React-hermes: 7f0e87d44b1c7cfbdd11aa3c070d04435fe75d57
React-ImageManager: c22bb185bc1b1557071da61f929070ed56fb3859
React-jsi: e4c75a1cf727c8761908ac2eeb1084e47ba88a26
React-jsiexecutor: 8361f78286021782d885e0888bb059a4045c59b9
React-jsinspector: 9b56a373a6797114e1d89a7dffa98ee98af67a8f
Expand All @@ -991,7 +1011,7 @@ SPEC CHECKSUMS:
React-RCTAnimation: 6741f7be3e269e057c1426074cc70f34b56e114b
React-RCTAppDelegate: 0b3b2c1e02c02f952f5033535ddb23d690e3b890
React-RCTBlob: fd1ee93e48aa67b0183346a59754375de93de63d
React-RCTFabric: db1d7fe55db4811b63ae4060078e7048ebb4a918
React-RCTFabric: 533df4e11d99af97a78d4a400b4cab44925358e9
React-RCTImage: 055685a12c88939437f6520d9e7c120cd666cbf1
React-RCTLinking: b149b3ff1f96fa93fc445230b9c171adb0e5572c
React-RCTNetwork: 21abb4231182651f48b7035beaa011b1ab7ae8f4
Expand All @@ -1000,14 +1020,14 @@ SPEC CHECKSUMS:
React-RCTTest: 81ebfa8c2e1b0b482effe12485e6486dc0ff70d7
React-RCTText: 4e5ae05b778a0ed2b22b012af025da5e1a1c4e54
React-RCTVibration: ecfd04c1886a9c9a4e31a466c0fbcf6b36e92fde
React-rncore: 1235cadc4feaa607c9af12ca157b8ae991ade3a5
React-rncore: 3ef1d281e86300d2c8f97625f4a2fcea6602c5d5
React-runtimeexecutor: c7b2cd6babf6cc50340398bfbb7a9da13c93093f
ReactCommon: fdc30b91d89bfd2ed919c2cbccb460435f1f43f4
ScreenshotManager: 37152a3841a53f2de5c0013c58835b8738894553
ScreenshotManager: fb68e0677077569df974c9cbeaeb54f764d002ba
SocketRocket: fccef3f9c5cedea1353a9ef6ada904fde10d6608
Yoga: 1b1a12ff3d86a10565ea7cbe057d42f5e5fb2a07
YogaKit: f782866e155069a2cca2517aafea43200b01fd5a

PODFILE CHECKSUM: 920fb3b0e3c9dbdf8d86707f80cf0e7f2dc85c70
PODFILE CHECKSUM: 5f0460f3a7599f90e5d4759fdec7d7343fe7923d

COCOAPODS: 1.11.3
2 changes: 2 additions & 0 deletions scripts/cocoapods/__tests__/codegen_utils-test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,7 @@ def get_podspec_fabric_and_script_phases(script_phases)
specs[:dependencies].merge!({
'React-graphics': [],
'React-rncore': [],
'React-Fabric': [],
})

specs[:'script_phases'] = script_phases
Expand All @@ -570,6 +571,7 @@ def get_podspec_when_use_frameworks

specs[:dependencies].merge!({
'React-graphics': [],
'React-Fabric': [],
})

return specs
Expand Down
12 changes: 4 additions & 8 deletions scripts/cocoapods/__tests__/fabric-test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,21 +42,17 @@ def test_setupFabric_whenNewArchEnabled_installPods
setup_fabric!(:react_native_path => prefix, new_arch_enabled: true)

# Assert
check_installed_pods(prefix, install_rncore: false)
check_installed_pods(prefix)
end

def check_installed_pods(prefix, install_rncore: true)
assert_equal($podInvocationCount, install_rncore ? 5 : 4)
def check_installed_pods(prefix)
assert_equal($podInvocationCount, 5)

check_pod("React-Fabric", :path => "#{prefix}/ReactCommon")
check_pod("React-graphics", :path => "#{prefix}/ReactCommon/react/renderer/graphics")
check_pod("React-RCTFabric", :path => "#{prefix}/React", :modular_headers => true)
check_pod("RCT-Folly/Fabric", :podspec => "#{prefix}/third-party-podspecs/RCT-Folly.podspec")
if install_rncore
check_pod("React-rncore", :path => "#{prefix}/ReactCommon")
else
assert_nil($podInvocation["React-rncore"])
end
check_pod("React-ImageManager", :path => "#{prefix}/ReactCommon/react/renderer/imagemanager/platform/ios")
end

def check_pod(name, path: nil, modular_headers: nil, podspec: nil)
Expand Down
1 change: 1 addition & 0 deletions scripts/cocoapods/codegen_utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ def get_react_codegen_spec(package_json_file, folly_version: '2021.07.22.00', fa
if fabric_enabled
spec[:'dependencies'].merge!({
'React-graphics': [],
'React-Fabric': [],
});
end

Expand Down
2 changes: 1 addition & 1 deletion scripts/cocoapods/fabric.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ def setup_fabric!(react_native_path: "../node_modules/react-native", new_arch_en
pod 'React-Fabric', :path => "#{react_native_path}/ReactCommon"
pod 'React-graphics', :path => "#{react_native_path}/ReactCommon/react/renderer/graphics"
pod 'React-RCTFabric', :path => "#{react_native_path}/React", :modular_headers => true
pod 'React-ImageManager', :path => "#{react_native_path}/ReactCommon/react/renderer/imagemanager/platform/ios"
pod 'RCT-Folly/Fabric', :podspec => "#{react_native_path}/third-party-podspecs/RCT-Folly.podspec"


pod 'React-rncore', :path => "#{react_native_path}/ReactCommon" if !new_arch_enabled
end
Loading

0 comments on commit 5d175c6

Please sign in to comment.