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

Do not build JSI in React-jsi when Hermes is enabled, resolve JSI ODR violation #35038

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion Libraries/AppDelegate/React-RCTAppDelegate.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,22 @@ new_arch_enabled_flag="RCT_NEW_ARCH_ENABLED"
is_new_arch_enabled = ENV[new_arch_enabled_flag] == "1"
other_cflags = "$(inherited) -DRN_FABRIC_ENABLED " + folly_flags + (is_new_arch_enabled ? " -D"+"RCT_NEW_ARCH_ENABLED" : "")

use_hermes = ENV['USE_HERMES'] == '1'

header_search_paths = [
"$(PODS_TARGET_SRCROOT)/ReactCommon",
"$(PODS_ROOT)/Headers/Private/React-Core",
"$(PODS_ROOT)/boost",
"$(PODS_ROOT)/DoubleConversion",
"$(PODS_ROOT)/RCT-Folly",
"${PODS_ROOT}/Headers/Public/FlipperKit",
"$(PODS_ROOT)/Headers/Public/ReactCommon",
"$(PODS_ROOT)/Headers/Public/React-RCTFabric"
].concat(use_hermes ? [
"$(PODS_ROOT)/Headers/Public/React-hermes",
"$(PODS_ROOT)/Headers/Public/hermes-engine"
] : []).map{|p| "\"#{p}\""}.join(" ")

Pod::Spec.new do |s|
s.name = "React-RCTAppDelegate"
s.version = version
Expand All @@ -38,7 +54,7 @@ Pod::Spec.new do |s|
# This guard prevent to install the dependencies when we run `pod install` in the old architecture.
s.compiler_flags = other_cflags
s.pod_target_xcconfig = {
"HEADER_SEARCH_PATHS" => "\"$(PODS_TARGET_SRCROOT)/ReactCommon\" \"$(PODS_ROOT)/Headers/Private/React-Core\" \"$(PODS_ROOT)/boost\" \"$(PODS_ROOT)/DoubleConversion\" \"$(PODS_ROOT)/RCT-Folly\" \"${PODS_ROOT}/Headers/Public/React-hermes\" \"${PODS_ROOT}/Headers/Public/hermes-engine\" \"${PODS_ROOT}/Headers/Public/FlipperKit\" \"$(PODS_ROOT)/Headers/Public/ReactCommon\" \"$(PODS_ROOT)/Headers/Public/React-RCTFabric\"",
"HEADER_SEARCH_PATHS" => header_search_paths,
"OTHER_CPLUSPLUSFLAGS" => other_cflags,
"CLANG_CXX_LANGUAGE_STANDARD" => "c++17"
}
Expand Down
1 change: 0 additions & 1 deletion Libraries/Blob/React-RCTBlob.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,4 @@ Pod::Spec.new do |s|
s.dependency "React-Core/RCTBlobHeaders", version
s.dependency "React-Core/RCTWebSocket", version
s.dependency "React-RCTNetwork", version
s.dependency "React-jsi", version
end
28 changes: 22 additions & 6 deletions React-Core.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ folly_compiler_flags = '-DFOLLY_NO_CONFIG -DFOLLY_MOBILE=1 -DFOLLY_USE_LIBCPP=1
folly_version = '2021.07.22.00'
boost_compiler_flags = '-Wno-documentation'

use_hermes = ENV['USE_HERMES'] == '1'

header_subspecs = {
'CoreModulesHeaders' => 'React/CoreModules/**/*.h',
'RCTActionSheetHeaders' => 'Libraries/ActionSheetIOS/*.h',
Expand All @@ -34,6 +36,19 @@ header_subspecs = {
'RCTVibrationHeaders' => 'Libraries/Vibration/*.h',
}

header_search_paths = [
"$(PODS_TARGET_SRCROOT)/ReactCommon",
"$(PODS_ROOT)/boost",
"$(PODS_ROOT)/DoubleConversion",
"$(PODS_ROOT)/RCT-Folly",
"${PODS_ROOT}/Headers/Public/FlipperKit",
"$(PODS_ROOT)/Headers/Public/ReactCommon",
"$(PODS_ROOT)/Headers/Public/React-RCTFabric"
].concat(use_hermes ? [
"$(PODS_ROOT)/Headers/Public/React-hermes",
"$(PODS_ROOT)/Headers/Public/hermes-engine"
] : []).map{|p| "\"#{p}\""}.join(" ")

Pod::Spec.new do |s|
s.name = "React-Core"
s.version = version
Expand All @@ -48,12 +63,13 @@ Pod::Spec.new do |s|
s.header_dir = "React"
s.framework = "JavaScriptCore"
s.pod_target_xcconfig = {
"HEADER_SEARCH_PATHS" => "\"$(PODS_TARGET_SRCROOT)/ReactCommon\" \"$(PODS_ROOT)/boost\" \"$(PODS_ROOT)/DoubleConversion\" \"$(PODS_ROOT)/RCT-Folly\" \"${PODS_ROOT}/Headers/Public/React-hermes\" \"${PODS_ROOT}/Headers/Public/hermes-engine\" \"${PODS_ROOT}/Headers/Public/FlipperKit\" \"$(PODS_ROOT)/Headers/Public/ReactCommon\" \"$(PODS_ROOT)/Headers/Public/React-RCTFabric\"",
"FRAMEWORK_SEARCH_PATHS" => "\"${PODS_CONFIGURATION_BUILD_DIR}/React-hermes\"",
"DEFINES_MODULE" => "YES",
"GCC_PREPROCESSOR_DEFINITIONS" => "RCT_METRO_PORT=${RCT_METRO_PORT}",
"CLANG_CXX_LANGUAGE_STANDARD" => "c++17",
}
"HEADER_SEARCH_PATHS" => header_search_paths,
"DEFINES_MODULE" => "YES",
"GCC_PREPROCESSOR_DEFINITIONS" => "RCT_METRO_PORT=${RCT_METRO_PORT}",
"CLANG_CXX_LANGUAGE_STANDARD" => "c++17",
}.merge!(use_hermes ? {
"FRAMEWORK_SEARCH_PATHS" => "\"$(PODS_CONFIGURATION_BUILD_DIR)/React-hermes\""
} : {})
s.user_target_xcconfig = { "HEADER_SEARCH_PATHS" => "\"$(PODS_ROOT)/Headers/Private/React-Core\""}
s.default_subspec = "Default"

Expand Down
4 changes: 2 additions & 2 deletions ReactCommon/hermes/React-hermes.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ boost_compiler_flags = '-Wno-documentation'
Pod::Spec.new do |s|
s.name = "React-hermes"
s.version = version
s.summary = "-" # TODO
s.summary = "Hermes engine for React Native"
s.homepage = "https://reactnative.dev/"
s.license = package['license']
s.author = "Facebook, Inc. and its affiliates"
Expand All @@ -44,7 +44,7 @@ Pod::Spec.new do |s|
}.merge!(build_type == :debug ? { "GCC_PREPROCESSOR_DEFINITIONS" => "HERMES_ENABLE_DEBUGGER=1" } : {})
s.header_dir = "reacthermes"
s.dependency "React-cxxreact", version
s.dependency "React-jsi", version
s.dependency "React-jsidynamic", version
s.dependency "React-jsiexecutor", version
s.dependency "React-jsinspector", version
s.dependency "React-perflogger", version
Expand Down
41 changes: 41 additions & 0 deletions ReactCommon/jsi/React-jsc.podspec
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# 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

Pod::Spec.new do |s|
s.name = "React-jsc"
s.version = version
s.summary = "JavaScriptCore engine for React Native"
s.homepage = "https://reactnative.dev/"
s.license = package["license"]
s.author = "Facebook, Inc. and its affiliates"
s.platforms = { :ios => "12.4" }
s.source = source
s.source_files = "JSCRuntime.{cpp,h}"
s.exclude_files = "**/test/*"
s.framework = "JavaScriptCore"
s.dependency "React-jsi", version
s.default_subspec = "Default"

s.subspec "Default" do
# no-op
end

s.subspec "Fabric" do |ss|
ss.pod_target_xcconfig = { "OTHER_CFLAGS" => "$(inherited) -DRN_FABRIC_ENABLED" }
end
end
45 changes: 18 additions & 27 deletions ReactCommon/jsi/React-jsi.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@

require "json"

js_engine = ENV['USE_HERMES'] == "0" ?
:jsc :
:hermes

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

Expand All @@ -16,41 +20,28 @@ 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'
folly_version = '2021.07.22.00'
boost_compiler_flags = '-Wno-documentation'

Pod::Spec.new do |s|
s.name = "React-jsi"
s.version = version
s.summary = "-" # TODO
s.summary = "JavaScript Interface layer for React Native"
s.homepage = "https://reactnative.dev/"
s.license = package["license"]
s.author = "Facebook, Inc. and its affiliates"
s.platforms = { :ios => "12.4" }
s.source = source
s.source_files = "**/*.{cpp,h}"
s.exclude_files = [
"jsi/JSIDynamic.{h,cpp}",
"jsi/jsilib-*.{h,cpp}",
"**/test/*"
]
s.framework = "JavaScriptCore"
s.compiler_flags = folly_compiler_flags + ' ' + boost_compiler_flags
s.pod_target_xcconfig = { "HEADER_SEARCH_PATHS" => "\"$(PODS_ROOT)/boost\" \"$(PODS_ROOT)/RCT-Folly\" \"$(PODS_ROOT)/DoubleConversion\"" }
s.header_dir = "jsi"
s.default_subspec = "Default"

s.dependency "boost", "1.76.0"
s.dependency "DoubleConversion"
s.dependency "RCT-Folly", folly_version
s.dependency "glog"

s.subspec "Default" do
# no-op
end

s.subspec "Fabric" do |ss|
ss.pod_target_xcconfig = { "OTHER_CFLAGS" => "$(inherited) -DRN_FABRIC_ENABLED" }
if js_engine == :jsc
s.source_files = "**/*.{cpp,h}"
s.exclude_files = [
"jsi/JSIDynamic.{h,cpp}",
"jsi/jsilib-posix.cpp",
"jsi/jsilib-windows.cpp",
"**/test/*"
]
s.header_dir = "jsi"
elsif js_engine == :hermes
# JSI is provided by hermes-engine when Hermes is enabled
s.source_files = ""
s.dependency "hermes-engine"
end
end
39 changes: 21 additions & 18 deletions scripts/cocoapods/__tests__/codegen_utils-test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ def teardown
Dir.reset()
end

# ================================== #
# Test - GenerateReactCodegenPodspec #
# ================================== #
# ================================== #
# Test - GenerateReactCodegenPodspec #
# ================================== #

def testGenerateReactCodegenPodspec_whenItHasBeenAlreadyGenerated_doesNothing
# Arrange
Expand Down Expand Up @@ -80,18 +80,19 @@ def testGenerateReactCodegenPodspec_whenItHasNotBeenAlreadyGenerated_generatesIt
assert_true(CodegenUtils.react_codegen_podspec_generated)
end

# ========================== #
# Test - GetReactCodegenSpec #
# ========================== #
# ========================== #
# Test - GetReactCodegenSpec #
# ========================== #

def testGetReactCodegenSpec_whenFabricDisabledAndNoScriptPhases_generatesAPodspec
# Arrange
# Arrange
File.files_to_read('package.json' => '{ "version": "99.98.97"}')

# Act
# Act
podspec = CodegenUtils.new().get_react_codegen_spec(
'package.json',
:fabric_enabled => false,
:hermes_enabled => true,
:script_phases => nil
)

Expand All @@ -101,13 +102,14 @@ def testGetReactCodegenSpec_whenFabricDisabledAndNoScriptPhases_generatesAPodspe
end

def testGetReactCodegenSpec_whenFabricEnabledAndScriptPhases_generatesAPodspec
# Arrange
# Arrange
File.files_to_read('package.json' => '{ "version": "99.98.97"}')

# Act
podspec = CodegenUtils.new().get_react_codegen_spec(
'package.json',
:fabric_enabled => true,
:hermes_enabled => true,
:script_phases => "echo Test Script Phase"
)

Expand All @@ -117,11 +119,11 @@ def testGetReactCodegenSpec_whenFabricEnabledAndScriptPhases_generatesAPodspec
end

# =============================== #
# Test - GetCodegenConfigFromFile #
# =============================== #
# Test - GetCodegenConfigFromFile #
# =============================== #

def testGetCodegenConfigFromFile_whenFileDoesNotExists_returnEmpty
# Arrange
# Arrange

# Act
codegen = CodegenUtils.new().get_codegen_config_from_file('package.json', 'codegenConfig')
Expand All @@ -131,7 +133,7 @@ def testGetCodegenConfigFromFile_whenFileDoesNotExists_returnEmpty
end

def testGetCodegenConfigFromFile_whenFileExistsButHasNoKey_returnEmpty
# Arrange
# Arrange
File.mocked_existing_files(['package.json'])
File.files_to_read('package.json' => '{ "codegenConfig": {}}')

Expand All @@ -143,7 +145,7 @@ def testGetCodegenConfigFromFile_whenFileExistsButHasNoKey_returnEmpty
end

def testGetCodegenConfigFromFile_whenFileExistsAndHasKey_returnObject
# Arrange
# Arrange
File.mocked_existing_files(['package.json'])
File.files_to_read('package.json' => '{ "codegenConfig": {"name": "MySpec"}}')

Expand Down Expand Up @@ -217,7 +219,7 @@ def testGetListOfJSSpecs_whenDoesNotUsesLibraries_returnAListOfFiles

# ================================== #
# Test - GetReactCodegenScriptPhases #
# ================================== #
# ================================== #

def testGetReactCodegenScriptPhases_whenAppPathNotDefined_abort
# Arrange
Expand Down Expand Up @@ -370,9 +372,9 @@ def testUseReactCodegenDiscovery_whenParametersAreGood_executeCodegen
])
end

# ============================= #
# Test - CleanUpCodegenFolder #
# ============================= #
# ============================= #
# Test - CleanUpCodegenFolder #
# ============================= #

def testCleanUpCodegenFolder_whenCleanupDone_doNothing
# Arrange
Expand Down Expand Up @@ -463,6 +465,7 @@ def get_podspec_no_fabric_no_script
"RCTTypeSafety": ["99.98.97"],
"React-Core": ["99.98.97"],
"React-jsi": ["99.98.97"],
"hermes-engine": ["99.98.97"],
"ReactCommon/turbomodule/core": ["99.98.97"]
}
}
Expand Down
14 changes: 10 additions & 4 deletions scripts/cocoapods/__tests__/fabric-test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

require "test/unit"
require_relative "../fabric.rb"
require_relative "../utils.rb"
require_relative "./test_utils/podSpy.rb"

class FabricTest < Test::Unit::TestCase
Expand All @@ -13,24 +14,30 @@ def setup
podSpy_cleanUp()
end

def teardown
podSpy_cleanUp()
end

# ================== #
# TEST - setupFabric #
# ================== #
def test_setupFabric_installsPods
# Arrange
prefix = "../.."

# Act
setup_fabric!(prefix)
setup_fabric!(:react_native_path => prefix)

# Assert
check_installed_pods(prefix)
end

def check_installed_pods(prefix)
assert_equal($podInvocationCount, 6)
assert_equal($podInvocationCount, 5)

check_pod("React-Fabric", :path => "#{prefix}/ReactCommon")
check_pod("React-rncore", :path => "#{prefix}/ReactCommon")
check_pod("React-graphics", :path => "#{prefix}/ReactCommon/react/renderer/graphics")
check_pod("React-jsi/Fabric", :path => "#{prefix}/ReactCommon/jsi")
check_pod("React-RCTFabric", :path => "#{prefix}/React", :modular_headers => true)
check_pod("RCT-Folly/Fabric", :podspec => "#{prefix}/third-party-podspecs/RCT-Folly.podspec")
end
Expand All @@ -45,5 +52,4 @@ def check_pod(name, path: nil, modular_headers: nil, podspec: nil)

assert_equal(params, expected_params)
end

end
Loading