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.

Sign up for GitHub

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

Fix use_react_native to support absolute paths #37545

Closed
wants to merge 5 commits into from
Closed

Fix use_react_native to support absolute paths #37545

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6b58c75
Fix use_react_native to support absolute paths
gabrieldonadel May 24, 2023
ae3c14e
Extract absolute path check into a function
gabrieldonadel May 24, 2023
c243149
Update codegen tests
gabrieldonadel May 24, 2023
4ffdb48
Fix rn test path for CI
gabrieldonadel May 24, 2023
7594dde
Use system APIs
gabrieldonadel Jun 7, 2023
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
41 changes: 41 additions & 0 deletions packages/react-native/scripts/cocoapods/__tests__/codegen-test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,47 @@ def testCheckAndGenerateEmptyThirdPartyProvider_whenBothMissing_buildCodegen()
})
end

def testCheckAndGenerateEmptyThirdPartyProvider_withAbsoluteReactNativePath_buildCodegen()
# Arrange
rn_path = '/Users/distiller/react-native/packages/react-native'
codegen_cli_path = rn_path + "/../@react-native/codegen"
DirMock.mocked_existing_dirs([
codegen_cli_path,
])

# Act
checkAndGenerateEmptyThirdPartyProvider!(rn_path, false, dir_manager: DirMock, file_manager: FileMock)

# Assert
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, [
rn_path + "/React/Fabric/" + @third_party_provider_header,
rn_path + "/React/Fabric/" + @tmp_schema_list_file
])
assert_equal(DirMock.exist_invocation_params, [
rn_path + "/../react-native-codegen",
codegen_cli_path,
codegen_cli_path + "/lib",
])
assert_equal(Pod::UI.collected_messages, [
"[Codegen] building #{codegen_cli_path}.",
"[Codegen] generating an empty RCTThirdPartyFabricComponentsProvider"
])
assert_equal($collected_commands, [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], {
"command" => "node",
"arguments" => [
rn_path + "/scripts/generate-provider-cli.js",
"--platform", 'ios',
"--schemaListPath", rn_path + "/React/Fabric/" + @tmp_schema_list_file,
"--outputDir", rn_path + "/React/Fabric"
]
})
end

# ================= #
# Test - RunCodegen #
# ================= #
Expand Down
14 changes: 11 additions & 3 deletions packages/react-native/scripts/cocoapods/codegen.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
# - 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 = "#{relative_installation_root}/#{react_native_path}/../react-native-codegen";
codegen_npm_path = "#{relative_installation_root}/#{react_native_path}/../@react-native/codegen";
codegen_repo_path = "#{basePath(react_native_path, relative_installation_root)}/../react-native-codegen";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following up with @NickGerleman suggestion, probably it is better to use some system APIs...

Suggested change
codegen_repo_path = "#{basePath(react_native_path, relative_installation_root)}/../react-native-codegen";
codegen_repo_path = File.join(basePath(react_native_path, relative_installation_root), "..", "react-native-codegen")

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing this seems to break some codegen tests, I'll try to investigate what's going on latter today

codegen_npm_path = "#{basePath(react_native_path, relative_installation_root)}/../@react-native/codegen";
codegen_cli_path = ""

if dir_manager.exist?(codegen_repo_path)
Expand Down Expand Up @@ -61,7 +61,7 @@ def checkAndGenerateEmptyThirdPartyProvider!(react_native_path, new_arch_enabled
Pod::Executable.execute_command(
'node',
[
"#{relative_installation_root}/#{react_native_path}/scripts/generate-provider-cli.js",
"#{basePath(react_native_path, relative_installation_root)}/scripts/generate-provider-cli.js",
"--platform", 'ios',
"--schemaListPath", temp_schema_list_path,
"--outputDir", "#{output_dir}"
Expand Down Expand Up @@ -108,3 +108,11 @@ def run_codegen!(
codegen_utils.generate_react_codegen_podspec!(react_codegen_spec, codegen_output_dir)
end
end

def basePath(react_native_path, relative_installation_root)
if react_native_path.start_with?("/")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if react_native_path.start_with?("/")
if Pathname.new(react_native_path).absolute

This suggestion also need a require 'pathname' after the license.
(See here for reference)

react_native_path
else
"#{relative_installation_root.to_s}/#{react_native_path}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"#{relative_installation_root.to_s}/#{react_native_path}"
File.join(relative_installation_root.to_s, react_native_path)"

end
end