From 3dd6a83c0e39c54b4ff050589e4e13f1ccc66a5d Mon Sep 17 00:00:00 2001 From: Tommy Nguyen <4123478+tido64@users.noreply.github.com> Date: Mon, 13 Nov 2023 07:58:58 -0800 Subject: [PATCH] Fix `build_codegen!` not finding `@react-native/codegen` in pnpm setups (#41399) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: `build_codegen!` currently assumes that `react-native/codegen` gets installed next to `react-native`. In a pnpm setup, it's found under `/~/react-native/node_modules/react-native/codegen` instead. However, as dmytrorykun pointed out, we don't actually need to build it outside of this repository. ## Changelog: [GENERAL] [FIXED] - `react-native/codegen` shouldn't be built unless it's in the repo — fixes `pod install` failures in pnpm setups Pull Request resolved: https://github.com/facebook/react-native/pull/41399 Test Plan: We have a patched version of `react-native` working in a pnpm setup here: https://github.com/microsoft/rnx-kit/pull/2811 Reviewed By: dmytrorykun Differential Revision: D51201643 Pulled By: cipolleschi fbshipit-source-id: 53767ae08686a20f03b3b93abcbc7d5383083872 --- .../cocoapods/__tests__/codegen-test.rb | 32 ++++++++++--------- .../react-native/scripts/cocoapods/codegen.rb | 21 +++--------- 2 files changed, 22 insertions(+), 31 deletions(-) diff --git a/packages/react-native/scripts/cocoapods/__tests__/codegen-test.rb b/packages/react-native/scripts/cocoapods/__tests__/codegen-test.rb index 8d472aea862b70..6987399ad87f6d 100644 --- a/packages/react-native/scripts/cocoapods/__tests__/codegen-test.rb +++ b/packages/react-native/scripts/cocoapods/__tests__/codegen-test.rb @@ -68,7 +68,7 @@ def testCheckAndGenerateEmptyThirdPartyProvider_whenFileAlreadyExists_doNothing( assert_equal(Pod::Executable.executed_commands.length, 0) end - def testCheckAndGenerateEmptyThirdPartyProvider_whenHeaderMissingAndCodegenMissing_raiseError() + def testCheckAndGenerateEmptyThirdPartyProvider_whenHeaderMissingAndCodegenMissing_dontBuildCodegen() # Arrange FileMock.mocked_existing_files([ @@ -76,7 +76,7 @@ def testCheckAndGenerateEmptyThirdPartyProvider_whenHeaderMissingAndCodegenMissi ]) # Act - assert_raise { + assert_nothing_raised { checkAndGenerateEmptyThirdPartyProvider!(@prefix, false, dir_manager: DirMock, file_manager: FileMock) } @@ -84,16 +84,18 @@ def testCheckAndGenerateEmptyThirdPartyProvider_whenHeaderMissingAndCodegenMissi assert_equal(Pathname.pwd_invocation_count, 1) assert_equal(Pod::Config.instance.installation_root.relative_path_from_invocation_count, 1) assert_equal(FileMock.exist_invocation_params, [ - @base_path + "/" + @prefix + "/React/Fabric/" + @third_party_provider_header + @base_path + "/" + @prefix + "/React/Fabric/" + @third_party_provider_header, + @base_path + "/" + @prefix + "/React/Fabric/tmpSchemaList.txt", ]) assert_equal(DirMock.exist_invocation_params, [ @base_path + "/"+ @prefix + "/../react-native-codegen", - @base_path + "/"+ @prefix + "/../@react-native/codegen", ]) - assert_equal(Pod::UI.collected_messages, []) + assert_equal(Pod::UI.collected_messages, [ + "[Codegen] generating an empty RCTThirdPartyFabricComponentsProvider", + ]) assert_equal($collected_commands, []) - assert_equal(FileMock.open_files.length, 0) - assert_equal(Pod::Executable.executed_commands.length, 0) + assert_equal(FileMock.open_files.length, 1) + assert_equal(Pod::Executable.executed_commands.length, 1) end def testCheckAndGenerateEmptyThirdPartyProvider_whenImplementationMissingAndCodegenrepoExists_dontBuildCodegen() @@ -145,7 +147,7 @@ def testCheckAndGenerateEmptyThirdPartyProvider_whenImplementationMissingAndCode def testCheckAndGenerateEmptyThirdPartyProvider_whenBothMissing_buildCodegen() # Arrange - codegen_cli_path = @base_path + "/" + @prefix + "/../@react-native/codegen" + codegen_cli_path = @base_path + "/" + @prefix + "/../react-native-codegen" DirMock.mocked_existing_dirs([ codegen_cli_path, ]) @@ -160,15 +162,14 @@ def testCheckAndGenerateEmptyThirdPartyProvider_whenBothMissing_buildCodegen() @base_path + "/" + @prefix + "/React/Fabric/" + @tmp_schema_list_file ]) assert_equal(DirMock.exist_invocation_params, [ - @base_path + "/" + @prefix + "/../react-native-codegen", codegen_cli_path, codegen_cli_path + "/lib", ]) assert_equal(Pod::UI.collected_messages, [ - "[Codegen] building #{codegen_cli_path}.", + "[Codegen] building #{codegen_cli_path}", "[Codegen] generating an empty RCTThirdPartyFabricComponentsProvider" ]) - assert_equal($collected_commands, ["~/app/ios/../../../@react-native/codegen/scripts/oss/build.sh"]) + assert_equal($collected_commands, ["~/app/ios/../../../react-native-codegen/scripts/oss/build.sh"]) assert_equal(FileMock.open_files[0].collected_write, ["[]"]) assert_equal(FileMock.open_files[0].fsync_invocation_count, 1) assert_equal(Pod::Executable.executed_commands[0], { @@ -185,7 +186,7 @@ def testCheckAndGenerateEmptyThirdPartyProvider_whenBothMissing_buildCodegen() def testCheckAndGenerateEmptyThirdPartyProvider_withAbsoluteReactNativePath_buildCodegen() # Arrange rn_path = 'packages/react-native' - codegen_cli_path = rn_path + "/../@react-native/codegen" + codegen_cli_path = rn_path + "/../react-native-codegen" DirMock.mocked_existing_dirs([ @base_path + "/" + codegen_cli_path, ]) @@ -201,15 +202,16 @@ def testCheckAndGenerateEmptyThirdPartyProvider_withAbsoluteReactNativePath_buil @base_path + "/" + rn_path + "/React/Fabric/" + @tmp_schema_list_file ]) assert_equal(DirMock.exist_invocation_params, [ - @base_path + "/" + rn_path + "/../react-native-codegen", @base_path + "/" + codegen_cli_path, @base_path + "/" + codegen_cli_path + "/lib", ]) assert_equal(Pod::UI.collected_messages, [ - "[Codegen] building #{@base_path + "/" + codegen_cli_path}.", + "[Codegen] building #{@base_path + "/" + codegen_cli_path}", "[Codegen] generating an empty RCTThirdPartyFabricComponentsProvider" ]) - assert_equal($collected_commands, [@base_path + "/" + rn_path + "/../@react-native/codegen/scripts/oss/build.sh"]) + assert_equal($collected_commands, [ + @base_path + "/" + rn_path + "/../react-native-codegen/scripts/oss/build.sh", + ]) assert_equal(FileMock.open_files[0].collected_write, ["[]"]) assert_equal(FileMock.open_files[0].fsync_invocation_count, 1) assert_equal(Pod::Executable.executed_commands[0], { diff --git a/packages/react-native/scripts/cocoapods/codegen.rb b/packages/react-native/scripts/cocoapods/codegen.rb index 7c6af06e155223..7842f39f4226c5 100644 --- a/packages/react-native/scripts/cocoapods/codegen.rb +++ b/packages/react-native/scripts/cocoapods/codegen.rb @@ -11,23 +11,12 @@ # - dir_manager: a class that implements the `Dir` interface. Defaults to `Dir`, the Dependency can be injected for testing purposes. # @throws an error if it could not find the codegen folder. def build_codegen!(react_native_path, relative_installation_root, dir_manager: Dir) - codegen_repo_path = "#{basePath(react_native_path, relative_installation_root)}/../react-native-codegen"; - codegen_npm_path = "#{basePath(react_native_path, relative_installation_root)}/../@react-native/codegen"; - codegen_cli_path = "" + codegen_repo_path = "#{basePath(react_native_path, relative_installation_root)}/../react-native-codegen" + return unless dir_manager.exist?(codegen_repo_path) && !dir_manager.exist?("#{codegen_repo_path}/lib") - if dir_manager.exist?(codegen_repo_path) - codegen_cli_path = codegen_repo_path - elsif dir_manager.exist?(codegen_npm_path) - codegen_cli_path = codegen_npm_path - else - raise "[codegen] Could not find react-native-codegen." - end - - if !dir_manager.exist?("#{codegen_cli_path}/lib") - Pod::UI.puts "[Codegen] building #{codegen_cli_path}." - system("#{codegen_cli_path}/scripts/oss/build.sh") - end - end + Pod::UI.puts "[Codegen] building #{codegen_repo_path}" + system("#{codegen_repo_path}/scripts/oss/build.sh") +end # It generates an empty `ThirdPartyProvider`, required by Fabric to load the components #