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

Jump to bottom

[template] Use $REACT_NATIVE_PATH instead of relative path #41968

Closed
wants to merge 1 commit into from
Closed

[template] Use $REACT_NATIVE_PATH instead of relative path #41968

wants to merge 1 commit into from

Conversation

kraenhansen
Copy link
Contributor

@kraenhansen kraenhansen commented Dec 17, 2023 •
edited
Loading

Summary:

I believe it's valuable to be able to initialise the React Native template into a mono-repo and have it work with zero updates to the configuration.
In its current form the template's Xcode project makes assumptions on the relative location of the react-native package, while it could instead use the REACT_NATIVE_PATH variable set in the scripts/cocoapods/utils.rb script:

react-native/packages/react-native/scripts/cocoapods/utils.rb

Line 82 in 2441fa2

config.build_settings["REACT_NATIVE_PATH"] = File.join("${PODS_ROOT}", "..", react_native_path)

via

react-native/packages/react-native/template/ios/Podfile

Line 35 in 2441fa2

config[:reactNativePath],

Changelog:

[IOS] [ADDED] - Add use of the REACT_NATIVE_PATH in the "Bundle React Native code and images" shell script build phase. This will help apps initialized into a mono-repo which hoists the react-native package.

Test Plan:

I initialized the React Native template into an NPM workspaces mono-repo and experienced a failure running the script phase. I updated it to the code in this PR, which resolved the issue.

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Dec 17, 2023
@kraenhansen kraenhansen changed the title Use $REACT_NATIVE_PATH instead of relative path [template] Use $REACT_NATIVE_PATH instead of relative path Dec 17, 2023
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 16,525,105 -37
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 19,899,905 -35
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 2441fa2
Branch: main

@facebook-github-bot
Copy link
Contributor

@robhogan has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook facebook deleted a comment from facebook-github-bot Dec 17, 2023
robhogan
robhogan approved these changes Dec 18, 2023
View reviewed changes
Copy link
Contributor

@robhogan robhogan left a comment

Choose a reason for hiding this comment

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

Thanks for this! We still have a bit of work to do to make REACT_NATIVE_PATH itself correct of course, but this is a nice consolidation.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Dec 18, 2023
@facebook-github-bot
Copy link
Contributor

@robhogan merged this pull request in 289e783.

@kraenhansen kraenhansen deleted the kh/ios-react-native-path branch December 18, 2023 17:24
Othinn pushed a commit to Othinn/react-native that referenced this pull request Jan 9, 2024
@kraenhansen @Othinn
Use $REACT_NATIVE_PATH instead of relative path (facebook#41968)
a08a13d 
Summary:
I believe it's valuable to be able to initialise the React Native template into a mono-repo and have it work with zero updates to the configuration.
In its current form the template's Xcode project makes assumptions on the relative location of the `react-native` package, while it could instead use the `REACT_NATIVE_PATH` variable set in the `scripts/cocoapods/utils.rb` script:

https://github.com/facebook/react-native/blob/2441fa284716ef782ec12dd0c2801548f8c47339/packages/react-native/scripts/cocoapods/utils.rb#L82

via

https://github.com/facebook/react-native/blob/2441fa284716ef782ec12dd0c2801548f8c47339/packages/react-native/template/ios/Podfile#L35

## Changelog:

[IOS] [ADDED] - Add use of the `REACT_NATIVE_PATH` in the "Bundle React Native code and images" shell script build phase. This will help apps initialized into a mono-repo which hoists the `react-native` package.

Pull Request resolved: facebook#41968

Test Plan: I initialized the React Native template into an NPM workspaces mono-repo and experienced a failure running the script phase. I updated it to the code in this PR, which resolved the issue.

Reviewed By: christophpurrer, cipolleschi

Differential Revision: D52240559

Pulled By: robhogan

fbshipit-source-id: 1c5710c8ffe9d289f32c5ed83cb58ae27f3c931a
blakef pushed a commit to blakef/template that referenced this pull request Feb 28, 2024
@kraenhansen @facebook-github-bot
Use $REACT_NATIVE_PATH instead of relative path (#41968)
3c68d87 
Summary:
I believe it's valuable to be able to initialise the React Native template into a mono-repo and have it work with zero updates to the configuration.
In its current form the template's Xcode project makes assumptions on the relative location of the `react-native` package, while it could instead use the `REACT_NATIVE_PATH` variable set in the `scripts/cocoapods/utils.rb` script:

https://github.com/facebook/react-native/blob/2441fa284716ef782ec12dd0c2801548f8c47339/packages/react-native/scripts/cocoapods/utils.rb#L82

via

https://github.com/facebook/react-native/blob/2441fa284716ef782ec12dd0c2801548f8c47339/packages/react-native/template/ios/Podfile#L35

## Changelog:

[IOS] [ADDED] - Add use of the `REACT_NATIVE_PATH` in the "Bundle React Native code and images" shell script build phase. This will help apps initialized into a mono-repo which hoists the `react-native` package.

Pull Request resolved: facebook/react-native#41968

Test Plan: I initialized the React Native template into an NPM workspaces mono-repo and experienced a failure running the script phase. I updated it to the code in this PR, which resolved the issue.

Reviewed By: christophpurrer, cipolleschi

Differential Revision: D52240559

Pulled By: robhogan

fbshipit-source-id: 1c5710c8ffe9d289f32c5ed83cb58ae27f3c931a

Original: facebook/react-native@289e783
blakef pushed a commit to react-native-community/template that referenced this pull request Feb 29, 2024
@kraenhansen @facebook-github-bot
Use $REACT_NATIVE_PATH instead of relative path (#41968)
1adc11c 
Summary:
I believe it's valuable to be able to initialise the React Native template into a mono-repo and have it work with zero updates to the configuration.
In its current form the template's Xcode project makes assumptions on the relative location of the `react-native` package, while it could instead use the `REACT_NATIVE_PATH` variable set in the `scripts/cocoapods/utils.rb` script:

https://github.com/facebook/react-native/blob/2441fa284716ef782ec12dd0c2801548f8c47339/packages/react-native/scripts/cocoapods/utils.rb#L82

via

https://github.com/facebook/react-native/blob/2441fa284716ef782ec12dd0c2801548f8c47339/packages/react-native/template/ios/Podfile#L35

## Changelog:

[IOS] [ADDED] - Add use of the `REACT_NATIVE_PATH` in the "Bundle React Native code and images" shell script build phase. This will help apps initialized into a mono-repo which hoists the `react-native` package.

Pull Request resolved: facebook/react-native#41968

Test Plan: I initialized the React Native template into an NPM workspaces mono-repo and experienced a failure running the script phase. I updated it to the code in this PR, which resolved the issue.

Reviewed By: christophpurrer, cipolleschi

Differential Revision: D52240559

Pulled By: robhogan

fbshipit-source-id: 1c5710c8ffe9d289f32c5ed83cb58ae27f3c931a

Original-Commit: facebook/react-native@289e783
blakef pushed a commit to react-native-community/template that referenced this pull request Feb 29, 2024
@kraenhansen @facebook-github-bot
Use $REACT_NATIVE_PATH instead of relative path (#41968)
026cee5 
Summary:
I believe it's valuable to be able to initialise the React Native template into a mono-repo and have it work with zero updates to the configuration.
In its current form the template's Xcode project makes assumptions on the relative location of the `react-native` package, while it could instead use the `REACT_NATIVE_PATH` variable set in the `scripts/cocoapods/utils.rb` script:

https://github.com/facebook/react-native/blob/2441fa284716ef782ec12dd0c2801548f8c47339/packages/react-native/scripts/cocoapods/utils.rb#L82

via

https://github.com/facebook/react-native/blob/2441fa284716ef782ec12dd0c2801548f8c47339/packages/react-native/template/ios/Podfile#L35

## Changelog:

[IOS] [ADDED] - Add use of the `REACT_NATIVE_PATH` in the "Bundle React Native code and images" shell script build phase. This will help apps initialized into a mono-repo which hoists the `react-native` package.

Pull Request resolved: facebook/react-native#41968

Test Plan: I initialized the React Native template into an NPM workspaces mono-repo and experienced a failure running the script phase. I updated it to the code in this PR, which resolved the issue.

Reviewed By: christophpurrer, cipolleschi

Differential Revision: D52240559

Pulled By: robhogan

fbshipit-source-id: 1c5710c8ffe9d289f32c5ed83cb58ae27f3c931a

Original-Commit: facebook/react-native@289e783
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robhogan robhogan robhogan approved these changes

Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kraenhansen @analysis-bot @facebook-github-bot @robhogan