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

chore: suggest adding flag in Podfile instead of console #4284

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

WoLewicki
Copy link
Contributor

PR changing the suggested way of switching the new/old arch on iOS so it is similar to Android. Also, it seems more reasonable to have it in the Podfile, otherwise you (and your whole team) would have to remember about doing it every time.

PR changing the suggested way of switching the new/old arch on `iOS` so it is similar to `Android`. Also, it seems more reasonable to have it in the `Podfile`, otherwise you (and your whole team) would have to remember about doing it every time.
Copy link

netlify bot commented Oct 17, 2024

Deploy Preview for react-native ready!

Name Link
🔨 Latest commit 118bb31
🔍 Latest deploy log https://app.netlify.com/sites/react-native/deploys/6710ec0f955a5400082b37b9
😎 Deploy Preview https://deploy-preview-4284--react-native.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@j-piasecki
Copy link
Contributor

Out of curiosity, should the new arch be enabled using environmental variable or inside Podfile?

use_react_native!(
    ...
    new_arch_enabled: true,
    ...
)

@@ -230,8 +230,10 @@ If, for any reasons, you can't use the New Architecture, you can still opt-out f
### iOS
1. Install your CocoaPods dependencies with the command:
1. Open the `ios/Podfile` file
2. Add `ENV['RCT_NEW_ARCH_ENABLED'] = '0'` in the main scope of the file
Copy link
Contributor

Choose a reason for hiding this comment

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

in the main scope of the file

Can we simplify this? Or can we add a diff block to explain this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

add a diff block to explain this?

People often have many different things in their Podfile so not sure about the diff. What about:
2. Add ENV['RCT_NEW_ARCH_ENABLED'] = '0' at the top of the file

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
2. Add `ENV['RCT_NEW_ARCH_ENABLED'] = '0'` in the main scope of the file
2. Add `ENV['RCT_NEW_ARCH_ENABLED'] = '0'` in the main scope of the Podfile ([reference Podfile](https://github.com/react-native-community/template/blob/0.76-stable/template/ios/Podfile) in the template)
```diff
+ ENV['RCT_NEW_ARCH_ENABLED'] = '0'
# Resolve react_native_pods.rb with node to allow for hoisting
require Pod::Executable.execute_command('node', ['-p',
'require.resolve(

@cipolleschi
Copy link
Contributor

@j-piasecki the two approach are alternatives.
You can set the ENV var or you can use that boolean. The reason why I'd rather go with the ENV variable is that you can define the config at the beginning of the file, while the parameter of the use_react_native! is a bit hidden in the podfile and easy to miss... :(

The Podfile used to be much leaner when I introduced the parameter, thus easier to use..

Copy link
Contributor

@cortinico cortinico left a comment

Choose a reason for hiding this comment

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

Accepting to unblock, I'm good with @cipolleschi comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants