Skip to content

Commit

Permalink
Do not build JSI in React-jsi when Hermes is enabled, resolve JSI ODR…
Browse files Browse the repository at this point in the history
… violation

Summary:
React-jsi provides JSI to allow React Native to interface with JavaScriptCore.
The hermes-engine Pod provides a second copy of JSI, as Hermes is built and linked statically with JSI.
This second copy of JSI would lead to an [ODR Violation](https://en.cppreference.com/w/cpp/language/definition).

To resolve this, when Hermes is enabled:
- React-hermes and hermes-engine are installed.
- React-jsc is not installed.
- React-jsi continues to be installed.
- React-jsi will not build JSI.
- React-jsi will declare a dependency on hermes-engine.

The result is that the JSI dependency for React Native is satisfied by hermes-engine, and there is no duplicate JSI library in the project.

When Hermes is disabled:
- React-jsi and React-jsc are installed.
- React-hermes and hermes-engine are not installed.
- React-jsi will build JSI.

Changelog:
[iOS][Changed] Resolve JSI ODR violation, make hermes-engine the JSI provider when Hermes is enabled

Reviewed By: cipolleschi

Differential Revision: D40334913

fbshipit-source-id: c108f258dfe9eefb5b755bc38dc63eea9f3a02b3
  • Loading branch information
hramos authored and facebook-github-bot committed Oct 20, 2022
1 parent c12558a commit a5f3728
Show file tree
Hide file tree
Showing 29 changed files with 70 additions and 60 deletions.
1 change: 0 additions & 1 deletion Libraries/Blob/React-RCTBlob.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ Pod::Spec.new do |s|
s.dependency "React-Codegen", version
s.dependency "ReactCommon/turbomodule/core", version
s.dependency "React-jsi", version
s.dependency "React-jsc", version
s.dependency "React-Core/RCTBlobHeaders", version
s.dependency "React-Core/RCTWebSocket", version
s.dependency "React-RCTNetwork", version
Expand Down
1 change: 0 additions & 1 deletion Libraries/Image/React-RCTImage.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ Pod::Spec.new do |s|
s.dependency "RCTTypeSafety", version
s.dependency "ReactCommon/turbomodule/core", version
s.dependency "React-jsi", version
s.dependency "React-jsc", version
s.dependency "React-Core/RCTImageHeaders", version
s.dependency "React-RCTNetwork", version
end
1 change: 0 additions & 1 deletion Libraries/LinkingIOS/React-RCTLinking.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,4 @@ Pod::Spec.new do |s|
s.dependency "React-Core/RCTLinkingHeaders", version
s.dependency "ReactCommon/turbomodule/core", version
s.dependency "React-jsi", version
s.dependency "React-jsc", version
end
1 change: 0 additions & 1 deletion Libraries/NativeAnimation/React-RCTAnimation.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,5 @@ Pod::Spec.new do |s|
s.dependency "RCTTypeSafety", version
s.dependency "ReactCommon/turbomodule/core", version
s.dependency "React-jsi", version
s.dependency "React-jsc", version
s.dependency "React-Core/RCTAnimationHeaders", version
end
1 change: 0 additions & 1 deletion Libraries/Network/React-RCTNetwork.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,5 @@ Pod::Spec.new do |s|
s.dependency "RCTTypeSafety", version
s.dependency "ReactCommon/turbomodule/core", version
s.dependency "React-jsi", version
s.dependency "React-jsc", version
s.dependency "React-Core/RCTNetworkHeaders", version
end
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/RCTPushNotificationHeaders", version
s.dependency "ReactCommon/turbomodule/core", version
s.dependency "React-jsi", version
s.dependency "React-jsc", version
end
1 change: 0 additions & 1 deletion Libraries/Settings/React-RCTSettings.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,5 @@ Pod::Spec.new do |s|
s.dependency "RCTTypeSafety", version
s.dependency "ReactCommon/turbomodule/core", version
s.dependency "React-jsi", version
s.dependency "React-jsc", version
s.dependency "React-Core/RCTSettingsHeaders", version
end
1 change: 0 additions & 1 deletion Libraries/Vibration/React-RCTVibration.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,5 @@ Pod::Spec.new do |s|
s.dependency "React-Codegen", version
s.dependency "ReactCommon/turbomodule/core", version
s.dependency "React-jsi", version
s.dependency "React-jsc", version
s.dependency "React-Core/RCTVibrationHeaders", version
end
1 change: 0 additions & 1 deletion React-Core.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ Pod::Spec.new do |s|
s.dependency "React-cxxreact", version
s.dependency "React-perflogger", version
s.dependency "React-jsi", version
s.dependency "React-jsc", version
s.dependency "React-jsiexecutor", version
s.dependency "Yoga"
s.dependency "glog"
Expand Down
1 change: 0 additions & 1 deletion React/CoreModules/React-CoreModules.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,4 @@ Pod::Spec.new do |s|
s.dependency "React-RCTImage", version
s.dependency "ReactCommon/turbomodule/core", version
s.dependency "React-jsi", version
s.dependency "React-jsc", version
end
1 change: 0 additions & 1 deletion React/FBReactNativeSpec/FBReactNativeSpec.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ Pod::Spec.new do |s|
s.dependency "RCTTypeSafety", version
s.dependency "React-Core", version
s.dependency "React-jsi", version
s.dependency "React-jsc", version
s.dependency "ReactCommon/turbomodule/core", version

use_react_native_codegen!(s, {
Expand Down
1 change: 0 additions & 1 deletion ReactCommon/React-Fabric.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ Pod::Spec.new do |s|
s.dependency "RCTTypeSafety", version
s.dependency "ReactCommon/turbomodule/core", version
s.dependency "React-jsi", version
s.dependency "React-jsc", version

s.subspec "animations" do |ss|
ss.dependency folly_dep_name, folly_version
Expand Down
1 change: 0 additions & 1 deletion ReactCommon/React-bridging.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,4 @@ Pod::Spec.new do |s|

s.dependency "RCT-Folly", folly_version
s.dependency "React-jsi", version
s.dependency "React-jsc", version
end
1 change: 0 additions & 1 deletion ReactCommon/ReactCommon.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ Pod::Spec.new do |s|
ss.dependency "React-Core", version
ss.dependency "React-cxxreact", version
ss.dependency "React-jsi", version
ss.dependency "React-jsc", version
ss.dependency "RCT-Folly", folly_version
s.dependency "React-logger", version
ss.dependency "DoubleConversion"
Expand Down
1 change: 0 additions & 1 deletion ReactCommon/cxxreact/React-cxxreact.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,5 @@ Pod::Spec.new do |s|
s.dependency "React-runtimeexecutor", version
s.dependency "React-perflogger", version
s.dependency "React-jsi", version
s.dependency "React-jsc", version
s.dependency "React-logger", version
end
2 changes: 1 addition & 1 deletion ReactCommon/hermes/React-hermes.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 0 additions & 1 deletion ReactCommon/jsi/React-jsc.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ Pod::Spec.new do |s|
s.exclude_files = "**/test/*"
s.framework = "JavaScriptCore"
s.dependency "React-jsi", version

s.default_subspec = "Default"

s.subspec "Default" do
Expand Down
27 changes: 19 additions & 8 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 @@ -25,12 +29,19 @@ Pod::Spec.new do |s|
s.author = "Facebook, Inc. and its affiliates"
s.platforms = { :ios => "12.4" }
s.source = source
s.source_files = "jsi/*.{cpp,h}"
s.exclude_files = [
"jsi/JSIDynamic.{h,cpp}",
"jsi/jsilib-posix.cpp",
"jsi/jsilib-windows.cpp",
"**/test/*"
]
s.header_dir = "jsi"

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
1 change: 0 additions & 1 deletion ReactCommon/jsi/React-jsidynamic.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,4 @@ Pod::Spec.new do |s|
s.dependency "RCT-Folly", folly_version
s.dependency "glog"
s.dependency "React-jsi", version
s.dependency "React-jsc", version
end
1 change: 0 additions & 1 deletion ReactCommon/runtimeexecutor/React-runtimeexecutor.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,4 @@ Pod::Spec.new do |s|
s.header_dir = "ReactCommon"

s.dependency "React-jsi", version
s.dependency "React-jsc", version
end
1 change: 0 additions & 1 deletion packages/rn-tester/RCTTest/React-RCTTest.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,4 @@ Pod::Spec.new do |s|
s.dependency "React-CoreModules", version
s.dependency "ReactCommon/turbomodule/core", version
s.dependency "React-jsi", version
s.dependency "React-jsc", version
end
40 changes: 21 additions & 19 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,7 +465,7 @@ def get_podspec_no_fabric_no_script
"RCTTypeSafety": ["99.98.97"],
"React-Core": ["99.98.97"],
"React-jsi": ["99.98.97"],
"React-jsc": ["99.98.97"],
"hermes-engine": ["99.98.97"],
"ReactCommon/turbomodule/core": ["99.98.97"]
}
}
Expand Down
6 changes: 2 additions & 4 deletions scripts/cocoapods/__tests__/jsengine-test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def test_setupHermes_whenHermesScriptSucceeds_installsPods
"returned by",
"prepare-hermes-for-build",
])
assert_equal($podInvocationCount, 5)
assert_equal($podInvocationCount, 4)
assert_equal($podInvocation["React-jsi"][:path], "../../ReactCommon/jsi")
assert_equal($podInvocation["React-hermes"][:path], "../../ReactCommon/hermes")
assert_equal($podInvocation["libevent"][:version], "~> 2.1.12")
Expand All @@ -113,10 +113,8 @@ def test_setupHermes_installsPods_installsFabricSubspecWhenFabricEnabled
setup_hermes!(:react_native_path => @react_native_path, :fabric_enabled => fabric_enabled)

# Assert
assert_equal($podInvocationCount, 6)
assert_equal($podInvocationCount, 4)
assert_equal($podInvocation["React-jsi"][:path], "../../ReactCommon/jsi")
assert_equal($podInvocation["React-jsc"][:path], "../../ReactCommon/jsi")
assert_equal($podInvocation["React-jsc/Fabric"][:path], "../../ReactCommon/jsi")
assert_equal($podInvocation["hermes-engine"][:podspec], "../../sdks/hermes/hermes-engine.podspec")
assert_equal($podInvocation["React-hermes"][:path], "../../ReactCommon/hermes")
assert_equal($podInvocation["libevent"][:version], "~> 2.1.12")
Expand Down
3 changes: 2 additions & 1 deletion scripts/cocoapods/__tests__/test_utils/CodegenUtilsMock.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def get_react_codegen_script_phases(
return @react_codegen_script_phases
end

def get_react_codegen_spec(package_json_file, folly_version: '2021.07.22.00', fabric_enabled: false, script_phases: nil)
def get_react_codegen_spec(package_json_file, folly_version: '2021.07.22.00', fabric_enabled: false, hermes_enabled: true, script_phases: nil)
@get_react_codegen_spec_params.push({
package_json_file: package_json_file,
folly_version: folly_version,
Expand All @@ -88,6 +88,7 @@ def use_react_native_codegen_discovery!(
app_path,
react_native_path: "../node_modules/react-native",
fabric_enabled: false,
hermes_enabled: true,
config_file_dir: '',
codegen_output_dir: 'build/generated/ios',
config_key: 'codegenConfig',
Expand Down
5 changes: 4 additions & 1 deletion scripts/cocoapods/codegen.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ def run_codegen!(
disable_codegen: false,
react_native_path: "../node_modules/react-native",
fabric_enabled: false,
hermes_enabled: true,
codegen_output_dir: 'build/generated/ios',
config_key: 'codegenConfig',
package_json_file: '~/app/package.json',
Expand All @@ -86,6 +87,7 @@ def run_codegen!(
app_path,
:react_native_path => react_native_path,
:fabric_enabled => fabric_enabled,
:hermes_enabled => hermes_enabled,
:config_file_dir => config_file_dir,
:codegen_output_dir => codegen_output_dir,
:config_key => config_key,
Expand All @@ -96,7 +98,8 @@ def run_codegen!(
# This gets generated in use_react_native_codegen_discovery when codegen discovery is enabled.
react_codegen_spec = codegen_utils.get_react_codegen_spec(
package_json_file,
:fabric_enabled => fabric_enabled
:fabric_enabled => fabric_enabled,
:hermes_enabled => hermes_enabled
)
codegen_utils.generate_react_codegen_podspec!(react_codegen_spec, codegen_output_dir)
end
Expand Down
Loading

0 comments on commit a5f3728

Please sign in to comment.