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.

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

feat(ios): allow for custom project dir in react-native-xcode script #35449

Closed
wants to merge 3 commits into from

Conversation

itxch
Copy link
Contributor

@itxch itxch commented Nov 23, 2022

Summary

The react-native-xcode script assumes the that node_modules folder are in the same location as the react native project. This adds the ability to declare CUSTOM_DIR in .xcode.env file to let react-native-xcode know where the react native project is located - required for monorepos

Issue

At the moment if a project is not using the default react native file structure then when trying to create a build or archive using Xcode it will fail, with the following error:

File /Users/itxch/Library/Developer/Xcode/DerivedData/app-evgjukoxhuupzzdglopjnhcntvpw/Build/Intermediates.noindex/ArchiveIntermediates/jungul/BuildProductsPath/Release-iphoneos/app.app/main.jsbundle does not exist. This must be a bug with React Native, please report it here: https://github.com/facebook/react-native/issues

Background

My project is a monorepo, using npm workspaces.

This is the relevant file structure

files

This file structure is incompatible with line 63 of react-native-xcode.sh

PROJECT_ROOT=${PROJECT_ROOT:-"$REACT_NATIVE_DIR/../.."}

Note: $REACT_NATIVE_DIR is the react-native package directory inside node modules.

So by default this sets project root to be the parent folder of the node_modules folder. However in my case this is not correct, it should actually be:

PROJECT_ROOT=${PROJECT_ROOT:-"$REACT_NATIVE_DIR/../../app"}

Solution

Seeing as hardcoding the value would not work, the simplest solution is to set the directory via a variable

PROJECT_ROOT=${PROJECT_ROOT:-"$REACT_NATIVE_DIR/../../$CUSTOM_DIR"}

$CUSTOM_DIR can easily be set via .xcode.env file.

Changelog

[IOS] [Added] - allow for custom project dir in react-native-xcode script

Test Plan

After updating my .xcode.env file to include export CUSTOM_DIR=app, I can successfully create a build or an archive.
If the variable is not set it will be black, so it will not effect $PROJECT_ROOT

@facebook-github-bot
Copy link
Contributor

Hi @itxch!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

@react-native-bot react-native-bot added Platform: iOS iOS applications. Type: Enhancement A new feature or enhancement of an existing feature. labels Nov 23, 2022
@analysis-bot
Copy link

analysis-bot commented Nov 23, 2022

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,102,652 +4,816
android hermes armeabi-v7a 6,470,734 +3,779
android hermes x86 7,520,124 +4,076
android hermes x86_64 7,378,661 +4,292
android jsc arm64-v8a 8,967,444 +4,432
android jsc armeabi-v7a 7,698,293 +3,579
android jsc x86 9,029,584 +3,863
android jsc x86_64 9,507,429 +4,276

Base commit: 6003e70
Branch: main

@analysis-bot
Copy link

analysis-bot commented Nov 23, 2022

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 6003e70
Branch: main

@pull-bot
Copy link

PR build artifact for 62bbc96 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@pull-bot
Copy link

PR build artifact for 62bbc96 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 23, 2022
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@iBotPeaches
Copy link
Contributor

Hi @itxch,

I recently got something merged in regards to fixing our monorepo setup and it was resolved with this merge here: 4d7ddd4

What I find interesting about this PR is the xcode.env change as that makes it so much easier for other contributors to figure out how to adapt the build system for a monorepo setup. So I do like that aspect of this PR.

When I see CUSTOM_DIR - it doesn't really have much meaning to me. It just says "CUSTOM" which doesn't really depict what that variable should represent. Is it the project root? node modules root? etc

When I see you are using that to just compound onto PROJECT_ROOT. Why not just use the overridable aspect of PROJECT_ROOT that you can now do? Was there a reason that did not work that led you to this?

Since I think the only change we might need here is just an easier documented source of why you would change PROJECT_ROOT (ie monorepos)

@itxch
Copy link
Contributor Author

itxch commented Nov 24, 2022

Thanks for the reply @iBotPeaches

What I find interesting about this PR is the xcode.env change as that makes it so much easier for other contributors to figure out how to adapt the build system for a monorepo setup. So I do like that aspect of this PR.

Thanks :) coming from the react web world, I love .env files (with good docs).

Why not just use the overridable aspect of PROJECT_ROOT that you can now do? Was there a reason that did not work that led you to this?

I didn't realise that :- meant only override if it does not exist. I will test, if it works - and I see no reason for it not to - I will update the PR, to just be a change to .xcode.env file describing why you would want to set the PROJECT_ROOT variable

@itxch
Copy link
Contributor Author

itxch commented Nov 24, 2022

Was able to easily set PROJECT_DIR in .xcode.env file
export PROJECT_ROOT="$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )/../"
This sets it to the parent directory of the .xcode.env file.

Will update PR to just have some documentation around this - I'd say a comment in the .env file is good.

Thoughts @iBotPeaches

@iBotPeaches
Copy link
Contributor

Thoughts @iBotPeaches

Sounds like a plan. Glad it worked out

@facebook-github-bot
Copy link
Contributor

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

@cipolleschi
Copy link
Contributor

Thank you so much for taking care of this. We would like to end up in a world where it's very easy to configure React Native and this PR goes in that direction! :D

I imported the PR to keep an eye on it, but I agree with the suggested changes. I'll wait to land it until those are carried out.

@itxch
Copy link
Contributor Author

itxch commented Nov 30, 2022

@cipolleschi

Updated the PR:

# If you're not using the default file structure, e.g. a monorepo
# then you can pass in the root react native directory.
# E.g. To set it to be the parent directory of the ios folder
# export PROJECT_ROOT="$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )/../"

To be honest line 16 could probably be uncommented, as its a good default I feel like. Whats your thoughts?

@pull-bot
Copy link

PR build artifact for 7db8204 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@pull-bot
Copy link

PR build artifact for 7db8204 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@cortinico
Copy link
Contributor

To be honest line 16 could probably be uncommented, as its a good default I feel like. Whats your thoughts?

Jumping on this as Riccardo is off for some days.
I would refrain from uncommenting this unless is needed for the infra to work correctly @itxch

@itxch
Copy link
Contributor Author

itxch commented Dec 1, 2022

Jumping on this as Riccardo is off for some days. I would refrain from uncommenting this unless is needed for the infra to work correctly @itxch

Sure, we can keep it commented - its only required for monorepos

@facebook-github-bot
Copy link
Contributor

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

@github-actions
Copy link

github-actions bot commented Dec 2, 2022

This pull request was successfully merged by @itxch in 436da18.

When will my fix make it into a release? | Upcoming Releases

@github-actions github-actions bot added the Merged This PR has been merged. label Dec 2, 2022
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
…acebook#35449)

Summary:
The `react-native-xcode` script assumes the that `node_modules` folder are in the same location as the react native project. This adds the ability to declare `CUSTOM_DIR` in `.xcode.env` file to let react-native-xcode know where the react native project is located - required for monorepos

## Issue
At the moment if a project is not using the default react native file structure then when trying to create a build or archive using Xcode it will fail, with the following error:

> File /Users/itxch/Library/Developer/Xcode/DerivedData/app-evgjukoxhuupzzdglopjnhcntvpw/Build/Intermediates.noindex/ArchiveIntermediates/jungul/BuildProductsPath/Release-iphoneos/app.app/main.jsbundle does not exist. This must be a bug with React Native, please report it here: https://github.com/facebook/react-native/issues

## Background
My project is a monorepo, using `npm` workspaces.

#### This is the relevant file structure
![files](https://user-images.githubusercontent.com/54910400/203641210-596ee866-9f9f-429c-8bae-62d0f3afc623.png)

This file structure is incompatible with line 63 of `react-native-xcode.sh`

https://github.com/facebook/react-native/blob/c5a8425fada10b715f356731426db666975569c9/scripts/react-native-xcode.sh#L63
Note: `$REACT_NATIVE_DIR` is the `react-native` package directory inside node modules.

So by default this sets project root to be the parent folder of the `node_modules` folder. However in my case this is not correct, it should actually be:
```sh
PROJECT_ROOT=${PROJECT_ROOT:-"$REACT_NATIVE_DIR/../../app"}
```

## Solution
Seeing as hardcoding the value would not work, the simplest solution is to set the directory via a variable
```sh
PROJECT_ROOT=${PROJECT_ROOT:-"$REACT_NATIVE_DIR/../../$CUSTOM_DIR"}
```
`$CUSTOM_DIR` can easily be set via `.xcode.env` file.

## Changelog
[IOS] [Added] - allow for custom project dir in react-native-xcode script

Pull Request resolved: facebook#35449

Test Plan:
After updating my `.xcode.env` file to include `export CUSTOM_DIR=app`, I can successfully create a build or an archive.
If the variable is not set it will be black, so it will not effect `$PROJECT_ROOT`

Reviewed By: GijsWeterings

Differential Revision: D41529396

Pulled By: cortinico

fbshipit-source-id: 890e9867e31f83a08561df8c2de1069e771726fc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. Platform: iOS iOS applications. Type: Enhancement A new feature or enhancement of an existing feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants