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 (facebook#35038)

Summary:
Pull Request resolved: facebook#35038

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: 409407a193a35cbd21b0e8778537b3627e4c54a2
  • Loading branch information
hramos authored and OlimpiaZurek committed May 22, 2023
1 parent 508dbcf commit c22e7e7
Show file tree
Hide file tree
Showing 30 changed files with 79 additions and 83 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
32 changes: 9 additions & 23 deletions packages/rn-tester/Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ PODS:
- React-callinvoker (1000.0.0)
- React-Codegen (1000.0.0):
- FBReactNativeSpec (= 1000.0.0)
- hermes-engine (= 1000.0.0)
- RCT-Folly (= 2021.07.22.00)
- RCTRequired (= 1000.0.0)
- RCTTypeSafety (= 1000.0.0)
Expand Down Expand Up @@ -615,26 +616,12 @@ PODS:
- RCT-Folly (= 2021.07.22.00)
- RCT-Folly/Futures (= 2021.07.22.00)
- React-cxxreact (= 1000.0.0)
- React-jsi (= 1000.0.0)
- React-jsidynamic (= 1000.0.0)
- React-jsiexecutor (= 1000.0.0)
- React-jsinspector (= 1000.0.0)
- React-perflogger (= 1000.0.0)
- React-jsi (1000.0.0):
- boost (= 1.76.0)
- DoubleConversion
- glog
- RCT-Folly (= 2021.07.22.00)
- React-jsi/Default (= 1000.0.0)
- React-jsi/Default (1000.0.0):
- boost (= 1.76.0)
- DoubleConversion
- glog
- RCT-Folly (= 2021.07.22.00)
- React-jsi/Fabric (1000.0.0):
- boost (= 1.76.0)
- DoubleConversion
- glog
- RCT-Folly (= 2021.07.22.00)
- hermes-engine
- React-jsidynamic (1000.0.0):
- boost (= 1.76.0)
- DoubleConversion
Expand Down Expand Up @@ -810,7 +797,6 @@ DEPENDENCIES:
- React-graphics (from `../../ReactCommon/react/renderer/graphics`)
- React-hermes (from `../../ReactCommon/hermes`)
- React-jsi (from `../../ReactCommon/jsi`)
- React-jsi/Fabric (from `../../ReactCommon/jsi`)
- React-jsidynamic (from `../../ReactCommon/jsi`)
- React-jsiexecutor (from `../../ReactCommon/jsiexecutor`)
- React-jsinspector (from `../../ReactCommon/jsinspector`)
Expand Down Expand Up @@ -959,7 +945,7 @@ SPEC CHECKSUMS:
FlipperKit: cbdee19bdd4e7f05472a66ce290f1b729ba3cb86
fmt: ff9d55029c625d3757ed641535fd4a75fedc7ce9
glog: 04b94705f318337d7ead9e6d17c019bd9b1f6b1b
hermes-engine: 041c211d43030d9aeff8faa388c019d19e752172
hermes-engine: 2c30267d0c2771edc2d369ac694d45f1278ab08b
libevent: 4049cae6c81cdb3654a443be001fb9bdceff7913
OpenSSL-Universal: ebc357f1e6bc71fa463ccb2fe676756aff50e88c
RCT-Folly: 0080d0a6ebf2577475bda044aa59e2ca1f909cda
Expand All @@ -968,22 +954,22 @@ SPEC CHECKSUMS:
React: 8d809d414723bb5763093ddec7658066a21ccabc
React-bridging: c8806159f8ef90f27443857eed1efdb8c85940e1
React-callinvoker: 5f16202ad4e45f0607b1fae0f6955a8f7c87eef1
React-Codegen: 5adf19af97eb37a7d441c040521191e446255086
React-Core: ce4282fb714ffbe444b84d296d1728eaee4d0e9f
React-Codegen: d2434d5e4d238bceef25f40c4f58b199eb981ad0
React-Core: 3965263aa4b4e1ebf7b4fdb50d2f49ce7bf28f63
React-CoreModules: 675170bccf156da3a3348e04e2036ce401b2010d
React-cxxreact: 7276467c246302fedf598cc40d7003896ddb20ba
React-Fabric: 141459e61c825acf02d26ece099acbd9cbd87b99
React-graphics: 5ccc9cc0d91794fd42bc1c693e9aea207554bbef
React-hermes: 7b9306aee1d58ba030614e54eb8041056119430f
React-jsi: c2a71fdc55642893768c517199fb8a71805159dc
React-hermes: 0a5145bae4207edf0def8e28fbcb6a8fd6e806c2
React-jsi: c24dbcfdf7ea075138b73372387c7f17c0db56ef
React-jsidynamic: 2b14ac1b6d3a1b7daa1e5a424b98de87da981698
React-jsiexecutor: 14e899380e3fe9ca74c4e19727540a03e7574721
React-jsinspector: 7733dd522d044aef87caa39f3eda77593358a7eb
React-logger: c7960346b021767ed90971aff592a44e3d69f8bb
React-perflogger: c4fdd48988c2d3047186fc1bc1772d634cfca2ea
React-RCTActionSheet: 166fd1df85ac10219466b45d12a5884d3eaceac1
React-RCTAnimation: d6127046c6bb44bd3e67b7503c4ad7f91131b58e
React-RCTAppDelegate: 475ca9b80e26c1c4aed93ce04363092fa78cf788
React-RCTAppDelegate: e427b692bf829e40f9b318e2a29dca2ab5f36cf6
React-RCTBlob: 68675c89ebe6edf310dddd0774ba07b685f090a9
React-RCTFabric: a98a6effece6719669b8c6b4d2c33fb0edddc613
React-RCTImage: 6de9f0f4402af859849e97cc73a56a52f400f4c9
Expand Down
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
Loading

0 comments on commit c22e7e7

Please sign in to comment.